Skip to content

Commit 44f53ba

Browse files
ajcvickersajcvickers
committed
Updates based on review feedback.
1 parent 9931342 commit 44f53ba

File tree

3 files changed

+30
-33
lines changed

3 files changed

+30
-33
lines changed

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

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,13 @@ public void AfterSending(RequestMessage message, ConnectionId connectionId, Obje
149149

150150
public void ErrorSending(RequestMessage message, ConnectionId connectionId, ObjectId? serviceId, Exception exception, bool skipLogging)
151151
{
152-
CompleteCommandActivityWithException(exception);
153-
154152
if (_state == null)
155153
{
156154
return;
157155
}
158156

157+
CompleteFailedCommandActivity(exception);
158+
159159
if (_state.TryRemove(message.RequestId, out var state))
160160
{
161161
state.Stopwatch.Stop();
@@ -198,13 +198,13 @@ 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-
CompleteCommandActivityWithException(exception);
202-
203201
if (_state == null)
204202
{
205203
return;
206204
}
207205

206+
CompleteFailedCommandActivity(exception);
207+
208208
if (!_state.TryRemove(responseTo, out var state))
209209
{
210210
// this indicates a bug in the sending portion...
@@ -232,13 +232,13 @@ public void ConnectionFailed(ConnectionId connectionId, ObjectId? serviceId, Exc
232232
return;
233233
}
234234

235-
CompleteCommandActivityWithException(exception);
236-
237235
if (_state == null)
238236
{
239237
return;
240238
}
241239

240+
CompleteFailedCommandActivity(exception);
241+
242242
var requestIds = _state.Keys;
243243
foreach (var requestId in requestIds)
244244
{
@@ -787,16 +787,6 @@ private void CompleteCommandActivityWithSuccess(BsonDocument reply = null)
787787
}
788788
}
789789

790-
private void CompleteCommandActivityWithException(Exception exception)
791-
{
792-
if (_currentCommandActivity is not null)
793-
{
794-
MongoTelemetry.RecordException(_currentCommandActivity, exception);
795-
_currentCommandActivity.Dispose();
796-
_currentCommandActivity = null;
797-
}
798-
}
799-
800790
private void HandleCommandFailure(
801791
CommandState state,
802792
BsonDocument reply,

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

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ public void ShouldRedactCommand_should_return_expected_result(string commandJson
5858
public void ShouldTrackState_should_be_correct(
5959
[Values(false, true)] bool logCommands,
6060
[Values(false, true)] bool captureCommandSucceeded,
61-
[Values(false, true)] bool captureCommandFailed,
62-
[Values(false, true)] bool traceCommands)
61+
[Values(false, true)] bool captureCommandFailed)
6362
{
6463
var mockLogger = new Mock<ILogger<LogCategories.Command>>();
6564
mockLogger.Setup(m => m.IsEnabled(LogLevel.Debug)).Returns(logCommands);
@@ -76,7 +75,7 @@ public void ShouldTrackState_should_be_correct(
7675
}
7776

7877
var eventLogger = new EventLogger<LogCategories.Command>(eventCapturer, mockLogger.Object);
79-
var tracingOptions = traceCommands ? new TracingOptions() : new TracingOptions { Disabled = true };
78+
var tracingOptions = new TracingOptions { Disabled = true };
8079
var commandHelper = new CommandEventHelper(eventLogger, tracingOptions);
8180

8281
// No ActivityListener, so tracing doesn't contribute to _shouldTrackState
@@ -85,7 +84,7 @@ public void ShouldTrackState_should_be_correct(
8584

8685
[Theory]
8786
[ParameterAttributeData]
88-
public void ShouldTrackState_should_be_correct_with_activity_listener(
87+
public void Callbacks_turn_on_when_listener_is_added_even_if_no_events(
8988
[Values(false, true)] bool logCommands,
9089
[Values(false, true)] bool captureCommandSucceeded,
9190
[Values(false, true)] bool captureCommandFailed,
@@ -94,13 +93,6 @@ public void ShouldTrackState_should_be_correct_with_activity_listener(
9493
ActivityListener listener = null;
9594
try
9695
{
97-
listener = new ActivityListener
98-
{
99-
ShouldListenTo = source => source.Name == "MongoDB.Driver",
100-
Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData
101-
};
102-
ActivitySource.AddActivityListener(listener);
103-
10496
var mockLogger = new Mock<ILogger<LogCategories.Command>>();
10597
mockLogger.Setup(m => m.IsEnabled(LogLevel.Debug)).Returns(logCommands);
10698

@@ -119,9 +111,27 @@ public void ShouldTrackState_should_be_correct_with_activity_listener(
119111
var tracingOptions = traceCommands ? new TracingOptions() : new TracingOptions { Disabled = true };
120112
var commandHelper = new CommandEventHelper(eventLogger, tracingOptions);
121113

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);
114+
// When there are no listeners, these only return true if logging is enabled or an event is registered,
115+
// regardless of whether tracing is enabled.
116+
commandHelper.ShouldCallBeforeSending.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands);
117+
commandHelper.ShouldCallAfterSending.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands);
118+
commandHelper.ShouldCallErrorSending.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands);
119+
commandHelper.ShouldCallAfterReceiving.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands);
120+
commandHelper.ShouldCallErrorReceiving.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands);
121+
122+
listener = new ActivityListener
123+
{
124+
ShouldListenTo = source => source.Name == "MongoDB.Driver",
125+
Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData
126+
};
127+
ActivitySource.AddActivityListener(listener);
128+
129+
// With listeners registered, these always return true when unless everything is disabled.
130+
commandHelper.ShouldCallBeforeSending.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands || traceCommands);
131+
commandHelper.ShouldCallAfterSending.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands || traceCommands);
132+
commandHelper.ShouldCallErrorSending.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands || traceCommands);
133+
commandHelper.ShouldCallAfterReceiving.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands || traceCommands);
134+
commandHelper.ShouldCallErrorReceiving.Should().Be(captureCommandSucceeded || captureCommandFailed || logCommands || traceCommands);
125135
}
126136
finally
127137
{

‎tests/SmokeTests/MongoDB.Driver.SmokeTests.Sdk/OpenTelemetryTests.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
using System.Collections.Generic;
1818
using System.Diagnostics;
1919
using System.Linq;
20-
using System.Threading;
2120
using FluentAssertions;
2221
using MongoDB.Bson;
2322
using MongoDB.Driver.Core.Configuration;
@@ -45,8 +44,6 @@ public void MongoClient_should_create_activities_when_tracing_enabled()
4544
collection.InsertOne(new BsonDocument("name", "test"));
4645
collection.Find(Builders<BsonDocument>.Filter.Empty).FirstOrDefault();
4746
collection.DeleteOne(Builders<BsonDocument>.Filter.Eq("name", "test"));
48-
49-
SpinWait.SpinUntil(() => capturedActivities.Count >= 6, millisecondsTimeout: 10000);
5047
}
5148
finally
5249
{

0 commit comments

Comments
 (0)