From ef6fbbdab4846b621a8be8b9c72feba7270a819c Mon Sep 17 00:00:00 2001 From: Adrian Cole <64215+codefromthecrypt@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:10:44 -1000 Subject: [PATCH] Adds support for org.mongodb:mongodb-driver-core v5 (#1431) Signed-off-by: Adrian Cole --- instrumentation/mongodb/RATIONALE.md | 8 ++ instrumentation/mongodb/pom.xml | 51 +++++++- .../mongodb/src/it/mongodb_floor/README.md | 2 + .../mongodb/src/it/mongodb_floor/pom.xml | 120 ++++++++++++++++++ .../brave/mongodb_v3/ITMongoDBTracing.java | 17 +++ .../java/brave/mongodb/MongoDBDriver.java | 112 ++++++++++++++++ .../mongodb/TraceMongoCommandListener.java | 12 +- .../java/brave/mongodb/ITMongoDBTracing.java | 50 +++++--- .../java/brave/mongodb/MongoDBContainer.java | 3 +- .../java/brave/mongodb/MongoDBDriverTest.java | 39 ++++++ .../TraceMongoCommandListenerTest.java | 36 ++---- .../servlet/internal/ServletRuntime.java | 2 +- 12 files changed, 393 insertions(+), 59 deletions(-) create mode 100644 instrumentation/mongodb/src/it/mongodb_floor/README.md create mode 100644 instrumentation/mongodb/src/it/mongodb_floor/pom.xml create mode 100644 instrumentation/mongodb/src/it/mongodb_floor/src/test/java/brave/mongodb_v3/ITMongoDBTracing.java create mode 100644 instrumentation/mongodb/src/main/java/brave/mongodb/MongoDBDriver.java create mode 100644 instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBDriverTest.java diff --git a/instrumentation/mongodb/RATIONALE.md b/instrumentation/mongodb/RATIONALE.md index 9a68c39a58..85caadf777 100644 --- a/instrumentation/mongodb/RATIONALE.md +++ b/instrumentation/mongodb/RATIONALE.md @@ -1,5 +1,13 @@ # brave-instrumentation-mongodb rationale +## Floor JRE version + +MongoDB client 3.x has a floor JRE version of 1.6, while 4.x moved to 8. We +test on MongoDB client 3.x, despite it being 1.6 bytecode. This is a +maintenance compromise beginning with MongoDB 5.x. We believe this will be less +significant than more widely used libraries, such as Servlet or OkHttp, which +could be used in old environments or Android. + ## Default data policy We tried to make the default data policy similar to other instrumentation, such as MySQL, and also current practice from existing sites. Like other instrumentation, the policy is intentionally conservative, in efforts to avoid large diff --git a/instrumentation/mongodb/pom.xml b/instrumentation/mongodb/pom.xml index 37fd5d43c9..04811e3778 100644 --- a/instrumentation/mongodb/pom.xml +++ b/instrumentation/mongodb/pom.xml @@ -31,16 +31,23 @@ ${project.basedir}/../.. - 3.12.14 + 5.0.1 + 3.11.0 org.mongodb - mongodb-driver + mongodb-driver-core ${mongodb-driver.version} provided + + org.mongodb + mongodb-driver-sync + ${mongodb-driver.version} + test + ${project.groupId} brave-tests @@ -55,14 +62,46 @@ + + + + maven-invoker-plugin + + + de.qaware.maven + go-offline-maven-plugin + + + package + + resolve-dependencies + + + + + + + + org.mongodb + mongodb-driver + ${floor-mongodb-driver.version} + MAIN + jar + + + + + + + release - - 1.6 - 1.6 - 6 + + 1.7 + 1.7 + 7 diff --git a/instrumentation/mongodb/src/it/mongodb_floor/README.md b/instrumentation/mongodb/src/it/mongodb_floor/README.md new file mode 100644 index 0000000000..ba9fc9ec09 --- /dev/null +++ b/instrumentation/mongodb/src/it/mongodb_floor/README.md @@ -0,0 +1,2 @@ +# mongodb_floor +This tests that MongoDBTracing can be used with mongodb <5 diff --git a/instrumentation/mongodb/src/it/mongodb_floor/pom.xml b/instrumentation/mongodb/src/it/mongodb_floor/pom.xml new file mode 100644 index 0000000000..2a69bd8b11 --- /dev/null +++ b/instrumentation/mongodb/src/it/mongodb_floor/pom.xml @@ -0,0 +1,120 @@ + + + + 4.0.0 + + @project.groupId@ + mongodb_floor + @project.version@ + mongodb_floor + + + UTF-8 + UTF-8 + + 1.8 + 1.8 + + + + + org.mongodb + mongodb-driver + @floor-mongodb-driver.version@ + provided + + + + @project.groupId@ + brave-instrumentation-mongodb + @project.version@ + + + + @project.groupId@ + brave-tests + @project.version@ + + + + org.junit.jupiter + junit-jupiter + @junit-jupiter.version@ + + + org.mockito + mockito-junit-jupiter + @mockito.version@ + + + + org.apache.logging.log4j + log4j-slf4j-impl + @log4j.version@ + + + org.apache.logging.log4j + log4j-core + @log4j.version@ + + + org.testcontainers + junit-jupiter + @testcontainers.version@ + + + + + @project.build.testSourceDirectory@ + + + @project.basedir@/src/test/resources + + + + + maven-compiler-plugin + @maven-compiler-plugin.version@ + + + **/IT*.java + + + + + + maven-surefire-plugin + @maven-surefire-plugin.version@ + + true + + **/IT*.java + + + false + + false + + false + + + + + diff --git a/instrumentation/mongodb/src/it/mongodb_floor/src/test/java/brave/mongodb_v3/ITMongoDBTracing.java b/instrumentation/mongodb/src/it/mongodb_floor/src/test/java/brave/mongodb_v3/ITMongoDBTracing.java new file mode 100644 index 0000000000..02e4c33fcd --- /dev/null +++ b/instrumentation/mongodb/src/it/mongodb_floor/src/test/java/brave/mongodb_v3/ITMongoDBTracing.java @@ -0,0 +1,17 @@ +/* + * Copyright 2013-2024 The OpenZipkin Authors + * + * 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 brave.mongodb_floor; + +class ITMongoDBTracing extends brave.mongodb.ITMongoDBTracing { +} diff --git a/instrumentation/mongodb/src/main/java/brave/mongodb/MongoDBDriver.java b/instrumentation/mongodb/src/main/java/brave/mongodb/MongoDBDriver.java new file mode 100644 index 0000000000..0b23b69909 --- /dev/null +++ b/instrumentation/mongodb/src/main/java/brave/mongodb/MongoDBDriver.java @@ -0,0 +1,112 @@ +/* + * Copyright 2013-2024 The OpenZipkin Authors + * + * 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 brave.mongodb; + +import brave.Span; +import com.mongodb.ServerAddress; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.net.InetSocketAddress; + +import static brave.internal.Throwables.propagateIfFatal; + +/** + * Access to MongoDB version-specific features + * + *

Originally designed by OkHttp team, derived from {@code okhttp3.internal.platform.Platform} + */ +class MongoDBDriver { + private static final MongoDBDriver MONGO_DB_DRIVER = findMongoDBDriver(); + + /** adds the remote IP and port to the span */ + void setRemoteIpAndPort(Span span, ServerAddress address) { + // default to no-op instead of crash on future drift + } + + MongoDBDriver() { + } + + public static MongoDBDriver get() { + return MONGO_DB_DRIVER; + } + + /** + * Attempt to match the driver version from two known patterns: + * + *

    + *
  1. org.mongodb:mongodb-driver - 3.x
    2. + *
  2. org.mongodb:mongodb-driver-core - 5.x
    3. + *
+ */ + private static MongoDBDriver findMongoDBDriver() { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + try { + MethodHandle getHost = + lookup.findVirtual(ServerAddress.class, "getHost", MethodType.methodType(String.class)); + MethodHandle getPort = + lookup.findVirtual(ServerAddress.class, "getPort", MethodType.methodType(int.class)); + return new Driver5x(getHost, getPort); + } catch (NoSuchMethodException | IllegalAccessException e) { + // not 5.x + } + try { + MethodHandle getSocketAddress = lookup.findVirtual(ServerAddress.class, "getSocketAddress", + MethodType.methodType(InetSocketAddress.class)); + return new Driver3x(getSocketAddress); + } catch (NoSuchMethodException | IllegalAccessException e) { + // unknown version + } + + // Unknown + return new MongoDBDriver(); + } + + static final class Driver3x extends MongoDBDriver { + final MethodHandle getSocketAddress; + + Driver3x(MethodHandle getSocketAddress) { + this.getSocketAddress = getSocketAddress; + } + + @Override void setRemoteIpAndPort(Span span, ServerAddress serverAddress) { + try { + InetSocketAddress socketAddress = + (InetSocketAddress) getSocketAddress.invokeExact(serverAddress); + span.remoteIpAndPort(socketAddress.getAddress().getHostAddress(), socketAddress.getPort()); + } catch (Throwable t) { + propagateIfFatal(t); + } + } + } + + static final class Driver5x extends MongoDBDriver { + final MethodHandle getHost, getPort; + + Driver5x(MethodHandle getHost, MethodHandle getPort) { + this.getHost = getHost; + this.getPort = getPort; + } + + @Override void setRemoteIpAndPort(Span span, ServerAddress serverAddress) { + try { + String host = (String) getHost.invokeExact(serverAddress); + int port = (int) getPort.invokeExact(serverAddress); + span.remoteIpAndPort(host, port); + } catch (Throwable t) { + propagateIfFatal(t); + } + } + } +} diff --git a/instrumentation/mongodb/src/main/java/brave/mongodb/TraceMongoCommandListener.java b/instrumentation/mongodb/src/main/java/brave/mongodb/TraceMongoCommandListener.java index 51e6adfc0d..a7d9b6d070 100644 --- a/instrumentation/mongodb/src/main/java/brave/mongodb/TraceMongoCommandListener.java +++ b/instrumentation/mongodb/src/main/java/brave/mongodb/TraceMongoCommandListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2023 The OpenZipkin Authors + * Copyright 2013-2024 The OpenZipkin Authors * * 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 @@ -35,7 +35,7 @@ /** * A MongoDB command listener that will report via Brave how long each command takes and other * information about the commands. - * + *

* See RATIONALE.md * for implementation notes. */ @@ -87,13 +87,7 @@ final class TraceMongoCommandListener implements CommandListener { span.tag("mongodb.cluster_id", connectionId.getServerId().getClusterId().getValue()); } - try { - InetSocketAddress socketAddress = - connectionDescription.getServerAddress().getSocketAddress(); - span.remoteIpAndPort(socketAddress.getAddress().getHostAddress(), socketAddress.getPort()); - } catch (MongoSocketException ignored) { - - } + MongoDBDriver.get().setRemoteIpAndPort(span, connectionDescription.getServerAddress()); } span.start(); diff --git a/instrumentation/mongodb/src/test/java/brave/mongodb/ITMongoDBTracing.java b/instrumentation/mongodb/src/test/java/brave/mongodb/ITMongoDBTracing.java index 08759fc99a..59584c224e 100644 --- a/instrumentation/mongodb/src/test/java/brave/mongodb/ITMongoDBTracing.java +++ b/instrumentation/mongodb/src/test/java/brave/mongodb/ITMongoDBTracing.java @@ -28,7 +28,7 @@ import com.mongodb.client.internal.MongoClientImpl; import com.mongodb.connection.ClusterId; import com.mongodb.event.CommandListener; -import java.lang.reflect.Field; +import java.lang.invoke.VarHandle; import org.bson.BsonDocument; import org.bson.BsonString; import org.bson.Document; @@ -41,6 +41,9 @@ import org.testcontainers.junit.jupiter.Testcontainers; import static brave.Span.Kind.CLIENT; +import static java.lang.invoke.MethodHandles.Lookup; +import static java.lang.invoke.MethodHandles.lookup; +import static java.lang.invoke.MethodHandles.privateLookupIn; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.entry; @@ -49,7 +52,7 @@ @Tag("docker") @Testcontainers(disabledWithoutDocker = true) @Timeout(60) -class ITMongoDBTracing extends ITRemote { +public class ITMongoDBTracing extends ITRemote { // public for invoker test static final String DATABASE_NAME = "myDatabase"; static final String COLLECTION_NAME = "myCollection"; static final String INVALID_COLLECTION_NAME = "?.$"; @@ -72,13 +75,13 @@ class ITMongoDBTracing extends ITRemote { } @BeforeEach void getClusterId() throws Exception { - // TODO: Figure out an easier way to get this! - Field clusterIdField = - Class.forName("com.mongodb.internal.connection.BaseCluster").getDeclaredField("clusterId"); - - clusterIdField.setAccessible(true); - ClusterId clusterId = - (ClusterId) clusterIdField.get(((MongoClientImpl) mongoClient).getCluster()); + // Object because the type changed between 3.x and 5.x + Object cluster = ((MongoClientImpl) mongoClient).getCluster(); + // Before 5.x, there is no public API to get the clusterId. + Class clazz = Class.forName("com.mongodb.internal.connection.BaseCluster"); + Lookup lookup = privateLookupIn(clazz, lookup()); + VarHandle clusterIdHandle = lookup.findVarHandle(clazz, "clusterId", ClusterId.class); + ClusterId clusterId = (ClusterId) clusterIdHandle.get(cluster); this.clusterId = clusterId.getValue(); } @@ -108,11 +111,12 @@ class ITMongoDBTracing extends ITRemote { mongoCursor.next(); // Name extracted from {"find": "myCollection"} - assertThat(testSpanHandler.takeRemoteSpan(CLIENT).name()).isEqualTo("find " + COLLECTION_NAME); + assertThat(testSpanHandler.takeRemoteSpan(CLIENT).name()) + .isEqualTo("find " + COLLECTION_NAME); // Name extracted from {"getMore": , "collection": "myCollection"} - assertThat(testSpanHandler.takeRemoteSpan(CLIENT).name()).isEqualTo( - "getMore " + COLLECTION_NAME); + assertThat(testSpanHandler.takeRemoteSpan(CLIENT).name()) + .isEqualTo("getMore " + COLLECTION_NAME); } /** @@ -143,8 +147,10 @@ class ITMongoDBTracing extends ITRemote { executeFind(COLLECTION_NAME); assertThat(testSpanHandler.takeRemoteSpan(CLIENT).tags()).containsOnly( - entry("mongodb.collection", COLLECTION_NAME), entry("mongodb.command", "find"), - entry("mongodb.cluster_id", clusterId)); + entry("mongodb.collection", COLLECTION_NAME), + entry("mongodb.command", "find"), + entry("mongodb.cluster_id", clusterId) + ); } @Test void addsTagsForLargePayloadCommand() { @@ -155,20 +161,24 @@ class ITMongoDBTracing extends ITRemote { database.getCollection("largeCollection").insertOne(largeDocument); assertThat(testSpanHandler.takeRemoteSpan(CLIENT).tags()).containsOnly( - entry("mongodb.collection", "largeCollection"), entry("mongodb.command", "insert"), - entry("mongodb.cluster_id", clusterId)); + entry("mongodb.collection", "largeCollection"), + entry("mongodb.command", "insert"), + entry("mongodb.cluster_id", clusterId) + ); } @Test void reportsServerAddress() { executeFind(COLLECTION_NAME); - assertThat(testSpanHandler.takeRemoteSpan(CLIENT).remoteServiceName()).isEqualTo( - "mongodb-" + DATABASE_NAME); + MutableSpan span = testSpanHandler.takeRemoteSpan(CLIENT); + assertThat(span.remoteServiceName()).isEqualTo("mongodb-" + DATABASE_NAME); + assertThat(span.remoteIp()).isEqualTo(mongo.host()); + assertThat(span.remotePort()).isEqualTo(mongo.port()); } @Test void setsError() { - assertThatThrownBy(() -> executeFind(INVALID_COLLECTION_NAME)).isInstanceOf( - MongoQueryException.class); + assertThatThrownBy(() -> executeFind(INVALID_COLLECTION_NAME)) + .isInstanceOf(MongoQueryException.class); // the error the interceptor receives is a MongoCommandException! testSpanHandler.takeRemoteSpanWithErrorMessage(CLIENT, ".*InvalidNamespace.*"); diff --git a/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBContainer.java b/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBContainer.java index d5c4556f38..237f20643e 100644 --- a/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBContainer.java +++ b/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBContainer.java @@ -66,8 +66,9 @@ MongoClientSettings.Builder mongoClientSettingsBuilder() { new ServerAddress(host(), port())))); } + /** Return an IP address to ensure remote IP checks work. */ String host() { - return getHost(); + return "127.0.0.1"; } int port() { diff --git a/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBDriverTest.java b/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBDriverTest.java new file mode 100644 index 0000000000..2bc19a1006 --- /dev/null +++ b/instrumentation/mongodb/src/test/java/brave/mongodb/MongoDBDriverTest.java @@ -0,0 +1,39 @@ +/* + * Copyright 2013-2024 The OpenZipkin Authors + * + * 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 brave.mongodb; + +import brave.Span; +import com.mongodb.ServerAddress; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class MongoDBDriverTest { + @Mock ServerAddress serverAddress; + @Mock Span span; + + @Test void setRemoteIpAndPort() { + when(serverAddress.getHost()).thenReturn("127.0.0.1"); + when(serverAddress.getPort()).thenReturn(27017); + + MongoDBDriver.get().setRemoteIpAndPort(span, serverAddress); + + verify(span).remoteIpAndPort("127.0.0.1", 27017); + } +} diff --git a/instrumentation/mongodb/src/test/java/brave/mongodb/TraceMongoCommandListenerTest.java b/instrumentation/mongodb/src/test/java/brave/mongodb/TraceMongoCommandListenerTest.java index 982873377f..3a818dff56 100644 --- a/instrumentation/mongodb/src/test/java/brave/mongodb/TraceMongoCommandListenerTest.java +++ b/instrumentation/mongodb/src/test/java/brave/mongodb/TraceMongoCommandListenerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2023 The OpenZipkin Authors + * Copyright 2013-2024 The OpenZipkin Authors * * 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 @@ -38,6 +38,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -225,33 +227,23 @@ void verifyCommandStartedMocks() { } CommandStartedEvent createCommandStartedEvent() { - return new CommandStartedEvent( - 1, - createConnectionDescription(), - "dbName", - "insert", - LONG_COMMAND - ); + CommandStartedEvent event = mock(CommandStartedEvent.class); + lenient().when(event.getRequestId()).thenReturn(1); + lenient().when(event.getConnectionDescription()).thenReturn(createConnectionDescription()); + when(event.getDatabaseName()).thenReturn("dbName"); + lenient().when(event.getCommandName()).thenReturn("insert"); + lenient().when(event.getCommand()).thenReturn(LONG_COMMAND); + return event; } CommandSucceededEvent createCommandSucceededEvent() { - return new CommandSucceededEvent( - 1, - createConnectionDescription(), - "insert", - new BsonDocument(), - 1000 - ); + return mock(CommandSucceededEvent.class); } CommandFailedEvent createCommandFailedEvent(Throwable throwable) { - return new CommandFailedEvent( - 1, - createConnectionDescription(), - "insert", - 2000, - throwable - ); + CommandFailedEvent event = mock(CommandFailedEvent.class); + lenient().when(event.getThrowable()).thenReturn(throwable); + return event; } ConnectionDescription createConnectionDescription() { diff --git a/instrumentation/servlet/src/main/java/brave/servlet/internal/ServletRuntime.java b/instrumentation/servlet/src/main/java/brave/servlet/internal/ServletRuntime.java index 266c456d3f..009e76e3df 100644 --- a/instrumentation/servlet/src/main/java/brave/servlet/internal/ServletRuntime.java +++ b/instrumentation/servlet/src/main/java/brave/servlet/internal/ServletRuntime.java @@ -69,7 +69,7 @@ private static ServletRuntime findServletRuntime() { try { Class.forName("javax.servlet.AsyncEvent"); HttpServletRequest.class.getMethod("isAsyncStarted"); - return new Servlet3(); // intentionally doesn't not access the type prior to the above guard + return new Servlet3(); // intentionally doesn't access the type prior to the above guard } catch (NoSuchMethodException e) { // pre Servlet v3 } catch (ClassNotFoundException e) {