Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SEMANTIC CONVENTIONS] Upgrade to semconv version 1.23.1 #2428

Merged
merged 4 commits into from
Dec 5, 2023

Conversation

marcalff
Copy link
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
Copy link

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
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
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
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
45 checks passed
@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