-
Notifications
You must be signed in to change notification settings - Fork 889
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
zipkin: Handle older semconv #3794
Conversation
It is quite unfortunate that we referenced experimental semconv from a Stable doc. You are right that formal rules requires us to make changes to this Stable doc in a way that preserves backward compatibility. At the same time it seems wrong to carry this duplicate information because we made a mistake in the past. I wonder if we have a way out here? |
I am not sure if it is really a "duplicate information". The intention is that Zipkin's I think it may be even more important to handle the old semconv than |
Nevermind. I didn't realize it is a ranked choice of one, not all of those. |
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.
LGTM pending @chalin's input on links and how that interacts with the website.
Ping @chalin |
I'm back! Looking at this now. |
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.
See inline for a new suggested pattern regex.
LGTM once the regex is updated per the comment.
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
@carlosalberto I think this PR is ready to be merged. |
Hey @Oberon00 As @trask mentioned, the note above the table should cover the case (as it actually covers all the listed values, e.g.
Please fill an issue as a followup in case you think we should add details about this. Hope that's fine! |
Fixes open-telemetry#3793 ## Changes Update OpenTelemetry to Zipkin Transformation to handle attributes from older semantic conventions in a backwards compatible way so that following attributes are (again) handled: - `net.peer.name` - `net.host.name` - `net.sock.peer.addr` & `net.sock.peer.port` - `server.socket.domain` - `server.socket.address` & `server.socket.port` The backwards compatibility of a stable OpenTelemetry to Zipkin Transformation was broken by: - open-telemetry#3402 - open-telemetry#3713
Fixes #3793
Changes
Update OpenTelemetry to Zipkin Transformation to handle attributes from older semantic conventions in a backwards compatible way so that following attributes are (again) handled:
net.peer.name
net.host.name
net.sock.peer.addr
&net.sock.peer.port
server.socket.domain
server.socket.address
&server.socket.port
The backwards compatibility of a stable OpenTelemetry to Zipkin Transformation was broken by:
net.peer.*
/net.host.*
withclient.*
/server.*
(andsource.*
/destination.*
) #3402(client|server).socket.(address|port)
attributes withnetwork.(peer|local).(address|port)
. #3713