Skip to content

Commit 743eaa8

Browse files
committed
Summary
I've successfully refactored the code to fix the bug where activity listeners were being checked when the connection was created (at which point none may be set), but not checked again when the connection was pulled from the pool. Key Changes: Constructor (CommandEventHelper.cs:60): Removed the listener check from _shouldTrackState initialization. Now _shouldTrackState only depends on whether event tracking is needed (_shouldTrackSucceeded || _shouldTrackFailed), not on tracing with listeners. Property Getters (CommandEventHelper.cs:71-94): Updated all property getters (ShouldCallBeforeSending, ShouldCallAfterSending, etc.) to dynamically check MongoTelemetry.ActivitySource.HasListeners() at execution time, ensuring that activity tracing is detected when connections are obtained from the pool. Lazy State Initialization (CommandEventHelper.cs:37, 741-745): Changed _state from readonly to allow lazy initialization. The state dictionary is now only created when needed - either for event tracking or when tracing with listeners is detected at runtime. Null Safety (CommandEventHelper.cs:132, 165, 192, 216, 233): Added null checks before accessing _state in all methods to handle cases where state tracking hasn't been initialized. TrackCommandState (CommandEventHelper.cs:734-770): Updated to check for listeners at execution time and lazily initialize the state dictionary only when tracing is actually needed. Test Update (CommandEventHelperTests.cs:122-124): Updated the test to reflect the new behavior where _shouldTrackState only reflects event tracking, not tracing with listeners (which is now checked dynamically). Benefits: Bug Fixed: Activity listeners are now checked when connections are obtained from the pool, not just when they're created No Unnecessary State Tracking: State tracking is only enabled when actually needed (either for events or when listeners are registered), avoiding the performance overhead of always tracking state Backward Compatible: The behavior is the same for all scenarios, just more efficient The changes ensure that the driver properly detects activity listeners whenever a connection is used, whether it's newly created or retrieved from the pool, while avoiding unnecessary state tracking when no listeners are registered.
1 parent bb33139 commit 743eaa8

File tree

3 files changed

+58
-39
lines changed

3 files changed

