Skip to content

Allow custom MongoHandlerObservationConvention usage #4607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Allow custom MongoHandlerObservationConvention usage #4607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa5601f
Allow custom MongoHandlerObservationConvention
francoiskha Jan 10, 2024
54c6308
remove deprecated constructor usage
francoiskha Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion .../java/org/springframework/data/mongodb/observability/MongoObservationCommandListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* @author OpenZipkin Brave Authors
* @author Marcin Grzejszczak
* @author Greg Turnquist
* @author François Kha
* @since 4.0
*/
public class MongoObservationCommandListener implements CommandListener {
Expand All @@ -48,7 +49,7 @@ public class MongoObservationCommandListener implements CommandListener {
private final ObservationRegistry observationRegistry;
private final @Nullable ConnectionString connectionString;

private final MongoHandlerObservationConvention observationConvention = new DefaultMongoHandlerObservationConvention();
private final MongoHandlerObservationConvention observationConvention;

/**
* Create a new {@link MongoObservationCommandListener} to record {@link Observation}s.
Expand All @@ -61,6 +62,7 @@ public MongoObservationCommandListener(ObservationRegistry observationRegistry)

this.observationRegistry = observationRegistry;
this.connectionString = null;
this.observationConvention = new DefaultMongoHandlerObservationConvention();
}

/**
Expand All @@ -77,6 +79,26 @@ public MongoObservationCommandListener(ObservationRegistry observationRegistry,

this.observationRegistry = observationRegistry;
this.connectionString = connectionString;
this.observationConvention = new DefaultMongoHandlerObservationConvention();
}

/**
* Create a new {@link MongoObservationCommandListener} to record {@link Observation}s. This constructor attaches the
* {@link ConnectionString} to every {@link Observation} and uses the given {@link MongoHandlerObservationConvention}
*
* @param observationRegistry must not be {@literal null}
* @param connectionString must not be {@literal null}
* @param observationConvention must not be {@literal null}
*/
public MongoObservationCommandListener(ObservationRegistry observationRegistry, ConnectionString connectionString, MongoHandlerObservationConvention observationConvention) {

Assert.notNull(observationRegistry, "ObservationRegistry must not be null");
Assert.notNull(connectionString, "ConnectionString must not be null");
Assert.notNull(observationConvention, "MongoHandlerObservationConvention must not be null");

this.observationRegistry = observationRegistry;
this.connectionString = connectionString;
this.observationConvention = observationConvention;
}

@Override
Expand Down
60 changes: 44 additions & 16 deletions .../org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static io.micrometer.core.tck.MeterRegistryAssert.*;
import static org.mockito.Mockito.*;

import com.mongodb.ConnectionString;
import io.micrometer.common.KeyValues;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler;
Expand Down Expand Up @@ -48,6 +49,7 @@
* @author Marcin Grzejszczak
* @author Greg Turnquist
* @author Mark Paluch
* @author François Kha
*/
class MongoObservationCommandListenerTests {

Expand All @@ -70,7 +72,7 @@ void setup() {
void commandStartedShouldNotInstrumentWhenAdminDatabase() {

// when
listener.commandStarted(new CommandStartedEvent(null, 0, null, "admin", "", null));
listener.commandStarted(new CommandStartedEvent(null, 0, 0, null, "admin", "", null));

// then
assertThat(meterRegistry).hasNoMetrics();
Expand All @@ -80,7 +82,7 @@ void commandStartedShouldNotInstrumentWhenAdminDatabase() {
void commandStartedShouldNotInstrumentWhenNoRequestContext() {

// when
listener.commandStarted(new CommandStartedEvent(null, 0, null, "some name", "", null));
listener.commandStarted(new CommandStartedEvent(null,0, 0, null, "some name", "", null));

// then
assertThat(meterRegistry).hasNoMetrics();
Expand All @@ -90,7 +92,7 @@ void commandStartedShouldNotInstrumentWhenNoRequestContext() {
void commandStartedShouldNotInstrumentWhenNoParentSampleInRequestContext() {

// when
listener.commandStarted(new CommandStartedEvent(new MapRequestContext(), 0, null, "some name", "", null));
listener.commandStarted(new CommandStartedEvent(new MapRequestContext(),0, 0, null, "some name", "", null));

// then
assertThat(meterRegistry).hasMeterWithName("spring.data.mongodb.command.active");
Expand All @@ -104,14 +106,13 @@ void successfullyCompletedCommandShouldCreateTimerWhenParentSampleInRequestConte
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, //
listener.commandStarted(new CommandStartedEvent(traceRequestContext,0, 0, //
new ConnectionDescription( //
new ServerId( //
new ClusterId("description"), //
new ServerAddress("localhost", 1234))),
"database", "insert", //
new ServerAddress("localhost", 1234))), "database", "insert", //
new BsonDocument("collection", new BsonString("user"))));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, null, "insert", null, 0));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0,0, null, "insert", null, 0));

// then
assertThatTimerRegisteredWithTags();
Expand All @@ -125,14 +126,14 @@ void successfullyCompletedCommandWithCollectionHavingCommandNameShouldCreateTime
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, //
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0,0, //
new ConnectionDescription( //
new ServerId( //
new ClusterId("description"), //
new ServerAddress("localhost", 1234))), //
"database", "aggregate", //
new BsonDocument("aggregate", new BsonString("user"))));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, null, "aggregate", null, 0));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext,0, 0, null, "aggregate", null, 0));

