Skip to content

Remove parent observation key instead of setting null #5083

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 4 commits into from
Closed

Remove parent observation key instead of setting null #5083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18ca693
Prepare issue branch.
mp911de Oct 27, 2025
b0efa15
Remove parent observation key instead of setting null.
mp911de Oct 27, 2025
9609b73
Polishing.
schauder Oct 29, 2025
873cb5f
Polishing.
mp911de Oct 29, 2025
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>5.0.0-SNAPSHOT</version>
<version>5.0.0-GH-5082-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data MongoDB</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb-distribution/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>5.0.0-SNAPSHOT</version>
<version>5.0.0-GH-5082-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-mongodb/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-mongodb-parent</artifactId>
<version>5.0.0-SNAPSHOT</version>
<version>5.0.0-GH-5082-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
9 changes: 8 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 @@ -17,6 +17,7 @@

import io.micrometer.observation.Observation;
import io.micrometer.observation.ObservationRegistry;
import io.micrometer.observation.ObservationView;
import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor;

import java.util.function.BiConsumer;
Expand Down Expand Up @@ -191,7 +192,13 @@ private void stopObservation(@Nullable RequestContext requestContext,
log.debug(
"Restoring parent observation [" + observation + "] for Mongo instrumentation and put it in Mongo context");
}
requestContext.put(ObservationThreadLocalAccessor.KEY, observation.getContext().getParentObservation());
ObservationView parentObservation = observation.getContext().getParentObservation();

if (parentObservation == null) {
requestContext.delete(ObservationThreadLocalAccessor.KEY);
} else {
requestContext.put(ObservationThreadLocalAccessor.KEY, parentObservation);
}
}
}

Expand Down
60 changes: 30 additions & 30 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 @@ -26,10 +26,12 @@
import io.micrometer.observation.Observation;
import io.micrometer.observation.ObservationRegistry;
import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor;
import reactor.core.publisher.BaseSubscriber;

import org.assertj.core.api.Assertions;
import org.bson.BsonDocument;
import org.bson.BsonString;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -43,6 +45,7 @@
import com.mongodb.event.CommandFailedEvent;
import com.mongodb.event.CommandStartedEvent;
import com.mongodb.event.CommandSucceededEvent;
import com.mongodb.reactivestreams.client.ReactiveContextProvider;

