Exclude Otel for MongoDb 3.7#8279
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5a821b458
ℹ️ 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".
BenchmarksBenchmark execution time: 2026-03-09 12:26:21 Comparing candidate commit d5a821b in PR branch Found 2 performance improvements and 9 performance regressions! Performance is the same for 163 metrics, 18 unstable metrics. scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody netcoreapp3.1
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody netcoreapp3.1
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch net472
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearchAsync net472
scenario:Benchmarks.Trace.NLogBenchmark.EnrichedLog net6.0
scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore net6.0
scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore netcoreapp3.1
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan net6.0
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan netcoreapp3.1
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8279) 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 (8279) - mean (75ms) : 73, 77
master - mean (75ms) : 72, 77
section Bailout
This PR (8279) - mean (79ms) : 77, 81
master - mean (79ms) : 77, 81
section CallTarget+Inlining+NGEN
This PR (8279) - mean (1,081ms) : 1037, 1126
master - mean (1,078ms) : 1035, 1121
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 (8279) - mean (117ms) : 108, 126
master - mean (115ms) : 111, 119
section Bailout
This PR (8279) - mean (117ms) : 114, 119
master - mean (116ms) : 114, 119
section CallTarget+Inlining+NGEN
This PR (8279) - mean (779ms) : 712, 847
master - mean (770ms) : 712, 827
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8279) - mean (103ms) : 99, 106
master - mean (103ms) : 100, 107
section Bailout
This PR (8279) - mean (104ms) : 102, 106
master - mean (104ms) : 102, 107
section CallTarget+Inlining+NGEN
This PR (8279) - mean (757ms) : 689, 826
master - mean (748ms) : 680, 817
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8279) - mean (102ms) : 99, 106
master - mean (101ms) : 98, 105
section Bailout
This PR (8279) - mean (104ms) : 102, 106
master - mean (103ms) : 100, 105
section CallTarget+Inlining+NGEN
This PR (8279) - mean (685ms) : 652, 717
master - mean (670ms) : 651, 689
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 (8279) - mean (195ms) : 191, 200
master - mean (196ms) : 191, 200
section Bailout
This PR (8279) - mean (199ms) : 196, 201
master - mean (199ms) : 196, 203
section CallTarget+Inlining+NGEN
This PR (8279) - mean (1,156ms) : 1094, 1218
master - mean (1,154ms) : 1105, 1202
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 (8279) - mean (279ms) : 273, 286
master - mean (279ms) : 273, 285
section Bailout
This PR (8279) - mean (281ms) : 276, 285
master - mean (281ms) : 273, 289
section CallTarget+Inlining+NGEN
This PR (8279) - mean (947ms) : 906, 987
master - mean (946ms) : 907, 986
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8279) - mean (273ms) : 267, 279
master - mean (272ms) : 266, 277
section Bailout
This PR (8279) - mean (272ms) : 269, 276
master - mean (271ms) : 265, 277
section CallTarget+Inlining+NGEN
This PR (8279) - mean (934ms) : 907, 961
master - mean (935ms) : 911, 959
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8279) - mean (271ms) : 267, 275
master - mean (270ms) : 266, 275
section Bailout
This PR (8279) - mean (272ms) : 268, 277
master - mean (271ms) : 266, 275
section CallTarget+Inlining+NGEN
This PR (8279) - mean (835ms) : 815, 854
master - mean (835ms) : 811, 859
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bouwkast
left a comment
There was a problem hiding this comment.
Thanks
I do wonder if this is definitely the approach we should be taking, but let's take that offline
➕
In case anyone hits this and gets duplicated spans prior to this being released we have DD_TRACE_DISABLED_ACTIVITY_SOURCES which is configurable, but this Ignore one isn't. However they also work in slightly different ways and have some behavioral differences
DD_TRACE_DISABLED_ACTIVITY_SOURCES=MongoDB.Driver
Summary of changes
Adds MongoDB.Driver to the Activity ignore handler, to avoid duplicate instrumentations
Reason for change
MongoDB .NET driver v3.7.0 adds support for OpenTelemetry, but this results in duplicate instrumentations for our MongoDb instrumentation. You can see this in play in the test-package-version PR here
An AI analysis (shown in full below), has the following summary:
Cannot replace custom instrumentation with OTel spans. Key blockers:
mongodb.query) — This is the most valuable tag for debugging and is completely absent from OTel spanshttpinstead ofmongodb) — Would break DB categorization in Datadog UIclient.requestinstead ofmongodb.query) — Loses MongoDB-specific identificationThere's a bunch of other tag differences, some of which may or may-not be a problem, but the fact they don't tag the query body likely precludes us deferring to the otel approach, as it would be crucial lacking information.
AI analysis comparing custom instrumentation to OTel
MongoDB 3.7.0 OTel Instrumentation Analysis
Context
MongoDB.Driver 3.7.0 adds built-in OpenTelemetry instrumentation. When used with our existing dd-trace-dotnet custom instrumentation, this creates duplicate/overlapping spans. We need to determine whether the OTel spans can replace our custom instrumentation, or whether we should disable them.
This analysis is based on the diff at
mongodb.diff, which compares the SchemaV0 snapshot for the3_0package version (before) against the new output with MongoDB 3.7.0 (after).Span Count Change
mongodb.queryspansThe span count more than doubles: +30 new OTel spans (15 L1 + 15 L2).
Note: the "before" count has 16 mongodb.query spans vs 15 in "after". The initial
findthat was directly under Main() (Id_2) now has an OTel L1 parent (Id_6), and there's one fewer mongodb.query span for thecountDocumentsoperations - actually looking more carefully, the count is the same (15 DD spans each for the 3 groups of sync/async x 5 operations + 1 initial find = 16... let me recount). Actually both before and after have 15mongodb.queryspans - same count. The diff just renumbers IDs.New Span Hierarchy
Before (2-tier):
After (4-tier):
Our
mongodb.queryspans are now sandwiched between two OTel spans. The OTel L1 span becomes the parent of our span, and our span becomes the parent of the OTel L2 span.Two Levels of OTel Spans
OTel Level 1 — Logical Operation Spans
client.request<operation> <db>.<collection>(e.g.,find test-db.employees)Samples.MongoDB(app service name, NOT separate)http(incorrect for a DB span!)clientTags:
db.collection.name: e.g.,employeesdb.namespace: e.g.,test-dbdb.operation.name: e.g.,find,delete,insert,countDocumentsdb.operation.summary: e.g.,find test-db.employeesdb.system.name:mongodbotel.library.name:MongoDB.Driverotel.library.version:3.7.0otel.status_code:STATUS_CODE_OKNo metrics. No host/port info at this level.
OTel Level 2 — Wire Protocol Command Spans
client.request<command>(e.g.,find,delete,aggregate) — just the command, no db nameSamples.MongoDB(app service name)http(incorrect for a DB span!)clientTags:
db.collection.name: e.g.,employeesdb.command.name: e.g.,find,delete,insert,aggregatedb.mongodb.lsid: Session ID (BSON)db.namespace: e.g.,test-dbdb.query.summary: e.g.,find test-db.employeesdb.system.name:mongodbnetwork.transport:tcpserver.address: e.g.,mongootel.library.name:MongoDB.Driverotel.library.version:3.7.0otel.status_code:STATUS_CODE_OKMetrics:
db.mongodb.driver_connection_id: e.g.,3.0db.mongodb.server_connection_id: e.g.,7.0server.port: e.g.,27017.0Tag-by-Tag Comparison: OTel Spans vs DD Custom Spans
DD Custom
mongodb.queryspan tags:componentMongoDbdb.nametest-dbdb.namespace=test-dbdb.namespace=test-dbmongodb.collectionemployeesdb.collection.name=employeesdb.collection.name=employeesmongodb.queryout.hostmongoserver.address=mongoout.port27017server.port=27017.0(metric)span.kindclientclientclient_dd.base_serviceSamples.MongoDBDD Custom span properties:
mongodb.queryclient.requestclient.request<op> <db>(e.g.,find test-db)<op> <db>.<coll>(e.g.,find test-db.employees)<command>(e.g.,find)Samples.MongoDB-mongodb(SchemaV0)Samples.MongoDBSamples.MongoDBmongodbhttphttpTags present in OTel but NOT in DD custom spans:
db.operation.name/db.command.namedb.operation.summary/db.query.summarydb.system.namedb.mongodb.lsid(session ID - L2 only)network.transport(L2 only)server.address(L2 only — similar toout.host)otel.library.name,otel.library.versionotel.status_codeMetrics present in OTel but NOT in DD custom spans:
db.mongodb.driver_connection_id(L2 only)db.mongodb.server_connection_id(L2 only)server.port(L2 only — similar toout.port)Operation Name Mapping:
countDocumentsvsaggregateNotable: The OTel L1 span uses the logical operation name
countDocuments, while our DD custom span (and OTel L2) use the wire protocol commandaggregate. This is because MongoDB implementscountDocuments()as an aggregate pipeline internally. The OTel L1 span provides the more user-friendly logical name.Critical Gaps if Replacing DD Custom with OTel
1.
mongodb.querytag — MISSING from OTelThe full BSON query body is the most significant tag in our custom instrumentation. Neither OTel level provides this. The OTel spans only include
db.query.summary(e.g.,find test-db.employees) which is just the operation + namespace, not the actual query filter/pipeline.2. Span Type —
httpinstead ofmongodbBoth OTel span levels use Type
http, while our custom spans correctly use Typemongodb. This affects categorization in the Datadog UI (DB queries vs HTTP calls).3. Span Name —
client.requestinstead ofmongodb.queryGeneric OTel name vs our specific operation name.
4. Service Name — No separate service (SchemaV0)
In SchemaV0, our custom spans use
Samples.MongoDB-mongodb(separate service), while OTel spans use the app service nameSamples.MongoDB. This means no dedicated MongoDB service in the service map.5. Resource Name — Different format
find test-db(operation + database)find test-db.employees(operation + db.collection — more specific, arguably better)find(just the command — less useful)6.
componenttag — MISSING from OTelOur DD spans set
component: MongoDb. OTel spans don't have this.What OTel Does Better
find test-db.employeesis more informative thanfind test-dbcountDocumentsinstead ofaggregate(L1 only)driver_connection_id,server_connection_id,network.transportdb.mongodb.lsiddb.namespace,db.system.name, etc.)Summary / Recommendation
Cannot replace custom instrumentation with OTel spans. Key blockers:
mongodb.query) — This is the most valuable tag for debugging and is completely absent from OTel spanshttpinstead ofmongodb) — Would break DB categorization in Datadog UIclient.requestinstead ofmongodb.query) — Loses MongoDB-specific identificationRecommended approach: Disable the OTel instrumentation for MongoDB 3.7.0+ to maintain the existing behavior. This likely means either:
If keeping both is desired for any reason, the OTel spans should at minimum be filtered out from the test snapshots, and ideally suppressed at runtime to avoid the 2.5x span multiplication.
Implementation details
Add
MongoDB.Driverto the activity ignore list.Test coverage
Bumped the tests to run with 3.7.0, so should be covered
Other details
I do wonder if this is definitely the approach we should be taking, but let's take that offline