Skip to content

[SEMANTIC CONVENTIONS] Upgrade to semconv version 1.23.1#2428

Merged
marcalff merged 4 commits intoopen-telemetry:mainfrom
marcalff:fix_semconv_2419
Dec 5, 2023
Merged

[SEMANTIC CONVENTIONS] Upgrade to semconv version 1.23.1#2428
marcalff merged 4 commits intoopen-telemetry:mainfrom
marcalff:fix_semconv_2419

Conversation

@marcalff
Copy link
Copy Markdown
Member

@marcalff marcalff commented Dec 4, 2023

Fixes #2419

Changes

Upgrade to:

  • semantic conventions 1.23.1
  • build-tools 0.23.0

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@marcalff marcalff requested a review from a team December 4, 2023 23:23
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2023

Codecov Report

Merging #2428 (6e457ba) into main (a86f8f0) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2428   +/-   ##
=======================================
  Coverage   87.06%   87.06%           
=======================================
  Files         199      199           
  Lines        6079     6079           
=======================================
  Hits         5292     5292           
  Misses        787      787           

@marcalff marcalff mentioned this pull request Dec 4, 2023
Copy link
Copy Markdown
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Difficult to identify the difference due to the change in the order of definitions. Hopefully, the generate script should be good to rely on. Else, let me know if something needs to be reviewed specifically.

@marcalff
Copy link
Copy Markdown
Member Author

marcalff commented Dec 5, 2023

Thanks. Difficult to identify the difference due to the change in the order of definitions. Hopefully, the generate script should be good to rely on. Else, let me know if something needs to be reviewed specifically.

Yes, relying on the generated code too.

I think the build-tools can be improved to may be sort all symbols first, to generate them in a more predictable and stable order, to limit spurious diffs ... I will raise that with build-tools.

I use a smoke test to check the diff sanity, looked ok:

git diff | grep static | sort -u

it will list a lot of semconv with +1 and -1 lines for the same name, indicating the declaration moved but was unchanged.

@lalitb
Copy link
Copy Markdown
Member

lalitb commented Dec 5, 2023

I think the build-tools can be improved to may be sort all symbols first, to generate them in a more predictable and stable order, to limit spurious diffs ... I will raise that with build-tools.

Agree, a more predictable order would help.

@marcalff marcalff merged commit 39ad238 into open-telemetry:main Dec 5, 2023
@marcalff marcalff deleted the fix_semconv_2419 branch June 3, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SEMANTIC CONVENTIONS] Upgrade to semconv version 1.23.1

2 participants