/**
* Series of test cases exercising {@link MongoObservationCommandListener}.
Expand Down Expand Up @@ -70,57 +73,74 @@ void setup() {
this.listener = new MongoObservationCommandListener(observationRegistry);
}

@AfterEach
void tearDown() {
Observation currentObservation = observationRegistry.getCurrentObservation();
if (currentObservation != null) {
currentObservation.stop();
observationRegistry.setCurrentObservationScope(null);
}
}

@Test
void commandStartedShouldNotInstrumentWhenAdminDatabase() {

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

// then
assertThat(meterRegistry).hasNoMetrics();
}

@Test
void commandStartedShouldNotInstrumentWhenNoRequestContext() {

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

// then
assertThat(meterRegistry).hasNoMetrics();
}

@Test
void commandStartedShouldNotInstrumentWhenNoParentSampleInRequestContext() {

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

// then
assertThat(meterRegistry).hasMeterWithName("spring.data.mongodb.command.active");
}

@Test // GH-4994
void commandStartedShouldAlwaysIncludeCollection() {

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

// then
// although command 'hello' is collection-less, metric must have tag "db.mongodb.collection"
assertThat(meterRegistry).hasMeterWithNameAndTags(
"spring.data.mongodb.command.active",
Tags.of("db.mongodb.collection", "none"));
}

@Test // GH-5082
void reactiveContextCompletesNormally() {

ReactiveContextProvider rcp = (ReactiveContextProvider) ContextProviderFactory.create(observationRegistry);
RequestContext context = rcp.getContext(new BaseSubscriber<>() {});

listener.commandStarted(new CommandStartedEvent(context, 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(context, 0, 0, null, "insert", null, null, 0));

assertThatTimerRegisteredWithTags();
}

@Test
void successfullyCompletedCommandShouldCreateTimerWhenParentSampleInRequestContext() {

// given
Observation parent = Observation.start("name", observationRegistry);
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, 0, //
new ConnectionDescription( //
new ServerId( //
Expand All @@ -129,18 +149,15 @@ void successfullyCompletedCommandShouldCreateTimerWhenParentSampleInRequestConte
new BsonDocument("collection", new BsonString("user"))));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, 0, null, "insert", null, null, 0));

// then
assertThatTimerRegisteredWithTags();
}

@Test
void successfullyCompletedCommandWithCollectionHavingCommandNameShouldCreateTimerWhenParentSampleInRequestContext() {

// given
Observation parent = Observation.start("name", observationRegistry);
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, 0, //
new ConnectionDescription( //
new ServerId( //
Expand All @@ -150,18 +167,15 @@ void successfullyCompletedCommandWithCollectionHavingCommandNameShouldCreateTime
new BsonDocument("aggregate", new BsonString("user"))));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, 0, null, "aggregate", null, null, 0));

// then
assertThatTimerRegisteredWithTags();
}

@Test
void successfullyCompletedCommandWithoutClusterInformationShouldCreateTimerWhenParentSampleInRequestContext() {

// given
Observation parent = Observation.start("name", observationRegistry);
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, 0, null, "database", "insert",
new BsonDocument("collection", new BsonString("user"))));
listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, 0, null, "insert", null, null, 0));
Expand All @@ -176,11 +190,9 @@ void successfullyCompletedCommandWithoutClusterInformationShouldCreateTimerWhenP
@Test
void commandWithErrorShouldCreateTimerWhenParentSampleInRequestContext() {

// given
Observation parent = Observation.start("name", observationRegistry);
RequestContext traceRequestContext = getContext();

// when
listener.commandStarted(new CommandStartedEvent(traceRequestContext, 0, 0, //
new ConnectionDescription( //
new ServerId( //
Expand All @@ -191,20 +203,17 @@ void commandWithErrorShouldCreateTimerWhenParentSampleInRequestContext() {
listener.commandFailed( //
new CommandFailedEvent(traceRequestContext, 0, 0, null, "db", "insert", 0, new IllegalAccessException()));

// then
assertThatTimerRegisteredWithTags();
}

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

// given
RequestContext traceRequestContext = getContext();

Observation observation = mock(Observation.class);
traceRequestContext.put(ObservationThreadLocalAccessor.KEY, observation);

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

verify(observation).getContext();
Expand All @@ -214,13 +223,11 @@ void completionShouldIgnoreIncompatibleObservationContext() {
@Test // GH-4481
void failureShouldIgnoreIncompatibleObservationContext() {

// given
RequestContext traceRequestContext = getContext();

Observation observation = mock(Observation.class);
traceRequestContext.put(ObservationThreadLocalAccessor.KEY, observation);

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

verify(observation).getContext();
Expand All @@ -230,7 +237,6 @@ void failureShouldIgnoreIncompatibleObservationContext() {
@Test // GH-4321
void shouldUseObservationConvention() {

// given
MongoHandlerObservationConvention customObservationConvention = new MongoHandlerObservationConvention() {
@Override
public boolean supportsContext(Observation.Context context) {
Expand All @@ -245,22 +251,18 @@ public String getName() {
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");
}

@Test // GH-5064
void completionRestoresParentObservation() {

// given
Observation parent = Observation.start("name", observationRegistry);
observationRegistry.setCurrentObservationScope(parent.openScope());
RequestContext traceRequestContext = getContext();

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

Expand All @@ -275,12 +277,10 @@ void completionRestoresParentObservation() {
@Test // GH-5064
void failureRestoresParentObservation() {

// given
Observation parent = Observation.start("name", observationRegistry);
observationRegistry.setCurrentObservationScope(parent.openScope());
RequestContext traceRequestContext = getContext();

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

Expand Down