refactor(instr-amqplib): use exported strings for attributes

[david-luna]

Which problem is this PR solving?

• Updates Update usage of semantic conventions to new exported strings #2025

Short description of the changes

• Update @opentelemetry/semantic-conventions to ^1.22
• Replace SemanticAttributes.* with exported strings SEMATTRS_*
• Update README with new semantic convention package version and key

[codecov]

Codecov Report

Merging #2086 (91e8ad9) into main (dfb2dff) will decrease coverage by 0.16%.
Report is 49 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head 91e8ad9 differs from pull request most recent head 3d0517d. Consider uploading reports for the commit 3d0517d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2086      +/-   ##
==========================================
- Coverage   90.97%   90.82%   -0.16%     
==========================================
  Files         146      148       +2     
  Lines        7492     7672     +180     
  Branches     1502     1537      +35     
==========================================
+ Hits         6816     6968     +152     
- Misses        676      704      +28     

Files	Coverage Δ
...lugins/node/instrumentation-amqplib/src/amqplib.ts	90.40% <100.00%> (ø)
plugins/node/instrumentation-amqplib/src/utils.ts	95.38% <100.00%> (ø)

... and 13 files with indirect coverage changes

[JamieDanielson]

Thanks! I left a comment about maybe doing the Attributes and AttributeValue change separate... but I'm not sure if it's necessary as I look at it more. If anything that should maybe be another issue similar to this, where we have a breakdown of issues that need to have deprecated SpanAttributes and SpanAttributeValue replaced with the new Attributes and AttributeValue here in contrib 🤔 It's a breaking change if minimum API version needs to the change, but it's not necessary here so it wouldn't be breaking here.

[JamieDanielson]

This look good. I don't think the SpanAttribute change needs to be separate here. On the issue #1778 we discussed maybe not needing the Notes column with the key after all, but it's already here and I don't think it's necessary to do the extra work to remove it. If anything we can remove it when we update semantic conventions, to minimize overhead.

[david-luna]

Thanks for the review @JamieDanielson :)

I guess the breaking change would happen when SpanAttributes and SpanAttributeValue objects are finally removed from @opentelemetry/semantic-conventions. All packages without the imports updated will fail to compile.

Actually that looks to be specifically pointing out packages in core that have a lower API version, and there may not be a next branch on contrib. 🤔

I thought so and it makes sense to remove deprecated object from current instrumentations. I wonder which kind of version bump we should do when the instrumentations update the API/SDK dependencies to next branch

[JamieDanielson]

SpanAttributes and SpanAttributeValue objects are finally removed from @opentelemetry/semantic-conventions.

These are actually in the API package, not semantic convention, but either way I think we're good here.

I wonder which kind of version bump we should do when the instrumentations update the API/SDK dependencies to next branch

I wonder this too! I think we may have talked about it briefly during one of the sig meetings, but as we get closer to release, we will have to revisit.

[trentm]

@david-luna Should this one have also updated the instr-amqplib test files?

% rg SemanticAttributes
test/amqplib-promise.test.ts
40:  SemanticAttributes,
150:        publishSpan.attributes[SemanticAttributes.MESSAGING_SYSTEM]
153:        publishSpan.attributes[SemanticAttributes.MESSAGING_DESTINATION]
156:        publishSpan.attributes[SemanticAttributes.MESSAGING_DESTINATION_KIND]
...

[david-luna]

Indeed. Good catch :)

I'll create a new PR for it

Thanks!