From aa5601fde14a93221e142a24b2f79fa9df1b4487 Mon Sep 17 00:00:00 2001 From: francoiskha Date: Wed, 10 Jan 2024 09:21:35 +0100 Subject: [PATCH 1/2] Allow custom MongoHandlerObservationConvention Closes #4321 add author --- .../MongoObservationCommandListener.java | 24 ++++++++++++- .../MongoObservationCommandListenerTests.java | 36 ++++++++++++++++--- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/observability/MongoObservationCommandListener.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/observability/MongoObservationCommandListener.java index a884919e16..da653a64f0 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/observability/MongoObservationCommandListener.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/observability/MongoObservationCommandListener.java @@ -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 { @@ -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. @@ -61,6 +62,7 @@ public MongoObservationCommandListener(ObservationRegistry observationRegistry) this.observationRegistry = observationRegistry; this.connectionString = null; + this.observationConvention = new DefaultMongoHandlerObservationConvention(); } /** @@ -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 diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java index 14a2d5a93c..7ae673d1b8 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java @@ -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; @@ -48,6 +49,7 @@ * @author Marcin Grzejszczak * @author Greg Turnquist * @author Mark Paluch + * @author François Kha */ class MongoObservationCommandListenerTests { @@ -108,8 +110,7 @@ void successfullyCompletedCommandShouldCreateTimerWhenParentSampleInRequestConte 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)); @@ -179,7 +180,8 @@ void commandWithErrorShouldCreateTimerWhenParentSampleInRequestContext() { assertThatTimerRegisteredWithTags(); } - @Test // GH-4481 + @Test + // GH-4481 void completionShouldIgnoreIncompatibleObservationContext() { // given @@ -195,7 +197,8 @@ void completionShouldIgnoreIncompatibleObservationContext() { verifyNoMoreInteractions(observation); } - @Test // GH-4481 + @Test + // GH-4481 void failureShouldIgnoreIncompatibleObservationContext() { // given @@ -211,6 +214,31 @@ void failureShouldIgnoreIncompatibleObservationContext() { 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, null, "some name", "", null)); + + // then + assertThat(meterRegistry).hasMeterWithName("custom.name.active"); + } + private RequestContext getContext() { return ((SynchronousContextProvider) ContextProviderFactory.create(observationRegistry)).getContext(); } From 54c630837df9735cb23453205855b81cc7951f86 Mon Sep 17 00:00:00 2001 From: francoiskha Date: Wed, 10 Jan 2024 10:58:15 +0100 Subject: [PATCH 2/2] remove deprecated constructor usage --- .../MongoObservationCommandListenerTests.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java index 7ae673d1b8..84e8354894 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java @@ -72,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(); @@ -82,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(); @@ -92,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"); @@ -106,13 +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 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(); @@ -126,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(); @@ -147,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"), @@ -174,7 +174,7 @@ 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(); @@ -191,7 +191,7 @@ 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); @@ -208,7 +208,7 @@ 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); @@ -233,7 +233,7 @@ public String getName() { customObservationConvention); // 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("custom.name.active");