[Tracer] (Event Grid 2/5) Register outbound SDK calltargets - #8912
pablomartinezbernardo wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BenchmarksBenchmark execution time: 2026-10-02 09:29:06 Comparing candidate commit 525e44f in PR branch Found 0 performance improvements and 10 performance regressions! Performance is the same for 62 metrics, 0 unstable metrics, 75 known flaky benchmarks, 51 flaky benchmarks without significant changes.
|
6962620 to
2674581
Compare
2674581 to
ab8152d
Compare
a43a196 to
a6f6e65
Compare
ab8152d to
2f6ba44
Compare
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8912) and master. ✅ No regressions detected |
2f6ba44 to
9818224
Compare
a6f6e65 to
4a68967
Compare
9818224 to
7a0f908
Compare
4a68967 to
c67ae6c
Compare
7a0f908 to
7af02af
Compare
c67ae6c to
14823fd
Compare
vandonr
left a comment
There was a problem hiding this comment.
I wish we could unify all those instrumentation targets into something more centralized in the package, but there doesn't seem to be any good candidate
14823fd to
af3da06
Compare
7af02af to
05dc004
Compare
| [EditorBrowsable(EditorBrowsableState.Never)] | ||
| public sealed class EventGridPublisherClientSendCloudEventsAsyncIntegration | ||
| { | ||
| internal static CallTargetState OnMethodBegin<TTarget, TEvents>(TTarget instance, ref TEvents events, CancellationToken cancellationToken) |
There was a problem hiding this comment.
For the best duck-typing performance, you should add a generic constraint so we do the ducktyping upfront and we don't have to do it later on in downstream method calls. And then in the EventGridCommon.CreateProducerSpan you can do the same, where you have the generic constraint TTarget : IEventGridPublisherClient
| internal static CallTargetState OnMethodBegin<TTarget, TEvents>(TTarget instance, ref TEvents events, CancellationToken cancellationToken) | |
| internal static CallTargetState OnMethodBegin<TTarget, TEvents>(TTarget instance, ref TEvents events, CancellationToken cancellationToken) | |
| where TTarget : IEventGridPublisherClient |
There was a problem hiding this comment.
This feedback applies to all the calltarget OnMethodBegin's where you call EventGridCommon.CreateProducerSpan
| public sealed class EventGridSenderClientSendIntegration | ||
| { | ||
| internal static CallTargetState OnMethodBegin<TTarget, TCloudEvent>(TTarget instance, TCloudEvent cloudEvent, CancellationToken cancellationToken) | ||
| where TTarget : IEventGridSenderClient |
There was a problem hiding this comment.
In a similar vein to my other duck-typing comment, I recommend doing the duck-type up front since we know what type it must be. Since you'll may want to handle a "null" singular event later, you'd then also make sure that ICloudEvent implements the IDuckType interface so you can check cloudEvent.Instance is not null as the "null check"
zacharycmontoya
left a comment
There was a problem hiding this comment.
LGTM with some small suggestions for eager duck-typing
af3da06 to
fda1d6a
Compare
3a30bb0 to
c2d2c85
Compare
fda1d6a to
cd06b71
Compare
c2d2c85 to
52645d7
Compare
cd06b71 to
edb4c37
Compare
52645d7 to
525e44f
Compare
edb4c37 to
c58e89b
Compare

Summary of changes
Registers outbound CallTarget instrumentation for
Azure.Messaging.EventGridandAzure.Messaging.EventGrid.Namespaces.Reason for change
Create producer spans and inject trace context where supported when applications publish Azure Event Grid events.
Implementation details
CloudEventandEventGridEventpayloads.Test coverage
End-to-end coverage is added in the follow-up outbound E2E PR in this stack.
Other details
This PR is part of a larger Graphite stack.