+58
-39
lines changed

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

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace MongoDB.Driver.Core.Connections
3434
internal class CommandEventHelper
3535
{
3636
private readonly EventLogger<LogCategories.Command> _eventLogger;
37-
private readonly ConcurrentDictionary<int, CommandState> _state;
37+
private ConcurrentDictionary<int, CommandState> _state;
3838

3939
private readonly bool _shouldProcessRequestMessages;
4040
private readonly bool _shouldTrackStarted;
@@ -57,7 +57,7 @@ public CommandEventHelper(EventLogger<LogCategories.Command> eventLogger, Tracin
5757
_tracingDisabled = tracingOptions?.Disabled == true;
5858
_queryTextMaxLength = tracingOptions?.QueryTextMaxLength ?? 0;
5959

60-
_shouldTrackState = _shouldTrackSucceeded || _shouldTrackFailed || !_tracingDisabled;
60+
_shouldTrackState = _shouldTrackSucceeded || _shouldTrackFailed;
6161
_shouldProcessRequestMessages = _shouldTrackStarted || _shouldTrackState;
6262

6363
if (_shouldTrackState)
@@ -68,30 +68,18 @@ public CommandEventHelper(EventLogger<LogCategories.Command> eventLogger, Tracin
6868
}
6969
}
7070

71-
public bool ShouldCallBeforeSending
72-
{
73-
get { return _shouldProcessRequestMessages; }
74-
}
71+
public bool ShouldCallBeforeSending => _shouldProcessRequestMessages || ShouldTraceWithActivityListener();
7572

76-
public bool ShouldCallAfterSending
77-
{
78-
get { return _shouldTrackState; }
79-
}
73+
public bool ShouldCallAfterSending => _shouldTrackState || ShouldTraceWithActivityListener();
8074

81-
public bool ShouldCallErrorSending
82-
{
83-
get { return _shouldTrackState; }
84-
}
75+
public bool ShouldCallErrorSending => _shouldTrackState || ShouldTraceWithActivityListener();
8576

86-
public bool ShouldCallAfterReceiving
87-
{
88-
get { return _shouldTrackState; }
89-
}
77+
public bool ShouldCallAfterReceiving => _shouldTrackState || ShouldTraceWithActivityListener();
9078

91-
public bool ShouldCallErrorReceiving
92-
{
93-
get { return _shouldTrackState; }
94-
}
79+
public bool ShouldCallErrorReceiving => _shouldTrackState || ShouldTraceWithActivityListener();
80+
81+
private bool ShouldTraceWithActivityListener()
82+
=> !_tracingDisabled && MongoTelemetry.ActivitySource.HasListeners();
9583

9684
public void CompleteFailedCommandActivity(Exception exception)
9785
{
@@ -129,8 +117,12 @@ public void BeforeSending(
129117

130118
public void AfterSending(RequestMessage message, ConnectionId connectionId, ObjectId? serviceId, bool skipLogging)
131119
{
132-
CommandState state;
133-
if (_state.TryGetValue(message.RequestId, out state) &&
120+
if (_state == null)
121+
{
122+
return;
123+
}
124+
125+
if (_state.TryGetValue(message.RequestId, out var state) &&
134126
state.ExpectedResponseType == ExpectedResponseType.None)
135127
{
136128
state.Stopwatch.Stop();
@@ -157,12 +149,16 @@ public void AfterSending(RequestMessage message, ConnectionId connectionId, Obje
157149

158150
public void ErrorSending(RequestMessage message, ConnectionId connectionId, ObjectId? serviceId, Exception exception, bool skipLogging)
159151
{
160-
CommandState state;
161-
if (_state.TryRemove(message.RequestId, out state))
152+
CompleteCommandActivityWithException(exception);
153+
154+
if (_state == null)
162155
{
163-
state.Stopwatch.Stop();
156+
return;
157+
}
164158

165-
CompleteCommandActivityWithException(exception);
159+
if (_state.TryRemove(message.RequestId, out var state))
160+
{
161+
state.Stopwatch.Stop();
166162

167163
_eventLogger.LogAndPublish(new CommandFailedEvent(
168164
state.CommandName,
@@ -179,8 +175,12 @@ public void ErrorSending(RequestMessage message, ConnectionId connectionId, Obje
179175

180176
public void AfterReceiving(ResponseMessage message, IByteBuffer buffer, ConnectionId connectionId, ObjectId? serviceId, MessageEncoderSettings encoderSettings, bool skipLogging)
181177
{
182-
CommandState state;
183-
if (!_state.TryRemove(message.ResponseTo, out state))
178+
if (_state == null)
179+
{
180+
return;
181+
}
182+
183+
if (!_state.TryRemove(message.ResponseTo, out var state))
184184
{
185185
// this indicates a bug in the sending portion...
186186
return;
@@ -198,17 +198,21 @@ public void AfterReceiving(ResponseMessage message, IByteBuffer buffer, Connecti
198198

199199
public void ErrorReceiving(int responseTo, ConnectionId connectionId, ObjectId? serviceId, Exception exception, bool skipLogging)
200200
{
201-
CommandState state;
202-
if (!_state.TryRemove(responseTo, out state))
201+
CompleteCommandActivityWithException(exception);
202+
203+
if (_state == null)
204+
{
205+
return;
206+
}
207+
208+
if (!_state.TryRemove(responseTo, out var state))
203209
{
204210
// this indicates a bug in the sending portion...
205211
return;
206212
}
207213

208214
state.Stopwatch.Stop();
209215

210-
CompleteCommandActivityWithException(exception);
211-
212216
_eventLogger.LogAndPublish(new CommandFailedEvent(
213217
state.CommandName,
214218
state.QueryNamespace.DatabaseNamespace,
@@ -223,13 +227,18 @@ public void ErrorReceiving(int responseTo, ConnectionId connectionId, ObjectId?
223227

224228
public void ConnectionFailed(ConnectionId connectionId, ObjectId? serviceId, Exception exception, bool skipLogging)
225229
{
226-
if (!_shouldTrackFailed && (_tracingDisabled || !MongoTelemetry.ActivitySource.HasListeners()))
230+
if (!_shouldTrackFailed && !ShouldTraceWithActivityListener())
227231
{
228232
return;
229233
}
230234

231235
CompleteCommandActivityWithException(exception);
232236

237+
if (_state == null)
238+
{
239+
return;
240+
}
241+
233242
var requestIds = _state.Keys;
234243
foreach (var requestId in requestIds)
235244
{
@@ -731,11 +740,19 @@ private void TrackCommandState(
731740
BsonDocument sessionId,
732741
long? transactionNumber)
733742
{
734-
if (!_shouldTrackState)
743+
var shouldTraceCommand = ShouldTraceWithActivityListener() && !shouldRedactCommand && !skipLogging;
744+
745+
if (!_shouldTrackState && !shouldTraceCommand)
735746
{
736747
return;
737748
}
738749

750+
if (!_shouldTrackState && shouldTraceCommand)
751+
{
752+
// Lazy initialize state dictionary when tracing is needed but event tracking is not
753+
_state ??= new ConcurrentDictionary<int, CommandState>();
754+
}
755+
739756
var commandState = new CommandState
740757
{
741758
CommandName = commandName,
@@ -746,7 +763,7 @@ private void TrackCommandState(
746763
ShouldRedactReply = shouldRedactCommand
747764
};
748765

749-
if (!_tracingDisabled && MongoTelemetry.ActivitySource.HasListeners() && !shouldRedactCommand && !skipLogging)
766+
if (shouldTraceCommand)
750767
{
751768
_currentCommandActivity = MongoTelemetry.StartCommandActivity(
752769
commandName,

tests/MongoDB.Driver.Tests/Core/Connections/CommandEventHelperTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,9 @@ public void ShouldTrackState_should_be_correct_with_activity_listener(
119119
var tracingOptions = traceCommands ? new TracingOptions() : new TracingOptions { Disabled = true };
120120
var commandHelper = new CommandEventHelper(eventLogger, tracingOptions);
121121

122-
commandHelper._shouldTrackState().Should().Be(logCommands || captureCommandSucceeded || captureCommandFailed || traceCommands);
122+
// With the new implementation, _shouldTrackState only reflects event tracking,
123+
// not tracing with listeners (which is checked dynamically at execution time)
124+
commandHelper._shouldTrackState().Should().Be(logCommands || captureCommandSucceeded || captureCommandFailed);
123125
}
124126
finally
125127
{

tests/SmokeTests/MongoDB.Driver.SmokeTests.Sdk/LibmongocryptTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void Explicit_encryption_with_libmongocrypt_package_works()
5959
kmsProviders.Add("local", localKey);
6060

6161
var keyVaultNamespace = CollectionNamespace.FromFullName("encryption.__keyVault");
62-
var keyVaultMongoClient = new MongoClient(InfrastructureUtilities.MongoUri);
62+
var keyVaultMongoClient = new MongoClient();
6363
var clientEncryptionSettings = new ClientEncryptionOptions(
6464
keyVaultMongoClient,
6565
keyVaultNamespace,

0 commit comments

Comments
 (0)