Propagate trace inforation from incoming headers#7932
Conversation
Signed-off-by: Yevgen Polyak <[email protected]>
| 2. Install [Delve](https://github.com/go-delve/delve/tree/master/Documentation/installation) for Go debugging, if desired. | ||
|
|
||
| 3. Install [golangci-lint](https://golangci-lint.run/usage/install) version 1.55.2. | ||
| 3. Install [golangci-lint](https://golangci-lint.run/welcome/install/) version 1.55.2. |
There was a problem hiding this comment.
Seems it is not relative to the main topic, please update it in other PR, one thing one PR is good enough.
There was a problem hiding this comment.
just to be clear, the old link is broken. I've removed this change, but please don't expect a separate PR from me.
| return err | ||
| } | ||
| } else { | ||
| // all messages consumed with "rawPayload=false" are deserialized as a CloudEvent, even when the payload is not a CloudEvent |
There was a problem hiding this comment.
What's the best place to document it? Here, I'm just stating what's happening. You can submit any payload without specifying rawPayload=true and it will treat it as a CloudEvent.
There was a problem hiding this comment.
this is an implementation detail tbh. It doesn't mean that they will be sent as CloudEvent. it's just that they are being deserialized as cloud events regardless of whether they are rawPayload = true or false.
Signed-off-by: Yevgen Polyak <[email protected]>
Signed-off-by: Yevgen Polyak <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7932 +/- ##
==========================================
+ Coverage 54.39% 54.45% +0.06%
==========================================
Files 507 507
Lines 27769 27790 +21
==========================================
+ Hits 15104 15134 +30
+ Misses 11461 11448 -13
- Partials 1204 1208 +4 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Yevgen Polyak <[email protected]>
|
@evhen14 see the failing static checks |
Signed-off-by: Yevgen Polyak <[email protected]>
| } | ||
| if _, ok := cloudEvent[contribpubsub.TraceParentField]; !ok { | ||
| if traceparent, ok := msg.Metadata[contribpubsub.TraceParentField]; ok { | ||
| cloudEvent[contribpubsub.TraceIDField] = traceparent |
There was a problem hiding this comment.
This seems to be different than the update logic above, where the TraceID populates both TraceID and TraceParent. In this block TraceParent is used for TraceID. Is this intentional?
There was a problem hiding this comment.
The traceparent override is based on this comment. I've made a change to make them consistent and adjusted the unit test
Signed-off-by: Yevgen Polyak <[email protected]>
Signed-off-by: Yevgen Polyak <[email protected]>
4 participants
Description
The change allows Pub/Sub users to tap into tracing without requiring to publish CloudEvents on the subscription side.
The change assumes that
traceparentandtracestateare the metadata fields used for this purpose. For backward compatibility, it also addstraceidfield to the internally created cloud event.Issue reference
#7918
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: