-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[EventGrid] Distributed Tracing for EventGrid #11562
[EventGrid] Distributed Tracing for EventGrid #11562
Conversation
@@ -0,0 +1,75 @@ | |||
// Copyright (c) Microsoft Corporation. |
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.
Would love you feedback on this approach, Jeff. We want the values we put in the event to be the same as what we put in the HTTP headers, and this seemed like the best way to do this.
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.
I think this is probably the best approach we can do for right now, though the more policies we add that want to fiddle with objects post-mapping but pre-serialization is making me think I should prioritize decoupling the two steps sooner rather than later.
sdk/eventgrid/eventgrid/src/cloudEventDistrubtedTracingEnricherPolicy.ts
Outdated
Show resolved
Hide resolved
sdk/eventgrid/eventgrid/src/cloudEventDistrubtedTracingEnricherPolicy.ts
Outdated
Show resolved
Hide resolved
sdk/eventgrid/eventgrid/src/cloudEventDistrubtedTracingEnricherPolicy.ts
Outdated
Show resolved
Hide resolved
sdk/eventgrid/eventgrid/src/cloudEventDistrubtedTracingEnricherPolicy.ts
Outdated
Show resolved
Hide resolved
db220b8
to
f413273
Compare
@xirzec Sorry for the delay in responding to your feedback. Could you please take another look? |
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.
Looks good!
- Create spans for each of the send operations on `EventGridPublisherClient` - When sending events in the Cloud Events schema, if we have a traceparent or tracestate and an event we are sending does not have any distributed tracing metadata (per the Cloud Events distributed tracing spec) we add this metadata associating the events with the span representing the HTTP operation that sends the event to the Azure Event Grid Service. Fixes: Azure#11056
f413273
to
1ffd3aa
Compare
Belatedly this also looks good to me! Good job! |
Create spans for each of the send operations on
EventGridPublisherClient
When sending events in the Cloud Events schema, if we have a
traceparent or tracestate and an event we are sending does not have
any distributed tracing metadata (per the Cloud Events distributed
tracing spec) we add this metadata associating the events with the
span representing the HTTP operation that sends the event to the Azure
Event Grid Service.
Fixes: #11056