Conversation
The driver used a single observation name ("mongodb") for both operation-level and command-level spans, which have different sets of low-cardinality tag keys. Prometheus requires all meters sharing a name to have identical tag key sets, causing the second observation type to be silently dropped.
Split MongodbObservation.MONGODB_OBSERVATION into MONGODB_OPERATION (name "mongodb.operation") and MONGODB_COMMAND (name "mongodb.command"), each declaring its own low-cardinality key set. Updated Tracer and TracingManager to pass the observation type through span creation.
Connection IDs, cursor IDs, session IDs, transaction numbers, and exception details were tagged as low-cardinality, causing unbounded Prometheus metric cardinality since their values change per-connection, per-cursor, or per-error. Moved CLIENT_CONNECTION_ID, SERVER_CONNECTION_ID, CURSOR_ID,TRANSACTION_NUMBER, SESSION_ID, EXCEPTION_MESSAGE, EXCEPTION_TYPE, and EXCEPTION_STACKTRACE from CommandLowCardinalityKeyNames to HighCardinalityKeyNames so they appear only in traces, not in metrics. Added tagHighCardinality(KeyValue) and tagHighCardinality(KeyValues) to the Span interface to support string-valued high-cardinality tags alongside the existing BsonDocument overload.
The query text max length configuration constant was stored in every Observation.Context and extracted back in the MicrometerSpan constructor. This value never changes between observations and is not output as any signal. Pass it directly via constructor parameter instead.
Observations were created with Micrometer's generic SenderContext, preventing users from filtering or customizing MongoDB observations by context type. This blocks the ObservationConvention pattern that Spring Boot needs for tag alignment. Introduced MongodbContext extending SenderContext<Object> with Kind.CLIENT, giving users a MongoDB-specific type to register ObservationHandler<MongodbContext> or ObservationConvention<MongodbContext> instances scoped to only MongoDB observations.
…f TracingManager Replaced all imperative tagLowCardinality/tagHighCardinality calls with a convention-based approach. TracingManager and InternalStreamConnection now populate domain fields on MongodbContext, and DefaultMongodbObservationConvention reads those fields at stop time to produce the final key-values. This decouples tag naming from span creation, enabling users to register a GlobalObservationConvention<MongodbContext> to customize tag names for their environment (e.g. Spring Boot tag alignment with their existing DefaultMongoCommandTagsProvider). Added domain fields to MongodbContext: observationType, commandName, databaseName, collectionName, serverAddress, connectionId, cursorId, transactionNumber, sessionId, queryText, responseStatusCode. Removed tagLowCardinality/tagHighCardinality from the Span interface as they are no longer used.
9b4b0ad to
bf91627
Compare
Update attribute name for OpenTelemetry
The driver called observation.start()/stop() but never openScope()/ closeScope(). Without scopes, registry.getCurrentObservation() returned null during MongoDB operations, breaking context propagation for any downstream code (Spring interceptors, user observations, MDC logging). For example, in withTransaction, a user observation created inside the callback would attach to the Spring HTTP parent instead of the MongoDB transaction span, because the transaction observation was never made "current" on the thread. Added openScope()/closeScope() to the Span interface with scope lifecycle management in MongoClusterImpl (operation spans), InternalStreamConnection (command spans), and TransactionSpan.
When a getMore command arrived, the cursor_id was being set on the parent operation span's MongodbContext even though the parent observation was already stopped. Modifying an observation after stop() is undefined behavior in Micrometer
There was a problem hiding this comment.
Pull request overview
Refactors Micrometer-based tracing to align better with OpenTelemetry/Micrometer conventions by separating operation vs command observations, moving tag production into an ObservationConvention, and introducing explicit span scope management in sync execution paths.
Changes:
- Split MongoDB observations into distinct operation- and command-level observation types to avoid tag-keyset collisions (e.g., Prometheus restrictions).
- Replace imperative tagging with a
MongodbContext+DefaultMongodbObservationConventionthat derives tags from populated domain fields. - Add explicit
openScope()/closeScope()lifecycle handling for spans in sync execution code paths and update unified test modifications accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java | Updates unified test skip rules for OpenTelemetry/Micrometer-related specs. |
| driver-sync/src/test/functional/com/mongodb/client/unified/Entities.java | Minor formatting adjustment in Micrometer observability settings setup. |
| driver-sync/src/test/functional/com/mongodb/client/observability/SpanTree.java | Updates test tag-key imports to match new low/high-cardinality key enums. |
| driver-sync/src/main/com/mongodb/client/internal/MongoClusterImpl.java | Opens/closes span scope around sync operation execution. |
| driver-sync/src/main/com/mongodb/client/internal/ClientSessionImpl.java | Opens transaction-span scope when starting a transaction (sync). |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/TransactionSpan.java | Ensures transaction span scope is closed before ending in finalization paths; adds openScope() API. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/TracingManager.java | Switches span creation to operation vs command observation types and populates MongodbContext fields for conventions. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/Tracer.java | Updates tracer API to accept an observation type when creating spans. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/Span.java | Replaces tag APIs with scope management + query-text setting + context access. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/MongodbObservation.java | Splits key names into operation vs command low-cardinality sets and reorganizes high-cardinality keys. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/MongodbContext.java | Introduces a MongoDB-specific Micrometer SenderContext to hold domain fields used by conventions. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/MicrometerTracer.java | Uses MongodbContext, registers the default convention, and implements new Span API. |
| driver-core/src/main/com/mongodb/internal/observability/micrometer/DefaultMongodbObservationConvention.java | New default global convention that emits tags from MongodbContext fields (including errors). |
| driver-core/src/main/com/mongodb/internal/connection/InternalStreamConnection.java | Uses new Span API, populates query text/status code via MongodbContext, and opens/closes scope around command spans. |
| config/checkstyle/suppressions.xml | Moves the printStackTrace suppression to the new convention class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move openScope() calls after code that can throw to prevent scope leaks on early exceptions in MongoClusterImpl (binding creation) and InternalStreamConnection (command setup). Update createTracingSpan Javadoc to reflect convention-based tagging.
…ntion - Renamed MongodbContext to MongodbObservationContext and moved it along with DefaultMongodbObservationConvention to the public package com.mongodb.observability.micrometer - Added observationConvention() to MicrometerObservabilitySettings so users can provide a custom convention - Added testCustomObservationConvention to AbstractMicrometerProseTest validating that a user-provided convention fully controls tag output
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
driver-core/src/main/com/mongodb/internal/connection/InternalStreamConnection.java:464
createTracingSpan(...)unconditionally callscommandDocumentSupplier.get()(to determine command name / sensitivity). When command logging or command-payload tracing is also enabled,sendAndReceiveInternalthen callsmessage.getCommandDocument(bsonOutput)again, causing the command document to be reconstructed twice from the same encoded buffers. Consider hydrating theBsonDocumentonce and reusing it for both tracing-span creation and logging/payload (e.g., via a memoized supplier or by passing the already-built document into tracing).
tracingSpan = operationContext
.getTracingManager()
.createTracingSpan(message,
operationContext,
() -> message.getCommandDocument(bsonOutput),
cmdName -> SECURITY_SENSITIVE_COMMANDS.contains(cmdName)
|| SECURITY_SENSITIVE_HELLO_COMMANDS.contains(cmdName),
() -> getDescription().getServerAddress(),
() -> getDescription().getConnectionId()
);
boolean isLoggingCommandNeeded = isLoggingCommandNeeded();
boolean isTracingCommandPayloadNeeded = tracingSpan != null && operationContext.getTracingManager().isCommandPayloadEnabled();
// Only hydrate the command document if necessary
BsonDocument commandDocument = null;
if (isLoggingCommandNeeded || isTracingCommandPayloadNeeded) {
commandDocument = message.getCommandDocument(bsonOutput);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import com.mongodb.internal.session.SessionContext; | ||
| import com.mongodb.lang.Nullable; | ||
| import com.mongodb.observability.ObservabilitySettings; | ||
| import com.mongodb.observability.micrometer.DefaultMongodbObservationConvention; |
There was a problem hiding this comment.
DefaultMongodbObservationConvention is imported but only referenced in Javadoc. Checkstyle's UnusedImports check will still flag this as an unused import and fail the build. Remove the import and use a fully-qualified type in the Javadoc (or reference it from code).
| import com.mongodb.observability.micrometer.DefaultMongodbObservationConvention; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -427,8 +427,9 @@ public <T> T execute(final ReadOperation<T, ?> operation, final ReadPreference r | |||
| Span span = operationContext.getTracingManager().createOperationSpan( | |||
| actualClientSession.getTransactionSpan(), operationContext, operation.getCommandName(), operation.getNamespace()); | |||
There was a problem hiding this comment.
createOperationSpan(...) is currently always given actualClientSession.getTransactionSpan(), even when actualClientSession.hasActiveTransaction() is false. Since ClientSessionImpl retains the last transactionSpan after commit/abort, this will incorrectly parent subsequent non-transaction operations under an already-finished transaction span. Pass the transaction span only when the session has an active transaction (or update getTransactionSpan() to return null when inactive) so trace nesting is correct.
| actualClientSession.getTransactionSpan(), operationContext, operation.getCommandName(), operation.getNamespace()); | |
| actualClientSession.hasActiveTransaction() ? actualClientSession.getTransactionSpan() : null, | |
| operationContext, operation.getCommandName(), operation.getNamespace()); |
| if (tracingManager.isEnabled()) { | ||
| transactionSpan = new TransactionSpan(tracingManager); | ||
| transactionSpan.openScope(); | ||
| } |
There was a problem hiding this comment.
transactionSpan is created (and now has its scope opened) in startTransaction(...), but it is never cleared when the transaction is finished. Because other code paths pass getTransactionSpan() into operation span creation, a committed/aborted transaction span can remain as the parent for later non-transaction operations within the same session. Consider clearing transactionSpan when transitioning out of an active transaction (e.g., in cleanupTransaction(...) / after commit+abort), and/or make getTransactionSpan() return null when !hasActiveTransaction().
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public Span addTransactionSpan() { | ||
| Span span = tracer.nextSpan("transaction", null, null); | ||
| span.tagLowCardinality(SYSTEM.withValue("mongodb")); | ||
| return span; | ||
| return tracer.nextSpan(MONGODB_OPERATION, "transaction", null, null); | ||
| } |
There was a problem hiding this comment.
addTransactionSpan() creates the transaction span as a MONGODB_OPERATION observation, but the driver never populates operation domain fields (db namespace / collection / operation name / summary) for transactions. With DefaultMongodbObservationConvention, this will emit a different set of tag keys for mongodb.operation when the span represents a transaction vs a normal operation, which can reintroduce Prometheus meter rejection due to inconsistent label-name sets for the same metric name.
Consider introducing a dedicated observation type for transactions (e.g., mongodb.transaction with its own fixed key set), or populating the operation fields for the transaction span so mongodb.operation observations are consistent.
| final MicrometerObservabilitySettings that = (MicrometerObservabilitySettings) o; | ||
| return enableCommandPayloadTracing == that.enableCommandPayloadTracing | ||
| && Objects.equals(observationRegistry, that.observationRegistry); | ||
| && Objects.equals(observationRegistry, that.observationRegistry) | ||
| && Objects.equals(observationConvention, that.observationConvention); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(observationRegistry, enableCommandPayloadTracing); | ||
| return Objects.hash(observationRegistry, enableCommandPayloadTracing, observationConvention); | ||
| } |
There was a problem hiding this comment.
equals/hashCode ignore maxQueryTextLength. This means two MicrometerObservabilitySettings instances that differ only by query-text truncation settings will compare equal and hash to the same bucket, which can lead to incorrect behavior if these settings are ever used as keys (e.g., caching, deduplication, tests).
Include maxQueryTextLength in both equals and hashCode (and consider whether other fields should also participate) so object equality reflects the full configuration.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @SuppressWarnings("NullableProblems") | ||
| @Test | ||
| void testCustomObservationConvention() { | ||
| ObservationConvention<MongodbObservationContext> customConvention = new ObservationConvention<MongodbObservationContext>() { | ||
| @Override | ||
| public boolean supportsContext(final Observation.Context context) { | ||
| return context instanceof MongodbObservationContext; | ||
| } | ||
|
|
||
| @Override | ||
| public KeyValues getLowCardinalityKeyValues(final MongodbObservationContext context) { | ||
| String commandName = context.getCommandName() != null ? context.getCommandName() : ""; | ||
| String databaseName = context.getDatabaseName() != null ? context.getDatabaseName() : ""; | ||
|
|
||
| KeyValues kv = KeyValues.of( | ||
| "db.system.name", "mongodb", | ||
| "db.namespace", databaseName, | ||
| "custom.tag", "custom-value"); | ||
|
|
||
| if (context.getObservationType() == MongodbObservation.MONGODB_COMMAND) { | ||
| // Rename: emit command name under "mongodb.command" instead of "db.command.name" | ||
| kv = kv.and("mongodb.command", commandName); | ||
| } | ||
| if (context.getObservationType() == MongodbObservation.MONGODB_OPERATION) { | ||
| kv = kv.and("db.operation.name", commandName); | ||
| } | ||
| return kv; | ||
| } | ||
| }; | ||
|
|
||
| MongoClientSettings clientSettings = getMongoClientSettingsBuilder() | ||
| .observabilitySettings(ObservabilitySettings.micrometerBuilder() | ||
| .observationRegistry(observationRegistry) | ||
| .observationConvention(customConvention) | ||
| .build()) | ||
| .build(); |
There was a problem hiding this comment.
testCustomObservationConvention doesn’t set ENV_OBSERVABILITY_ENABLED. Since other tests in this class mutate that env var and it’s only restored in @AfterAll, this makes the test order-/environment-dependent (it will fail when run in isolation or after a test that left the var disabled). Set ENV_OBSERVABILITY_ENABLED explicitly in this test (or reset it in @BeforeEach) to make the assertions deterministic.
| Span span = operationContext.getTracingManager().createOperationSpan( | ||
| actualClientSession.getTransactionSpan(), operationContext, operation.getCommandName(), operation.getNamespace()); | ||
| ReadBinding binding = getReadBinding(readPreference, actualClientSession, implicitSession); | ||
|
|
||
|
|
||
| if (span != null) { | ||
| span.openScope(); | ||
| } | ||
| try { |
There was a problem hiding this comment.
span is created before getReadBinding(...). If getReadBinding (or getReadPreferenceForBinding) throws, the operation span is never ended (and never closed if scope was opened later), which can leave an Observation in a started-but-unended state. Consider restructuring so span lifecycle (openScope/closeScope/end and error tagging) is guaranteed via a try/finally that also covers binding acquisition failures (e.g., acquire binding inside the try and release/close/end in a finally that always runs once a span is created).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Override | ||
| public boolean equals(final Object o) { | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| final MicrometerObservabilitySettings that = (MicrometerObservabilitySettings) o; | ||
| return enableCommandPayloadTracing == that.enableCommandPayloadTracing | ||
| && Objects.equals(observationRegistry, that.observationRegistry); | ||
| && Objects.equals(observationRegistry, that.observationRegistry) | ||
| && Objects.equals(observationConvention, that.observationConvention); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(observationRegistry, enableCommandPayloadTracing); | ||
| return Objects.hash(observationRegistry, enableCommandPayloadTracing, observationConvention); | ||
| } |
There was a problem hiding this comment.
MicrometerObservabilitySettings.equals / hashCode do not incorporate maxQueryTextLength. Two settings instances with different query text truncation behavior will compare equal and can collide in hash-based collections/caches. Include maxQueryTextLength in both equals and hashCode so equality reflects all configuration that affects behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private MongodbObservation observationType; | ||
| @Nullable | ||
| private String commandName; |
There was a problem hiding this comment.
MongodbObservationContext is a public API type (com.mongodb.observability.micrometer) but it exposes com.mongodb.internal.observability.micrometer.MongodbObservation via the observationType field/getter. This leaks an internal.* type into the public surface area and makes it hard to evolve internal observability types without breaking users. Consider introducing a public enum (e.g., MongodbObservationType { OPERATION, COMMAND }) in the public package, or moving MongodbObservation out of internal if it is intended to be user-consumable.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import com.mongodb.ServerAddress; | ||
| import com.mongodb.annotations.Beta; | ||
| import com.mongodb.annotations.Reason; | ||
| import com.mongodb.connection.ConnectionId; | ||
| import com.mongodb.internal.observability.micrometer.MongodbObservation; | ||
| import com.mongodb.lang.Nullable; |
There was a problem hiding this comment.
MongodbObservationContext is a public API type (com.mongodb.observability.micrometer), but it exposes com.mongodb.internal.observability.micrometer.MongodbObservation in its public surface (getObservationType / setObservationType). That leaks an internal type into the public API, making it hard to evolve the internal enum without breaking users who write custom ObservationConventions. Consider introducing a public enum (e.g., MongodbObservationType in the same public package) or splitting into distinct public context subtypes for command vs operation observations so users don’t need to depend on com.mongodb.internal.*.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return enableCommandPayloadTracing == that.enableCommandPayloadTracing | ||
| && Objects.equals(observationRegistry, that.observationRegistry); | ||
| && Objects.equals(observationRegistry, that.observationRegistry) | ||
| && Objects.equals(observationConvention, that.observationConvention); | ||
| } | ||
|
|
There was a problem hiding this comment.
equals/hashCode now account for observationConvention, but still ignore maxQueryTextLength. That means two settings instances with different query-text truncation can compare equal and collide in hash-based collections/caches. Include maxQueryTextLength in both equals and hashCode (and any other fields that define the settings’ behavior).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import com.mongodb.ServerAddress; | ||
| import com.mongodb.annotations.Beta; | ||
| import com.mongodb.annotations.Reason; | ||
| import com.mongodb.connection.ConnectionId; | ||
| import com.mongodb.internal.observability.micrometer.MongodbObservation; |
There was a problem hiding this comment.
This public API class (com.mongodb.observability.micrometer.DefaultMongodbObservationConvention) imports com.mongodb.internal.observability.micrometer.MongodbObservation. That couples the public convention API to internal implementation details and forces users to depend on internal packages. Prefer a public observation-type enum/type (or move MongodbObservation out of internal) so custom conventions can be implemented without internal imports.
| import com.mongodb.MongoClientSettings; | ||
| import com.mongodb.observability.micrometer.MongodbObservationContext; | ||
| import com.mongodb.internal.observability.micrometer.MongodbObservation; | ||
| import com.mongodb.lang.Nullable; |
There was a problem hiding this comment.
This test validates the public observationConvention(...) API, but it needs to import com.mongodb.internal.observability.micrometer.MongodbObservation to distinguish command vs operation (context.getObservationType() == ...). That’s a sign the public convention/context API is still leaking internal types. After introducing a public observation-type enum in the Micrometer API, update this test to use that public type instead.
JAVA-6159
AI Usage Summary
Claude-Caude with Opus 4.6 1M Model
What AI did well
Where the user corrected or pushed back
CommonLowCardinalityKeyNamesrefactor; user asked for suggestion only, not implementation — had to reverttagHighCardinalityin InternalStreamConnectiongetMongodbContext()insteadopenScope()in the constructor (shared by sync+reactive); user asked why scope is public — led to discovering it should only be called from synccontextWrite,contextCapture,AtomicReferencepatterns — all failed due toMono.from(subscriber -> ...)pattern. User questionedHooks.enableAutomaticContextPropagationcontext-propagation=autoalone would suffice — AI confirmed, user decided to stash reactive changes and keep it simpleTransactionSpanconstructoroperationSpan.context()contextWriteandcontextCaptureto prove tests still passed — exposed that tests weren't actually validating the code they claimed to test