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

update semconv to 1.26.0 #1316

Merged
merged 2 commits into from
Jun 28, 2024
Merged

Conversation

brettmc
Copy link
Collaborator

@brettmc brettmc commented May 22, 2024

note that messaging.client_id was renamed to messaging.client.id, which causes a conflict in const name. It hasn't been resolved upstream yet, but manually removing the duplicate seems like the best way to go for us.

note that messaging.client_id was renamed to messaging.client.id, which causes a conflict in const name.
It hasn't been resolved upstream yet, but manually removing the duplicate seems like the best way to
go for us.
@brettmc brettmc requested a review from a team May 22, 2024 03:53
@@ -17,3 +17,13 @@
* @deprecated Use CONTAINER_IMAGE_TAGS
*/
public const CONTAINER_IMAGE_TAG = 'container.image.tag';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these two were removed from semconv upstream

@@ -3,31 +3,11 @@
*/
public const FAAS_EXECUTION = 'faas.execution';

/**
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all these guys were re-added to semconv as deprecated, so no longer needed here.

Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.23%. Comparing base (eaba9e3) to head (dd5ccd9).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1316      +/-   ##
============================================
- Coverage     74.28%   74.23%   -0.05%     
  Complexity     2491     2491              
============================================
  Files           353      353              
  Lines          7135     7135              
============================================
- Hits           5300     5297       -3     
- Misses         1835     1838       +3     
Flag Coverage Δ
8.1 73.95% <ø> (-0.05%) ⬇️
8.2 74.19% <ø> (-0.03%) ⬇️
8.3 74.18% <ø> (+0.01%) ⬆️
8.4 74.14% <ø> (-0.05%) ⬇️

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

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

... and 4 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 eaba9e3...dd5ccd9. Read the comment docs.

@ChrisLightfootWild ChrisLightfootWild requested a review from a team May 22, 2024 05:28
@brettmc brettmc marked this pull request as draft May 22, 2024 23:43
@brettmc
Copy link
Collaborator Author

brettmc commented May 22, 2024

Converting back to draft, awaiting agreement in open-telemetry/semantic-conventions#1031 on how to handle messaging.client.id

@brettmc
Copy link
Collaborator Author

brettmc commented Jun 28, 2024

The recommendation for messaging.client_id -> messaging.client.id would be to drop the old attribute.

The recommendation from semconv maintainers is to clobber the old, which is what this PR does.

@brettmc brettmc marked this pull request as ready for review June 28, 2024 01:50
@brettmc brettmc merged commit a822c45 into open-telemetry:main Jun 28, 2024
9 of 10 checks passed
@brettmc brettmc deleted the semconv-1_26_0 branch June 28, 2024 01:51
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