CSHARP-3124: OpenTelemetry implementation #1837

adelinowona · 2025-12-09T16:57:25Z

No description provided.

sanych-sun

Partial review. Also please check failing tests

sanych-sun · 2025-12-09T21:51:13Z

src/MongoDB.Driver/MongoCollectionImpl.cs

+        {
+            // Check if all requests are of the same type
+            var firstType = requests[0].ModelType;
+            var allSameType = requests.All(r => r.ModelType == firstType);


Having this code here will lead to multiple enumerations of the requests. We had a ticket to remove the multiple enumerations. Should we move this code into the CreateBulkWriteOperation and merge it into the mapping loop?

sanych-sun · 2025-12-09T21:56:19Z

src/MongoDB.Driver/Core/Connections/CommandEventHelper.cs

+                    connectionId,
+                    $"{state.CommandName} command failed",
+                    null,
+                    reply);


So we construct the exception just to record it and never throw it? Looks suspicious.

sanych-sun · 2025-12-09T22:12:04Z

src/MongoDB.Driver/MongoDB.Driver.csproj

    <PackageReference Include="Snappier" Version="1.0.0" />
    <PackageReference Include="ZstdSharp.Port" Version="0.7.3" />
    <PackageReference Include="System.Buffers" Version="4.5.1" />
+    <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="8.0.0" />


I suppose the System.Diagnostics.DiagnosticSource version should match the target framework.

And looks like it's not needed for net6.

sanych-sun · 2025-12-09T22:16:57Z

src/MongoDB.Driver/OperationContext.cs

+            return new OperationContext(Clock, InitialTimestamp, Timeout, CancellationToken)
+            {
+                RootContext = RootContext,
+                OperationName = operationName,


We should copy values of all otel related fields in WithTimeout method as well. Otherwise they will be lost if we have to apply the timeout.

sanych-sun · 2025-12-09T22:18:52Z

src/MongoDB.Driver/OperationContext.cs

+
+        internal OperationContext WithOperationMetadata(string operationName, string databaseName, string collectionName, bool isTracingEnabled)
+        {
+            return new OperationContext(Clock, InitialTimestamp, Timeout, CancellationToken)


Providing InitialTimestamp and Timeout here looks wrong to me, because copied operationContext is suppose to "continue from the point where it was copied". We probably should provide RemainingTimeout instead, in the same way as WithTimeout doing..

sanych-sun · 2025-12-09T22:19:16Z

src/MongoDB.Driver/OperationContext.cs

            };
        }
+
+        internal OperationContext WithOperationMetadata(string operationName, string databaseName, string collectionName, bool isTracingEnabled)


Please add unit tests for the new method.

sanych-sun · 2025-12-09T23:47:41Z

src/MongoDB.Driver/ClusterKey.cs

        public bool LoadBalanced => _loadBalanced;
        public TimeSpan LocalThreshold { get { return _localThreshold; } }
        public LoggingSettings LoggingSettings { get { return _loggingSettings; } }
+        public TracingOptions TracingOptions { get { return _tracingOptions; } }


Should we add comparison of the TracingOptions to the Equals method?

BorisDog

Looks good overall, partial review.

BorisDog · 2025-12-10T18:11:51Z

src/MongoDB.Driver/MongoDB.Driver.csproj

    <PackageReference Include="Snappier" Version="1.0.0" />
    <PackageReference Include="ZstdSharp.Port" Version="0.7.3" />
    <PackageReference Include="System.Buffers" Version="4.5.1" />
+    <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="8.0.1" />


Should we include this in BSON lib instead, just in case we'd want to add serialization Otel?

BorisDog · 2025-12-10T18:12:39Z

src/MongoDB.Driver/MongoTelemetry.cs

