Skip to content
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

Clarification in Distributed Tracing Extension #603

Closed
slinkydeveloper opened this issue Apr 14, 2020 · 6 comments · Fixed by #607
Closed

Clarification in Distributed Tracing Extension #603

slinkydeveloper opened this issue Apr 14, 2020 · 6 comments · Fixed by #607

Comments

@slinkydeveloper
Copy link
Member

slinkydeveloper commented Apr 14, 2020

In the Distributed Tracing Extension, there is this paragraph which looks unclear to me: https://github.com/cloudevents/spec/blob/master/extensions/distributed-tracing.md#conflicts

My interpretation is: traceparent could live both as event extension and as http headers conforming to https://www.w3.org/TR/trace-context/ and, if that's the case, the one as event extension supersedes the one as standard header. Couple of questions:

  • What if an event with both representations not matching goes through a middleware non-cloudevents aware but tracing aware?
  • What should do a cloudevents middleware when there are both attributes? Should "sanitize" the event, removing one header and keeping the other?

I also have a proposal: would it be better to have the representations exclusive? aka: or you transport the extension in the protocol specific encoding (aka for http is https://www.w3.org/TR/trace-context/) or you transport it as ce attributes.

Links to knative/eventing#2918

@duglin
Copy link
Collaborator

duglin commented Apr 23, 2020

If I'm remembering correctly, when we discussed this situation we decided that if the two values are different then the CE one would be used to represent "the original" value from the producer - if that was of interest. While the non-CE one would be the true, aggregated, value. And it was the non-CE one that the tracing infrastructure in the consumer would use.

What if an event with both representations not matching goes through a middleware non-cloudevents aware but tracing aware?

Then it would process the tracing header per the tracing spec - which I believe includes the possibility of it modifying it, but ignore the CE one (meaning leave it untouched).

What should do a cloudevents middleware when there are both attributes? Should "sanitize" the event, removing one header and keeping the other?

Nope, it would leave the CE one untouched.

I also have a proposal: would it be better to have the representations exclusive?

I think we decided that we can't do that because then we'd lose data (or get things out of sync) if the message went thru both CE and non-CE aware middleware, but they were trace aware.

Anyone else remember? Is my memory correct?

@slinkydeveloper
Copy link
Member Author

slinkydeveloper commented Apr 23, 2020

If I'm remembering correctly, when we discussed this situation we decided that if the two values are different then the CE one would be used to represent "the original" value from the producer - if that was of interest. While the non-CE one would be the true, aggregated, value. And it was the non-CE one that the tracing infrastructure in the consumer would use.

My understanding is the opposite, from the spec:

In those cases, the serialization that followed the "general CloudEvents serialization rules" MUST be used as the CloudEvents attribute. The other, secondary, mapping MAY be picked-up and offered to the receiving application as "additional" metadata.

@slinkydeveloper
Copy link
Member Author

slinkydeveloper commented Apr 23, 2020

@clemensv @duglin there is something i still don't get. We have an use case where we have a middleware which receive events via http, push them to kafka and on the other side send these event back with http to a subscription. In the implementation I'm working on right now (knative/eventing-contrib#1155) we set the traceparent extension to propagate the trace through kafka, so the "kafka consumer" gets the traceparent back, builds a new span and continue the tracing.

After the discussion we had today in the WG meeting, I understood that I shouldn't, for any reason, touch the event, so i suppose my implementation should propagate the traceparent in other way. Am I right?

This brings me to another question: who should be the "entity" that sets this extension? only the original event source right?

@duglin
Copy link
Collaborator

duglin commented Apr 25, 2020

As I understand it, the trace info should follow the tracing spec w.r.t. placement and propagation across hops. The fact that the original value also appears in a CE attribute should not influence that. So, you could think of it as: just ignore the CE trace attribute unless (for some reason) you really want to know the original trace value. Which, as you said on the call, you probably won't need. TBH, I'm not 100% sure why this CE extension is useful.

Yes, I think the entity who added the trace info first should set the CE extension too - if it's aware of CE.

@duglin
Copy link
Collaborator

duglin commented Apr 25, 2020

we discussed this on this week's call, and @slinkydeveloper will try to write-up a PR

@slinkydeveloper
Copy link
Member Author

slinkydeveloper commented Apr 27, 2020

TBH, I'm not 100% sure why this CE extension is useful.

Same here: if you are tracing an event, and you have one of the child traces in the usual trace context headers, you can go back to the original trace and check out the whole message flow, so i don't see the point of transporting the original traceparent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants