Open
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4427a70e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// </summary> | ||
| /// <param name="transactionId">A stable identifier for the transaction being tracked (e.g. a message ID or trace ID).</param> | ||
| /// <param name="checkpointName">The logical name of the checkpoint (e.g. "kafka-produce", "http-send").</param> | ||
| public static void TrackTransaction(string transactionId, string checkpointName) |
There was a problem hiding this comment.
Deleting Datadog.Trace.DataStreams.TrackTransaction removes a public method from the Datadog.Trace.Manual package, so any customer code that adopted this manual DSM API will stop compiling after upgrading the NuGet package. The companion CallTarget was also removed, which means apps pinned to an older manual assembly would lose the transaction-tracking behavior when run with a newer tracer home; keep the API/instrumentation or obsolete it in a compatible release first.
Useful? React with 👍 / 👎.
Comment on lines
+35
to
+36
| var extractedContext = ContextPropagation.ExtractHeadersFromMessage(tracer, messageAttributesProxy); | ||
| parentContext = extractedContext.SpanContext; |
There was a problem hiding this comment.
When the same PublishRequest instance is published more than once, the _datadog message attribute written by the previous call is still present when this extraction runs, and CreateScope(..., parentContext) then overrides the current active request span with that stale context. That makes the second SNS span/message a child of the previous publish trace; only extract from message attributes when there is no active scope, or clear/overwrite the Datadog attribute before using it as a parent.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
the tracer.ActiveScope is null in the if statement checks to prefer thee active scope
chojomok
force-pushed
the
mohammad/mass_transit7
branch
from
f4427a7 to
5027224
Compare
Split from mohammad/mass_transit_intg. Contains shared MassTransit integration code (MassTransitCommon, duck types, diagnostic observer, generated files, build/sln updates) plus MassTransit 7-specific sample app, tests, and snapshots. MT8-specific files excluded.
chojomok
force-pushed
the
mohammad/mass_transit7
branch
from
5027224 to
51efcf3
Compare
When the same PublishRequest instance is reused for multiple Publish calls, the _datadog message attribute from the prior publish lingers on the request. Extracting it as a parent context made the new SNS span a child of the previous publish trace. Mirrors the RabbitMQ producer fix: only extract from message attributes when there is no active scope.
The ActivitySource-based handler is only used by MassTransit 8. MT7 instruments via DiagnosticSource (MassTransitDiagnosticObserver) and does not need this handler.
BenchmarksBenchmark execution time: 2026-04-29 22:25:42 Comparing candidate commit 2a335b0 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 26 metrics, 0 unstable metrics, 61 known flaky benchmarks, 26 flaky benchmarks without significant changes.
|
chojomok
commented
Apr 29, 2026
| @@ -0,0 +1,175 @@ | |||
| # MassTransit Integration | |||
Collaborator
Author
There was a problem hiding this comment.
going to delete this before merging - just hear for my own notes
The Windows and non-Windows snapshots were byte-identical, so use a single InMemory snapshot for both platforms and drop the now-unused IsWindows helper.
Move GetActivityKind, SetActivityKind, CreateResourceName, and EnhanceActivityMetadata to the MassTransit 8 PR — they are only called by MassTransitActivityHandler (the ActivitySource-based v8 handler), not by the v7 DiagnosticSource observer. ExtractTraceIdFromActivity had no callers on either branch and is deleted outright.
…dotnet into mohammad/mass_transit7
Drop trailing blank line before class closing brace left over from the method removal.
chojomok
commented
Apr 29, 2026
| public IEnumerable<string> Get(IDictionary<string, object> carrier, string key) | ||
| { | ||
| if (carrier.TryGetValue(key, out var value) && value is byte[] bytes) | ||
| if (carrier.TryGetValue(key, out var value)) |
Collaborator
Author
There was a problem hiding this comment.
adding support for context propagation with MassTransit
chojomok
commented
Apr 29, 2026
| /// <summary> | ||
| /// Extracts trace context from message attributes. | ||
| /// </summary> | ||
| public static PropagationContext ExtractHeadersFromMessage(Tracer tracer, IContainsMessageAttributes? carrier) |
Collaborator
Author
There was a problem hiding this comment.
a lot going on here - need to determine if it's needed
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8538) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (75ms) : 70, 80
master - mean (73ms) : 71, 76
section Bailout
This PR (8538) - mean (78ms) : 74, 82
master - mean (80ms) : 76, 84
section CallTarget+Inlining+NGEN
This PR (8538) - mean (1,089ms) : 1013, 1164
master - mean (1,076ms) : 1020, 1132
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (117ms) : 110, 123
master - mean (115ms) : 109, 121
section Bailout
This PR (8538) - mean (115ms) : 112, 118
master - mean (115ms) : 112, 117
section CallTarget+Inlining+NGEN
This PR (8538) - mean (784ms) : 757, 810
master - mean (779ms) : 750, 808
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (103ms) : 97, 110
master - mean (102ms) : 98, 107
section Bailout
This PR (8538) - mean (106ms) : 99, 112
master - mean (101ms) : 99, 104
section CallTarget+Inlining+NGEN
This PR (8538) - mean (939ms) : 906, 972
master - mean (940ms) : 894, 985
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (100ms) : 97, 103
master - mean (100ms) : 97, 104
section Bailout
This PR (8538) - mean (103ms) : 97, 110
master - mean (105ms) : 100, 109
section CallTarget+Inlining+NGEN
This PR (8538) - mean (823ms) : 787, 860
master - mean (822ms) : 786, 859
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (202ms) : 196, 208
master - mean (207ms) : 197, 216
section Bailout
This PR (8538) - mean (206ms) : 199, 214
master - mean (211ms) : 201, 220
section CallTarget+Inlining+NGEN
This PR (8538) - mean (1,195ms) : 1138, 1252
master - mean (1,218ms) : 1170, 1267
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (289ms) : 276, 302
master - mean (296ms) : 278, 314
section Bailout
This PR (8538) - mean (289ms) : 280, 297
master - mean (297ms) : 280, 313
section CallTarget+Inlining+NGEN
This PR (8538) - mean (950ms) : 918, 982
master - mean (976ms) : 946, 1007
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (283ms) : 270, 297
master - mean (294ms) : 274, 314
section Bailout
This PR (8538) - mean (283ms) : 271, 294
master - mean (292ms) : 272, 311
section CallTarget+Inlining+NGEN
This PR (8538) - mean (1,153ms) : 1114, 1192
master - mean (1,163ms) : 1111, 1215
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8538) - mean (282ms) : 269, 295
master - mean (285ms) : 274, 297
section Bailout
This PR (8538) - mean (283ms) : 268, 297
master - mean (286ms) : 275, 297
section CallTarget+Inlining+NGEN
This PR (8538) - mean (1,046ms) : 991, 1101
master - mean (1,044ms) : 993, 1094
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Adds auto-instrumentation for MassTransit 7. Produces
masstransit.send,masstransit.publish,masstransit.receive,masstransit.consume,masstransit.handle, andmasstransit.sagaspans, with context propagation across the in-memory, RabbitMQ, AWS SQS/SNS, and Azure Service Bus transports.This is the first half of a two-part split: MassTransit 8 support is added on top in the stacked follow-up PR. See #8055 for the original combined branch.
Reason for change
Customer-requested support for the MassTransit messaging library. MassTransit is one of the most widely used messaging abstractions in the .NET ecosystem and the only major broker-agnostic library still missing first-class APM coverage.
Implementation details
MassTransit 7 emits
DiagnosticSourceevents (it predatesActivitySource), so instrumentation hangs off a custom observer plus a few targeted CallTarget hooks for things diagnostic events don't cover.tracer/src/Datadog.Trace/DiagnosticListeners/MassTransitDiagnosticObserver.cs— listens forMassTransit.Transport.Send,MassTransit.Transport.Receive,MassTransit.Consumer.Consume,MassTransit.Consumer.Handle, andMassTransit.Saga.RaiseEventevents and creates Datadog spans.tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/MassTransit/— shared integration code:MassTransitCommon.cs— span creation, tagging, context propagation utilities.ContextPropagation.cs+SendHeadersInjectAdapter.cs— header injection across transports.DuckTypes/— version-agnostic access toSendContext,ConsumeContext,ReceiveContext, and headers.CallTarget/NotifyFaultedIntegration.cs— captures exceptions fromBaseReceiveContext.NotifyFaulted(diagnostic events don't carry the exception).CallTarget/InMemoryTransportMessageIntegration.cs— restores trace context across the in-memory transport's thread boundary.tracer/src/Datadog.Trace/Activity/Handlers/MassTransitActivityHandler.cs— placeholder handler kept here for the MT8 PR to consume. (No-op on this branch; included so the MT8 stack is purely additive.)IntegrationId.MassTransitand supportingMassTransitTags.A detailed architecture overview, including known limitations and flake notes, lives in
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/MassTransit-Integration.md.Test coverage
tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/MassTransit7Tests.cs— integration tests covering in-memory, RabbitMQ, and AWS SQS transports, plus a Windows-only in-memory variant.MassTransit7TestsInMemory.verified.txt,MassTransit7TestsInMemoryWindows.verified.txt,MassTransit7TestsRabbitMq.verified.txt,MassTransit7TestsSqs.verified.txt.tracer/test/test-applications/integrations/Samples.MassTransit7/exercises the producer / consumer / saga / fault paths against MassTransit 7.3.1.Samples.MassTransit.Shared/(consumers, contracts, fault observer, deterministic test signal) — reused by the MT8 PR.PackageVersionsGeneratorDefinitions.jsonentry forMassTransit7.Other details