-
Notifications
You must be signed in to change notification settings - Fork 132
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
[GH-75][GH-76] Document "component" tag and all other standardized tags in spec #81
Conversation
These look reasonable, especially inside the tracer itself as it is
implicitly only one side of an operation. I like the labels.
There is some trouble in systems where a span includes both the client and
server spans (ex zipkin v1), a query requires context as they would repeat.
Ex two component tags in the same span, and queries usually imply single
value.
For example, would a zipkin span that includes a client and server be kind
client or kind server? Which component wins? I'm not saying zipkin like
systems don't already have this problem, just noting impact. Ex In zipkin
we have local_component, which notes this is for a single-host span. That's
what local means.. That the span doesn't include two sides of an rpc.
I'm not saying don't publish these, just noting impact that might steer if
or how to note usage.
|
@adriancole good point. Not being intimately familiar with the implementation, would it be possible for the Zipkin collector to take advantage of these keys even as it reduces to a single span? For instance, would you see a Zipkin OT tracer implementation reporting the same span twice, once from each side of client/server? If so, when the collection tier stitches them together, it could use redundant/conflicting data like |
@dkuebric this is some missing (imo) functionality in Zipkin query and UI for preserving the ownership of data elements in those combined spans, even though it is available in the raw data. I do think there is value in each side reporting its own |
I should have guessed this thread would be hijacked as zipkin ui feature
request. I'm not going to indulge this further, as history has taught me
such an act will just soak up 2 hours rehashing things.
|
I'm sure I'm digging a hole, but to close the side-bar openzipkin/zipkin#963 is zipkin issue on presenting shared spans, and openzipkin/zipkin#989 (comment) talks about the duplicate tag issue. There's a display column to disambiguate annotations: log events, but not tags (binary annotations). Events vary on time and host, basically, and eventhough the underlying zipkin datastructure for tags includes host, it isn't displayed. Why? a couple reasons. First, it hasn't become important enough for anyone to actually raise a pull request. Also, there has been a majority of zipkin contributors that think the underlying tag datastructure should be a string->string per span. The complexity of the "binary annotation" object is actually the opposite, if you listen to others. The UI isn't wrong, the data structure should be simpler! But guess what.. no-one has changed that either. back to my original suggestion.
That's all I meant. Just consider an impact that may not be obvious. There are many tracing systems not participating on this thread, for example, and they may have a similar issue. |
@adriancole I don’t have nearly enough context on the open Zipkin issue or – especially – the history behind it (nor am I asking for such context), but FWIW I read @yurishkuro’s comment above as an attempt to offer an on-topic opinion:
... in the context of a real tracing system (Zipkin). In particular, I did not read Yuri's comment as a feature request about Zipkin. Though I agree that this PR is not the place to make such requests. ....... As for the |
@adriancole apologies if my comment sounded dismissive of your concern. I do not believe that Zipkin v1 model is fundamentally incapable of capturing identical tag names submitted separately by server and client tracers. Cassandra backend captures that data properly and makes it available in the raw data, making the disambiguation a UI issue. I could have clarified my comment by mentioning Cassandra backend. But by the same token, you could have also qualified your comment that the issue is only with some Zipkin backends, which for some reasons choose to ignore the data dimension they receive from upstream. Even for those Zipkin backends that literally merge spans at the storage level and lose association of tags, they can pre-process the spans at ingestion, as @dkuebric mentioned above. If a tracer follows OT recommendation, it would also emit "span kind" tag, so that collector (which still sees spans pre-merge) can replace I agree, it is worth mentioning this approach on the "Implementing OT" page, but I don't think it should affect the data specification or how people instrument their apps/libs with OT. |
btw, I think this PR is superseded by a larger change in #82 |
Closing in favour of #82 |
See #75, but also documented all other tags in the spec (that is not done yet) as per #76