-
Notifications
You must be signed in to change notification settings - Fork 868
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
Deprecate NetServerAttributesExtractor
#9156
Deprecate NetServerAttributesExtractor
#9156
Conversation
NetServerAttributesExtractor
SemconvStability.emitStableHttpSemconv(), | ||
SemconvStability.emitOldHttpSemconv()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've decided to have the new extractors emit the old spec as well: this will ease the transition from the old Net*
classes, and will reduce the @SuppressWarnings("deprecation")
annotations in our codebase.
The only attributes that are not covered by the new extractors are net.transport
and net.sock.family
(because the new attributes have different values), but there's like ~5 instrumentations using these anyway, so we'll just suppress deprecations there.
65cb9bb
to
cd74de0
Compare
cd74de0
to
fc58bc5
Compare
@@ -10,7 +10,6 @@ | |||
import io.opentelemetry.api.common.AttributesBuilder; | |||
import io.opentelemetry.context.Context; | |||
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; | |||
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you removed the import and just use the lengthy package name below. You did this in several places, so it looks intentional....it's just not clear to me how/why this helps the deprecation effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Errorprone requires that when you use deprecated types.
I don't like this either, but I can kinda agree with that: using deprecated classes should look ugly, to encourage you to refactor the code that uses them
Another part of #9013
The
NetClientAttributesExtractor
will be done in the next (last for this issue) PR.