-
Notifications
You must be signed in to change notification settings - Fork 896
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 Zipkin remoteEndpoint preferences #3087
Update Zipkin remoteEndpoint preferences #3087
Conversation
Do not reference semantic conventions that have been removed.
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.
Can you also update
<td>Attributes["net.peer.ip"]</td> |
and
<td>Attributes["net.peer.name"]</td> |
for consistency?
I would rather scope this PR to updating the Zipkin remote endpoint preferences. Maybe that is a good change for another PR? |
I am fine with this.
Let me know if you want to do it or I should create it. |
Yeah, please create one. Thanks! |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@lmolkova PTAL |
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.
Thanks!
@pellared Is your changes request still valid? |
Comments addressed. Approved 👍 |
cc @yurishkuro |
tbh, I don't understand how to read that table. It gives OTEL names and OTEL definitions - how does that help in translating to Zipkin format? I would've expected either the equivalent Zipkin tag or the dedicated field in Zipkin data model. |
The table is an ordered ranking. This PR does not introduce or advocate for the table, it only updates semantic conventions. The table currently contains semantic conventions that were removed. This removes the stale values and replaces them with the replacement semantic conventions. It is an editorial change. |
This looks good to go - will merge right after 1.18.0 is released, in case there's any follow up (by Yuri or anybody that knows Zipkin better than us). |
* Update Zipkin remoteEndpoint preferences Do not reference semantic conventions that have been removed. * net.peer.port -> net.sock.peer.port
Do not reference semantic conventions that have been removed.
net.peer.ip
attribute was removed: Define net.sock attributes and clarify logical net.peer|host.name attributes #2614net.sock.peer.addr
and ornet.sock.peer.name
http.host
attribute was removed: Remove alternative attribute sets from HTTP semantic conventions #2469