[DSM] Expose TrackTransaction as a public manual API#8440
Merged
Conversation
…ing tracking transactions manually
kr-igor
added
area:data-streams-monitoring
type:enhancement
kr-igor
changed the title
kr-igor
changed the title
kr-igor
changed the title
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8440) 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 (8440) - mean (73ms) : 70, 76
master - mean (76ms) : 70, 81
section Bailout
This PR (8440) - mean (77ms) : 75, 79
master - mean (78ms) : 74, 82
section CallTarget+Inlining+NGEN
This PR (8440) - mean (1,085ms) : 1016, 1155
master - mean (1,083ms) : 1021, 1145
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 (8440) - mean (113ms) : 108, 119
master - mean (117ms) : 110, 123
section Bailout
This PR (8440) - mean (116ms) : 109, 123
master - mean (115ms) : 112, 119
section CallTarget+Inlining+NGEN
This PR (8440) - mean (779ms) : 756, 801
master - mean (785ms) : 751, 819
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8440) - mean (104ms) : 98, 110
master - mean (101ms) : 96, 106
section Bailout
This PR (8440) - mean (103ms) : 99, 108
master - mean (102ms) : 97, 108
section CallTarget+Inlining+NGEN
This PR (8440) - mean (939ms) : 903, 975
master - mean (940ms) : 900, 981
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8440) - mean (101ms) : 97, 105
master - mean (103ms) : 97, 109
section Bailout
This PR (8440) - mean (105ms) : 99, 111
master - mean (102ms) : 99, 104
section CallTarget+Inlining+NGEN
This PR (8440) - mean (825ms) : 779, 870
master - mean (822ms) : 788, 856
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 (8440) - mean (206ms) : 193, 219
master - mean (205ms) : 196, 213
section Bailout
This PR (8440) - mean (209ms) : 200, 219
master - mean (209ms) : 201, 217
section CallTarget+Inlining+NGEN
This PR (8440) - mean (1,217ms) : 1150, 1284
master - mean (1,218ms) : 1163, 1272
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 (8440) - mean (296ms) : 279, 313
master - mean (298ms) : 280, 316
section Bailout
This PR (8440) - mean (297ms) : 282, 311
master - mean (299ms) : 282, 316
section CallTarget+Inlining+NGEN
This PR (8440) - mean (969ms) : 939, 1000
master - mean (971ms) : 939, 1003
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8440) - mean (288ms) : 271, 305
master - mean (291ms) : 276, 306
section Bailout
This PR (8440) - mean (289ms) : 273, 306
master - mean (290ms) : 276, 304
section CallTarget+Inlining+NGEN
This PR (8440) - mean (1,166ms) : 1120, 1212
master - mean (1,166ms) : 1125, 1206
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8440) - mean (287ms) : 273, 302
master - mean (290ms) : 273, 306
section Bailout
This PR (8440) - mean (289ms) : 273, 305
master - mean (291ms) : 273, 310
section CallTarget+Inlining+NGEN
This PR (8440) - mean (1,054ms) : 996, 1111
master - mean (1,052ms) : 993, 1111
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BenchmarksBenchmark execution time: 2026-04-28 16:57:29 Comparing candidate commit 66e7785 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 56 known flaky benchmarks, 31 flaky benchmarks without significant changes.
|
| /// <param name="checkpointName">The logical name of the checkpoint (e.g. "kafka-produce", "http-send").</param> | ||
| [Instrumented] | ||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static void TrackTransaction(ISpan span, string transactionId, string checkpointName) |
Member
There was a problem hiding this comment.
Making this an extension method on ISpan might help with discoverability and make it feel more idiomatic. You only need to add this to the first parameter in the method declaration:
- public static void TrackTransaction(ISpan span, string transactionId, string checkpointName)
+ public static void TrackTransaction(this ISpan span, string transactionId, string checkpointName)Using your example in the PR description, usage would look like:
using (var scope = Tracer.Instance.StartActive("my-operation"))
{
- DataStreams.TrackTransaction(scope.Span, messageId, "kafka-produce");
+ scope.Span.TrackTransaction(messageId, "kafka-produce");
}It's just C# syntax sugar. The compiler will convert it to DataStreams.TrackTransaction(...), and users can still call it that way as well.
Member
There was a problem hiding this comment.
I'm going to suggest the alternative, do we actually need this to take an ISpan, or can we implicitly apply it to the active span only? 😬 Given the method says "the active span representing the current operation" this seems like it might be ok
My concern is that taking an ISpan in the method puts us in a tricky position in practice. Customers can pass "custom" ISpan implementations and we basically have to fail in those cases. This is a problem we have today in other public APIs, and we basically have to fail in a bunch of cases.
Suggested change
| public static void TrackTransaction(ISpan span, string transactionId, string checkpointName) | |
| public static void TrackTransaction(string transactionId, string checkpointName) |
That said, if we have to provide an API that takes a span, we could add it later as required, it's just not great for us implementation wise
Member
There was a problem hiding this comment.
Interesting! Just to clarify, you mean the method call in user code would look like this (no span) and the method should act on whatever the active span is at that moment?
DataStreams.TrackTransaction(messageId, "kafka-produce");
Contributor
Author
There was a problem hiding this comment.
I'll use the active span and simplify the iterrface.
andrewlock
requested changes
Apr 13, 2026
| /// <param name="checkpointName">The logical name of the checkpoint (e.g. "kafka-produce", "http-send").</param> | ||
| [Instrumented] | ||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| public static void TrackTransaction(ISpan span, string transactionId, string checkpointName) |
Member
There was a problem hiding this comment.
I'm going to suggest the alternative, do we actually need this to take an ISpan, or can we implicitly apply it to the active span only? 😬 Given the method says "the active span representing the current operation" this seems like it might be ok
My concern is that taking an ISpan in the method puts us in a tricky position in practice. Customers can pass "custom" ISpan implementations and we basically have to fail in those cases. This is a problem we have today in other public APIs, and we basically have to fail in a bunch of cases.
Suggested change
| public static void TrackTransaction(ISpan span, string transactionId, string checkpointName) | |
| public static void TrackTransaction(string transactionId, string checkpointName) |
That said, if we have to provide an API that takes a span, we could add it later as required, it's just not great for us implementation wise
| MethodName = "TrackTransaction", | ||
| ReturnTypeName = ClrNames.Void, | ||
| ParameterTypeNames = ["Datadog.Trace.ISpan", ClrNames.String, ClrNames.String], | ||
| MinimumVersion = ManualInstrumentationConstants.MinVersion, |
Member
There was a problem hiding this comment.
The method doesn't exist until this version, so set the minimum to the current version
Suggested change
| MinimumVersion = ManualInstrumentationConstants.MinVersion, | |
| MinimumVersion = "3.42.0", |
Comment on lines
+39
to
+50
| if (span is IDuckType { Instance: Span s }) | ||
| { | ||
| s.TrackTransaction(manager, transactionId, checkpointName); | ||
| } | ||
| else if (span is Span autoSpan) | ||
| { | ||
| autoSpan.TrackTransaction(manager, transactionId, checkpointName); | ||
| } | ||
| else | ||
| { | ||
| manager.TrackTransaction(transactionId, checkpointName); | ||
| } |
Member
There was a problem hiding this comment.
If we ditch the span from the public API we get:
Suggested change
| if (span is IDuckType { Instance: Span s }) | |
| { | |
| s.TrackTransaction(manager, transactionId, checkpointName); | |
| } | |
| else if (span is Span autoSpan) | |
| { | |
| autoSpan.TrackTransaction(manager, transactionId, checkpointName); | |
| } | |
| else | |
| { | |
| manager.TrackTransaction(transactionId, checkpointName); | |
| } | |
| if (Tracer.Instance.InternalActiveScope?.Span is Span span) | |
| { | |
| span.TrackTransaction(manager, transactionId, checkpointName); | |
| } | |
| else | |
| { | |
| manager.TrackTransaction(transactionId, checkpointName); | |
| } |
| public void HappyPath_DuckTypedRealSpan_SetsTagAndTracksTransaction() | ||
| { | ||
| var dsm = CreateDsmWithWriter(out var writer); | ||
| var manager = CreateTracerManager(dsm); |
Member
There was a problem hiding this comment.
This is problematic - it creates a TracerManager that runs for the life of the test application and is never shut down. A better approach is to use the TracerHelpers.CreateScoped helper we have, and use it in a using declaration so that it's disposed.
I would personally also strongly prefer extracting a method that doesn't use the static Tracer.Instance method calls inside the integration, and instead passes in the "local" tracer instance instead, if that makes sense? (I can demonstrate if not!)
| await Task.Delay(millisecondsDelay: 100); | ||
|
|
||
| Console.WriteLine("Sending one message to the queue..."); | ||
| Datadog.Trace.DataStreams.TrackTransaction(scope.Span, "my-transaction-id", "send-checkpoint"); |
Member
There was a problem hiding this comment.
This doesn't actually seem to have an impact on the integration tests, which seems like a problem? 🤔 Essentially it means it's a test without asserts 😄
Contributor
Author
There was a problem hiding this comment.
It's verified in DataStreamsMonitoringManualApiTest -> TrackTransaction
andrewlock
reviewed
Apr 22, 2026
Member
andrewlock
left a comment
andrewlock
left a comment
There was a problem hiding this comment.
LGTM in principle, but I'd strongly suggest handling the null values scenario, ideally by splitting the public API and instrumentation target
| } | ||
| public static class DataStreams | ||
| { | ||
| public static void TrackTransaction(Datadog.Trace.ISpan span, string transactionId, string checkpointName) { } |
Member
There was a problem hiding this comment.
This API changed:
Suggested change
| public static void TrackTransaction(Datadog.Trace.ISpan span, string transactionId, string checkpointName) { } | |
| public static void TrackTransaction(string transactionId, string checkpointName) { } |
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
3 participants
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
Datadog.Trace.DataStreams.TrackTransaction(ISpan, string, string)as a public API inDatadog.Trace.Manual, allowing customers to record Data Streams Monitoring transaction checkpoints directly from their code.Reason for change
DSM transaction tracking doesn't work in all scenarios due to limited set of extractors (e.g. custom messaging layers, proprietary transports, or patterns where message IDs are only available in customer code). This API gives customers a manual escape hatch to record transactions when there's no extractor.
Implementation details
Follows the existing
[Instrumented]/ CallTarget shim pattern used byEventTrackingSdkandSpanExtensions:Datadog.Trace.Manual/DataStreams.cs— New public stub with an empty body, decorated with[Instrumented]and[MethodImpl(MethodImplOptions.NoInlining)]. The stub is the API surface customers reference from theDatadog.TraceNuGet package.ClrProfiler/AutoInstrumentation/ManualInstrumentation/DataStreams/DataStreamsTrackTransactionIntegration.cs—[InstrumentMethod]integration that intercepts the stub at runtime.OnMethodBeginresolves the caller'sISpanto an internalSpanvia duck typing, then:span.TrackTransaction(manager, transactionId, checkpointName)— sets thedsm.transaction.idtag on the span and sends the transaction to the DSM backend.ISpanimplementation): callsmanager.TrackTransaction(transactionId, checkpointName)directly — sends to the backend without tagging.PublicApiUsage.DataStreams_TrackTransactiontelemetry in both paths.Telemetry/Metrics/PublicApiUsage.cs— Added theDataStreams_TrackTransactionenum entry.Generated files —
InstrumentationDefinitions.g.csandPublicApiUsageExtensions_EnumExtensions.g.csregenerated automatically by Roslyn source generators during build.Behaviour when DSM is disabled:
IsTransactionTrackingEnabled(!_isInDefaultState && IsEnabled) guards both call paths, so the method is a silent no-op unlessDD_DATA_STREAMS_ENABLED=trueis explicitly set.Test coverage
Unit tests (
DataStreamsInstrumentationTests.cs):dsm.transaction.idtag is set on the internalSpanandIDataStreamsWriter.AddTransactionis called exactly once.ISpanimplementation — verifiesSetTagis never called on the mock span butAddTransactionis still called exactly once.Structural test (existing, automatic) —
DatadogTraceManualInstrumentationTestsscans all[InstrumentMethod]integrations and asserts every targeted Manual method has[Instrumented]+[MethodImpl(MethodImplOptions.NoInlining)], and vice versa. Passes without changes.Integration test (
DataStreamsMonitoringManualApiTest.TrackTransaction):Samples.DataStreams.ManualAPI(extended to callDataStreams.TrackTransaction) with the real CLR profiler attached.dsm.transaction.idspan tag is set to the expected value.MockDataStreamsBucket.TransactionsandTransactionCheckpointIds, verifying both the transaction ID and checkpoint name.Other details
DataStreamsis a newstatic classin theDatadog.Tracenamespace.DD_DATA_STREAMS_ENABLED=true; silently no-ops otherwise (same guard as the existing internalDataStreamsManager.TrackTransaction).Datadog.Trace.Manualproject does not referenceDatadog.Tracedirectly, so the stub body remains empty by design — the profiler provides the implementation at runtime.