+/// </summary>
+public static class MongoTelemetry
+{
+    private static readonly string s_driverVersion = ClientDocumentHelper.GetAssemblyVersion(typeof(MongoClient).Assembly);


__driverVersion

BorisDog · 2025-12-10T18:18:35Z

src/MongoDB.Driver/MongoTelemetry.cs

+    /// The ActivitySource used by MongoDB driver for OpenTelemetry tracing.
+    /// Applications can subscribe to this source to receive MongoDB traces.
+    /// </summary>
+    public static readonly ActivitySource ActivitySource = new("MongoDB.Driver", s_driverVersion);


Do we want/need to expose this publicly? Is it the recommended way?

BorisDog · 2025-12-11T01:29:52Z

src/MongoDB.Driver/MongoTelemetry.cs

+    {
+        var activity = ActivitySource.StartActivity(commandName, ActivityKind.Client);
+
+        if (activity == null)


minor: activity is checked for null by the callers, so maybe check in one place only?

BorisDog · 2025-12-11T01:32:27Z

src/MongoDB.Driver/MongoTelemetry.cs

+
+            SetConnectionTags(activity, connectionId);
+
+            if (command != null)


In what cases is the command null?

BorisDog · 2025-12-11T01:36:46Z

src/MongoDB.Driver/OperationExecutor.cs

+                    else
+                    {
+                        // For command-level exceptions, only set error status without recording details
+                        activity.SetStatus(ActivityStatusCode.Error);


minor: maybe this whole if/else block should move into MongoTelemetry.RecordException?

BorisDog · 2025-12-11T01:39:44Z

src/MongoDB.Driver/Core/Configuration/TracingOptions.cs

+    /// When set to true, no OpenTelemetry activities will be created for this client's operations.
+    /// Default is false (tracing enabled if configured via TracerProvider).
+    /// </summary>
+    public bool Disabled { get; set; }


Do we need this additional Disabled switch?

BorisDog · 2025-12-11T01:42:39Z

src/MongoDB.Driver/Core/Connections/CommandEventHelper.cs

-            _shouldTrackState = _shouldTrackSucceeded || _shouldTrackFailed;
+
+            // Check if tracing is disabled for this client
+            _shouldTrace = _tracingOptions?.Disabled != true;


This is probably not sufficient.
Do we still want to trace if there are not listeners?

BorisDog · 2025-12-11T01:56:42Z

src/MongoDB.Driver/OperationExecutor.cs

+                    operationContext.OperationName,
+                    operationContext.DatabaseName,
+                    operationContext.CollectionName)
+                : null;


Consider more compact overload:
MongoTelemetry.StartOperationActivity(operationContext);

And ideally (if it's reasonable of course) everything related to telemetry should be handled in single place, so should
TransactionActivityScope be under MongoTelemetry? Should MongoTelemetry.StartOperationActivity also invoke TransactionActivityScope ?

BorisDog · 2025-12-11T02:02:25Z

src/MongoDB.Driver/Core/Connections/CommandEventHelper.cs

            public CollectionNamespace QueryNamespace;
            public ExpectedResponseType ExpectedResponseType;
            public bool ShouldRedactReply;
+            public Activity CommandActivity;


Does this Activity need to be disposed?

adelinowona · 2025-12-11T20:25:54Z

src/MongoDB.Driver/MongoClient.cs

                _settings.TranslationOptions);

-        private OperationContext CreateOperationContext(IClientSessionHandle session, TimeSpan? timeout, CancellationToken cancellationToken)
+        private OperationContext CreateOperationContext(IClientSessionHandle session, TimeSpan? timeout, CancellationToken cancellationToken, string operationName = null)


btw I know Cancellation token should be the last parameter here but I did it this way for now since this work is still a draft and I didn't want to have to update every call.

Same idea stands for ExecuteReadOperation and ExecuteWriteOperation

adelinowona added the feature Adds new user-facing functionality. label Dec 9, 2025

adelinowona requested review from BorisDog, rstam and sanych-sun December 9, 2025 16:58

adelinowona added 8 commits December 9, 2025 16:33

Add OpenTelemetry specification tests

119ebac

Add core OpenTelemetry infrastructure

e863c98

Integrate tracing configuration into MongoClientSettings

fc9f61f

Implement operation-level tracing

b549899

Add tracing to collection and database operations

039aa24

Implement session and transaction tracing

a075ce1

Implement command-level tracing

d763e6a

Add test infrastructure for OpenTelemetry

f2e383d

adelinowona force-pushed the csharp3124 branch from 53e5434 to f2e383d Compare December 9, 2025 22:20

sanych-sun requested changes Dec 9, 2025

View reviewed changes

adelinowona added 3 commits December 9, 2025 23:21

Fix ClientSessionHandleTests for tests.

723a6e6

Fix CommandEventHelper tests

823deee

fix some more CommandEventHelperTests

0e674f6

BorisDog requested changes Dec 11, 2025

View reviewed changes

adelinowona commented Dec 11, 2025

View reviewed changes

fix spans not capturing in net472 and netstandard test variants

d08095c


		SetConnectionTags(activity, connectionId);

		if (command != null)

CSHARP-3124: OpenTelemetry implementation #1837

Are you sure you want to change the base?

CSHARP-3124: OpenTelemetry implementation #1837

Conversation

adelinowona commented Dec 9, 2025

Uh oh!

sanych-sun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanych-sun Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sanych-sun Dec 9, 2025 •

edited

Loading