Skip to content

refactor semantic conventions #1659

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

Merged
merged 7 commits into from
Aug 4, 2025
Merged

Conversation

brettmc
Copy link
Contributor

@brettmc brettmc commented Jul 7, 2025

Align with how the spec advises SemConv should be organised, and break incubating (stable + unstable) semconvs out into their own directory.
I played around with generating values as enums, but I could only get this to work if the output was in the same file as the attributes (which would not work well with composer autoloading), so I've used <attribute>_VALUE_<name>, similar to what we have now except all in the same file as the attributes.

Discussion:

  • From here on, we will not regenerate our existing *Attributes and *AttributeValues files, but try to memorialize them to preserve BC (to discuss: marking these as deprecated now will make a lot of pipelines unhappy, should we wait?)
  • I haven't included any deprecated entries for both stable/incubating. I don't think weaver has a way to filter deprecations from a point-in-time, so it's all-or nothing. If there are deprecations of stable semconvs in the future, we'll need to deal with that (for reference, Java excludes deprecations from their stable SemConv generation: https://github.com/open-telemetry/semantic-conventions-java/blob/release/v1.34.0/buildscripts/templates/registry/java/weaver.yaml#L24)

Align with how the spec advises SemConv should be organised, and break unstable
semconvs out into their own directory.
I played around with generating values as enums, but I could only get this to work
if the output was in the same file as the attributes (which would not work well with
composer autoloading), so I've used <attribute>_VALUE_<name>, similar to what we have
now except all in the same file.
@brettmc brettmc requested a review from a team as a code owner July 7, 2025 06:38
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.66%. Comparing base (3968f74) to head (af9a21b).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1659      +/-   ##
============================================
+ Coverage     68.64%   68.66%   +0.02%     
  Complexity     2854     2854              
============================================
  Files           425      425              
  Lines          8699     8699              
============================================
+ Hits           5971     5973       +2     
+ Misses         2728     2726       -2     
Flag Coverage Δ
8.1 68.41% <ø> (+0.03%) ⬆️
8.2 68.62% <ø> (+0.03%) ⬆️
8.3 68.59% <ø> (+0.01%) ⬆️
8.4 68.59% <ø> (-0.02%) ⬇️
8.5 68.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/SemConv/Version.php 100.00% <ø> (ø)

... and 5 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3968f74...af9a21b. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brettmc brettmc mentioned this pull request Jul 7, 2025
@brettmc brettmc merged commit 9d68afb into open-telemetry:main Aug 4, 2025
11 checks passed
WyriHaximus added a commit to WyriHaximus-labs/opentelemetry-php that referenced this pull request Aug 24, 2025
brettmc pushed a commit that referenced this pull request Aug 24, 2025
… (#1695)

* Update weaver to v0.17.1 from v0.16.1

This resolves: #1694

* [SemConv] Add TLS trace attributes to new structure introduced in #1659
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.

3 participants