Skip to content

Add Client Metadata Update Support. #1708

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.

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

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 18 commits
Commits
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
4 changes: 4 additions & 0 deletions buildSrc/src/main/kotlin/conventions/testing-base.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,7 @@ testlogger {
showFailedStandardStreams = true
logLevel = LogLevel.LIFECYCLE
}

dependencies {
testImplementation(libs.assertj)
}
Comment on lines +109 to +111
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AssertJ has been used in this PR and added to test-base as it is a useful library that could be shared across all modules.

Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ private ServerTuple(final ClusterableServer server, final ServerDescription desc
}
}

AbstractMultiServerCluster(final ClusterId clusterId, final ClusterSettings settings, final ClusterableServerFactory serverFactory) {
super(clusterId, settings, serverFactory);
AbstractMultiServerCluster(final ClusterId clusterId,
final ClusterSettings settings,
final ClusterableServerFactory serverFactory,
final ClientMetadata clientMetadata) {
super(clusterId, settings, serverFactory, clientMetadata);
isTrue("connection mode is multiple", settings.getMode() == MULTIPLE);
clusterType = settings.getRequiredClusterType();
replicaSetName = settings.getRequiredReplicaSetName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.stream.Stream;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;
import java.util.stream.Stream;

import static com.mongodb.assertions.Assertions.assertNotNull;
import static com.mongodb.assertions.Assertions.isTrue;
Expand Down Expand Up @@ -101,26 +101,36 @@ abstract class BaseCluster implements Cluster {
private final ClusterListener clusterListener;
private final Deque<ServerSelectionRequest> waitQueue = new ConcurrentLinkedDeque<>();
private final ClusterClock clusterClock = new ClusterClock();
private final ClientMetadata clientMetadata;
private Thread waitQueueHandler;

private volatile boolean isClosed;
private volatile ClusterDescription description;

BaseCluster(final ClusterId clusterId, final ClusterSettings settings, final ClusterableServerFactory serverFactory) {
BaseCluster(final ClusterId clusterId,
final ClusterSettings settings,
final ClusterableServerFactory serverFactory,
final ClientMetadata clientMetadata) {
this.clusterId = notNull("clusterId", clusterId);
this.settings = notNull("settings", settings);
this.serverFactory = notNull("serverFactory", serverFactory);
this.clusterListener = singleClusterListener(settings);
clusterListener.clusterOpening(new ClusterOpeningEvent(clusterId));
description = new ClusterDescription(settings.getMode(), ClusterType.UNKNOWN, Collections.emptyList(),
this.clusterListener.clusterOpening(new ClusterOpeningEvent(clusterId));
this.description = new ClusterDescription(settings.getMode(), ClusterType.UNKNOWN, Collections.emptyList(),
settings, serverFactory.getSettings());
this.clientMetadata = clientMetadata;
}

@Override
public ClusterClock getClock() {
return clusterClock;
}

@Override
public ClientMetadata getClientMetadata() {
return clientMetadata;
}

@Override
public ServerTuple selectServer(final ServerSelector serverSelector, final OperationContext operationContext) {
isTrue("open", !isClosed());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2008-present MongoDB, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.mongodb.internal.connection;

import com.mongodb.MongoDriverInformation;
import com.mongodb.lang.Nullable;
import org.bson.BsonDocument;

import static com.mongodb.internal.connection.ClientMetadataHelper.createClientMetadataDocument;
import static com.mongodb.internal.connection.ClientMetadataHelper.updateClientMedataDocument;

/**
* Represents metadata of the current MongoClient.
*
* <p>This class is not part of the public API and may be removed or changed at any time</p>
*/
public class ClientMetadata {
private volatile BsonDocument clientMetadataBsonDocument;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't thing that a volatile fields is enough. If append is called concurrently by multiple threads, then one of the updates to metadata will likely be lost.

Copy link
Member Author

@vbabanin vbabanin May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - volatile alone isn't enough for concurrent updates. I initially didn’t add locking because I expected concurrent metadata updates to be extremely rare in practice, but I’ve now added a lock to ensure consistency. Let me know what you think.

UPD: discussed offline. Removed clone() from updateMetadata(...) for clarify: 8ade58b


public ClientMetadata(@Nullable final String applicationName, final MongoDriverInformation mongoDriverInformation) {
this.clientMetadataBsonDocument = createClientMetadataDocument(applicationName, mongoDriverInformation);
}

/**
* Returns mutable BsonDocument that represents the client metadata.
*/
public BsonDocument getBsonDocument() {
return clientMetadataBsonDocument;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return a defensive copy (instead of a reference to this mutable instance?)

Suggested change
return clientMetadataBsonDocument;
return clientMetadataBsonDocument.clone();

Copy link
Member Author

@vbabanin vbabanin May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We mostly pass mutable BsonDocuments through layers (except at the public API). To keep consistency in the codebase, I think it’s fine not to clone here. I’m open to defensive copy as well. Let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine without defensive copy if it's consistent throughout the codebase.

}

public void append(final MongoDriverInformation mongoDriverInformation) {
this.clientMetadataBsonDocument = updateClientMedataDocument(clientMetadataBsonDocument, mongoDriverInformation);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.io.File;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;

Expand Down Expand Up @@ -180,6 +181,38 @@ static boolean clientMetadataDocumentTooLarge(final BsonDocument document) {
return buffer.getPosition() > MAXIMUM_CLIENT_METADATA_ENCODED_SIZE;
}

public static BsonDocument updateClientMedataDocument(final BsonDocument clientMetadataDocument,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Suggested change
public static BsonDocument updateClientMedataDocument(final BsonDocument clientMetadataDocument,
public static BsonDocument updateClientMetadataDocument(final BsonDocument clientMetadataDocument,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Renamed, thanks!

final MongoDriverInformation mongoDriverInformation) {
BsonDocument updatedClientMetadataDocument = clientMetadataDocument.clone();
BsonDocument driverInformation = clientMetadataDocument.getDocument("driver");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the user doesn't supply the driverName when usingMongoDriverInformation.builder(). Should we handle the NPE?

Copy link
Member Author

@vbabanin vbabanin May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean during initial metadata creation or subsequent updates?

For updates:
If the user-supplied MongoDriverInformation doesn’t include driverName, it’s ignored - null values aren’t added to the list returned by getDriverNames(), so we won’t receive null here:

private List<String> prependToList(final List<String> stringList, final String value) {
if (value == null) {
return stringList;
} else {
ArrayList<String> newList = new ArrayList<>();
newList.add(value);
newList.addAll(stringList);
return Collections.unmodifiableList(newList);
}
}
private Builder() {
List<String> immutableEmptyList = Collections.emptyList();
driverInformation = new MongoDriverInformation(immutableEmptyList, immutableEmptyList, immutableEmptyList);
}

For initial metadata creation:
If the user doesn’t specify driverName, we still include one by default to resulting client metadata BsonDocument. The driver name is initialized internally as it is a required field by the spec:

static MongoDriverInformation getDriverInformation(@Nullable final MongoDriverInformation mongoDriverInformation) {
MongoDriverInformation.Builder builder = mongoDriverInformation != null ? MongoDriverInformation.builder(mongoDriverInformation)
: MongoDriverInformation.builder();
return builder
.driverName(MongoDriverVersion.NAME)
.driverVersion(MongoDriverVersion.VERSION)
.driverPlatform(format("Java/%s/%s", getProperty("java.vendor", "unknown-vendor"),
getProperty("java.runtime.version", "unknown-version")))
.build();
}

This is also tested via prose tests:

public static Stream<Arguments> provideDriverInformation() {
return Stream.of(
Arguments.of("1.0", "Framework", "Framework Platform"),
Arguments.of("1.0", "Framework", null),
Arguments.of(null, "Framework", "Framework Platform"),
Arguments.of(null, "Framework", null)
);
}


List<String> driverNamesToAppend = mongoDriverInformation.getDriverNames();
List<String> driverVersionsToAppend = mongoDriverInformation.getDriverVersions();
List<String> driverPlatformsToAppend = mongoDriverInformation.getDriverPlatforms();

List<String> updatedDriverNames = new ArrayList<>(driverNamesToAppend.size() + 1);
List<String> updatedDriverVersions = new ArrayList<>(driverVersionsToAppend.size() + 1);
List<String> updateDriverPlatforms = new ArrayList<>(driverPlatformsToAppend.size() + 1);

updatedDriverNames.add(driverInformation.getString("name").getValue());
updatedDriverNames.addAll(driverNamesToAppend);

updatedDriverVersions.add(driverInformation.getString("version").getValue());
updatedDriverVersions.addAll(driverVersionsToAppend);

updateDriverPlatforms.add(clientMetadataDocument.getString("platform").getValue());
updateDriverPlatforms.addAll(driverPlatformsToAppend);

tryWithLimit(updatedClientMetadataDocument, d -> {
putAtPath(d, "driver.name", listToString(updatedDriverNames));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tryWithLimit invokes the Consumer<BsonDocument> lambda twice, we could build the appended string outside the tryWithLimit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point. In this case, I’d choose to follow the existing pattern in ClientMetadataHelper to keep the code consistent and neat, as this pattern exists elsewhere in the class. Since updateMetadata is expected to be called most likely once per application lifecycle and isn’t on a hot path or GC-critical, I’d prefer to keep it as is for now. Let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for consistency

putAtPath(d, "driver.version", listToString(updatedDriverVersions));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we try in a dedicated tryWithLimit?

Copy link
Member Author

@vbabanin vbabanin May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the spec, driver name and version must be updated atomically. If both can't fit, then both must be omitted - so splitting them in a tryWithLimit wouldn't comply with that requirement. We follow the same approach during initial metadata creation:

tryWithLimit(client, d -> {
putAtPath(d, "driver.name", listToString(fullDriverInfo.getDriverNames()));
putAtPath(d, "driver.version", listToString(fullDriverInfo.getDriverVersions()));
});

});
tryWithLimit(updatedClientMetadataDocument, d -> {
putAtPath(d, "platform", listToString(updateDriverPlatforms));
});
return updatedClientMetadataDocument;
}

public enum ContainerRuntime {
DOCKER("docker") {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public interface Cluster extends Closeable {
*/
ClusterClock getClock();

ClientMetadata getClientMetadata();

ServerTuple selectServer(ServerSelector serverSelector, OperationContext operationContext);

void selectServerAsync(ServerSelector serverSelector, OperationContext operationContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,27 +107,29 @@ public Cluster createCluster(final ClusterSettings originalClusterSettings, fina
InternalOperationContextFactory heartBeatOperationContextFactory =
new InternalOperationContextFactory(heartbeatTimeoutSettings, serverApi);

ClientMetadata clientMetadata = new ClientMetadata(
applicationName,
mongoDriverInformation != null ? mongoDriverInformation : MongoDriverInformation.builder().build());

if (clusterSettings.getMode() == ClusterConnectionMode.LOAD_BALANCED) {
ClusterableServerFactory serverFactory = new LoadBalancedClusterableServerFactory(serverSettings,
connectionPoolSettings, internalConnectionPoolSettings, streamFactory, credential, loggerSettings, commandListener,
applicationName, mongoDriverInformation != null ? mongoDriverInformation : MongoDriverInformation.builder().build(),
compressorList, serverApi, clusterOperationContextFactory);
return new LoadBalancedCluster(clusterId, clusterSettings, serverFactory, dnsSrvRecordMonitorFactory);
return new LoadBalancedCluster(clusterId, clusterSettings, serverFactory, clientMetadata, dnsSrvRecordMonitorFactory);
} else {
ClusterableServerFactory serverFactory = new DefaultClusterableServerFactory(serverSettings,
connectionPoolSettings, internalConnectionPoolSettings,
clusterOperationContextFactory, streamFactory, heartBeatOperationContextFactory, heartbeatStreamFactory, credential,
loggerSettings, commandListener, applicationName,
mongoDriverInformation != null ? mongoDriverInformation : MongoDriverInformation.builder().build(), compressorList,
loggerSettings, commandListener, compressorList,
serverApi, FaasEnvironment.getFaasEnvironment() != FaasEnvironment.UNKNOWN);

if (clusterSettings.getMode() == ClusterConnectionMode.SINGLE) {
return new SingleServerCluster(clusterId, clusterSettings, serverFactory);
return new SingleServerCluster(clusterId, clusterSettings, serverFactory, clientMetadata);
} else if (clusterSettings.getMode() == ClusterConnectionMode.MULTIPLE) {
if (clusterSettings.getSrvHost() == null) {
return new MultiServerCluster(clusterId, clusterSettings, serverFactory);
return new MultiServerCluster(clusterId, clusterSettings, serverFactory, clientMetadata);
} else {
return new DnsMultiServerCluster(clusterId, clusterSettings, serverFactory, dnsSrvRecordMonitorFactory);
return new DnsMultiServerCluster(clusterId, clusterSettings, serverFactory, clientMetadata, dnsSrvRecordMonitorFactory);
}
} else {
throw new UnsupportedOperationException("Unsupported cluster mode: " + clusterSettings.getMode());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.mongodb.LoggerSettings;
import com.mongodb.MongoCompressor;
import com.mongodb.MongoCredential;
import com.mongodb.MongoDriverInformation;
import com.mongodb.ServerAddress;
import com.mongodb.ServerApi;
import com.mongodb.connection.ClusterConnectionMode;
Expand Down Expand Up @@ -50,8 +49,6 @@ public class DefaultClusterableServerFactory implements ClusterableServerFactory
private final MongoCredentialWithCache credential;
private final LoggerSettings loggerSettings;
private final CommandListener commandListener;
private final String applicationName;
private final MongoDriverInformation mongoDriverInformation;
private final List<MongoCompressor> compressorList;
@Nullable
private final ServerApi serverApi;
Expand All @@ -63,8 +60,7 @@ public DefaultClusterableServerFactory(
final InternalOperationContextFactory clusterOperationContextFactory, final StreamFactory streamFactory,
final InternalOperationContextFactory heartbeatOperationContextFactory, final StreamFactory heartbeatStreamFactory,
@Nullable final MongoCredential credential, final LoggerSettings loggerSettings,
@Nullable final CommandListener commandListener, @Nullable final String applicationName,
@Nullable final MongoDriverInformation mongoDriverInformation,
@Nullable final CommandListener commandListener,
final List<MongoCompressor> compressorList, @Nullable final ServerApi serverApi, final boolean isFunctionAsAServiceEnvironment) {
this.serverSettings = serverSettings;
this.connectionPoolSettings = connectionPoolSettings;
Expand All @@ -76,8 +72,6 @@ public DefaultClusterableServerFactory(
this.credential = credential == null ? null : new MongoCredentialWithCache(credential);
this.loggerSettings = loggerSettings;
this.commandListener = commandListener;
this.applicationName = applicationName;
this.mongoDriverInformation = mongoDriverInformation;
this.compressorList = compressorList;
this.serverApi = serverApi;
this.isFunctionAsAServiceEnvironment = isFunctionAsAServiceEnvironment;
Expand All @@ -88,15 +82,17 @@ public ClusterableServer create(final Cluster cluster, final ServerAddress serve
ServerId serverId = new ServerId(cluster.getClusterId(), serverAddress);
ClusterConnectionMode clusterMode = cluster.getSettings().getMode();
SameObjectProvider<SdamServerDescriptionManager> sdamProvider = SameObjectProvider.uninitialized();
ClientMetadata clientMetadata = cluster.getClientMetadata();

ServerMonitor serverMonitor = new DefaultServerMonitor(serverId, serverSettings,
// no credentials, compressor list, or command listener for the server monitor factory
new InternalStreamConnectionFactory(clusterMode, true, heartbeatStreamFactory, null, applicationName,
mongoDriverInformation, emptyList(), loggerSettings, null, serverApi),
new InternalStreamConnectionFactory(clusterMode, true, heartbeatStreamFactory, null, clientMetadata,
emptyList(), loggerSettings, null, serverApi),
clusterMode, serverApi, isFunctionAsAServiceEnvironment, sdamProvider, heartbeatOperationContextFactory);

ConnectionPool connectionPool = new DefaultConnectionPool(serverId,
new InternalStreamConnectionFactory(clusterMode, streamFactory, credential, applicationName,
mongoDriverInformation, compressorList, loggerSettings, commandListener, serverApi),
new InternalStreamConnectionFactory(clusterMode, streamFactory, credential, clientMetadata,
compressorList, loggerSettings, commandListener, serverApi),
connectionPoolSettings, internalConnectionPoolSettings, sdamProvider, clusterOperationContextFactory);
ServerListener serverListener = singleServerListener(serverSettings);
SdamServerDescriptionManager sdam = new DefaultSdamServerDescriptionManager(cluster, serverId, serverListener, serverMonitor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ public final class DnsMultiServerCluster extends AbstractMultiServerCluster {
private final DnsSrvRecordMonitor dnsSrvRecordMonitor;
private volatile MongoException srvResolutionException;

public DnsMultiServerCluster(final ClusterId clusterId, final ClusterSettings settings, final ClusterableServerFactory serverFactory,
public DnsMultiServerCluster(final ClusterId clusterId, final ClusterSettings settings,
final ClusterableServerFactory serverFactory,
final ClientMetadata clientMetadata,
final DnsSrvRecordMonitorFactory dnsSrvRecordMonitorFactory) {
super(clusterId, settings, serverFactory);
super(clusterId, settings, serverFactory, clientMetadata);
dnsSrvRecordMonitor = dnsSrvRecordMonitorFactory.create(assertNotNull(settings.getSrvHost()), settings.getSrvServiceName(),
new DnsSrvRecordInitializer() {
private volatile boolean initialized;
Expand Down
Loading