// then
assertThatTimerRegisteredWithTags();
Expand All @@ -146,9 +147,9 @@ void successfullyCompletedCommandWithoutClusterInformationShouldCreateTimerWhenP
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, null, "database", "insert",
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, 0, null, "database", "insert",
new BsonDocument("collection", new BsonString("user"))));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, null, "insert", null, 0));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, 0, null, "insert", null, 0));

assertThat(meterRegistry).hasTimerWithNameAndTags(MongoObservation.MONGODB_COMMAND_OBSERVATION.getName(),
KeyValues.of(LowCardinalityCommandKeyNames.MONGODB_COLLECTION.withValue("user"),
Expand All @@ -173,13 +174,14 @@ void commandWithErrorShouldCreateTimerWhenParentSampleInRequestContext() {
"database", "insert", //
new BsonDocument("collection", new BsonString("user"))));
listener.commandFailed( //
new CommandFailedEvent(traceRequestContext, 0, null, "insert", 0, new IllegalAccessException()));
new CommandFailedEvent(traceRequestContext, 0, 0, null, "insert", 0, new IllegalAccessException()));

// then
assertThatTimerRegisteredWithTags();
}

@Test // GH-4481
@Test
// GH-4481
void completionShouldIgnoreIncompatibleObservationContext() {

// given
Expand All @@ -189,13 +191,14 @@ void completionShouldIgnoreIncompatibleObservationContext() {
traceRequestContext.put(ObservationThreadLocalAccessor.KEY, observation);

// when
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, null, "insert", null, 0));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, 0, null, "insert", null, 0));

verify(observation).getContext();
verifyNoMoreInteractions(observation);
}

@Test // GH-4481
@Test
// GH-4481
void failureShouldIgnoreIncompatibleObservationContext() {

// given
Expand All @@ -205,12 +208,37 @@ void failureShouldIgnoreIncompatibleObservationContext() {
traceRequestContext.put(ObservationThreadLocalAccessor.KEY, observation);

// when
listener.commandFailed(new CommandFailedEvent(traceRequestContext, 0, null, "insert", 0, null));
listener.commandFailed(new CommandFailedEvent(traceRequestContext, 0, 0, null, "insert", 0, null));

verify(observation).getContext();
verifyNoMoreInteractions(observation);
}

@Test
// GH-4321
void shouldUseObservationConvention() {
//given
MongoHandlerObservationConvention customObservationConvention = new MongoHandlerObservationConvention() {
@Override
public boolean supportsContext(Observation.Context context) {
return MongoHandlerObservationConvention.super.supportsContext(context);
}

@Override
public String getName() {
return "custom.name";
}
};
this.listener = new MongoObservationCommandListener(observationRegistry, mock(ConnectionString.class),
customObservationConvention);

// when
listener.commandStarted(new CommandStartedEvent(new MapRequestContext(), 0, 0,null, "some name", "", null));

// then
assertThat(meterRegistry).hasMeterWithName("custom.name.active");
}

private RequestContext getContext() {
return ((SynchronousContextProvider) ContextProviderFactory.create(observationRegistry)).getContext();
}
Expand Down