From 1936a739b3f23b66dccef8888027f4f76237964b Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Thu, 21 Mar 2024 13:43:57 +0000 Subject: [PATCH 1/3] use a dummy user for testing --- .../main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java index 6853a14088..7f249900d5 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java @@ -181,7 +181,7 @@ private ConnectionMetaData getConnectionMetaData(@Nullable Connection connection DatabaseMetaData metaData = connection.getMetaData(); connectionMetaData = ConnectionMetaData.parse(metaData.getURL()) .withConnectionInstance(safeGetCatalog(connection)) - .withConnectionUser(metaData.getUserName()) + .withConnectionUser("dummyuser") .build(); if (logger.isDebugEnabled()) { From d4b43dcb186c73f88b0eb056aa5282f26bd92e69 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Tue, 2 Apr 2024 09:43:22 +0100 Subject: [PATCH 2/3] exclusion list option for calling getUserName --- .../agent/jdbc/helper/JdbcConfiguration.java | 52 ++++++++++ .../apm/agent/jdbc/helper/JdbcHelper.java | 21 ++++- ....configuration.ConfigurationOptionProvider | 1 + .../helper/JdbcGetUserNameExclusionTest.java | 94 +++++++++++++++++++ 4 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/resources/META-INF/services/org.stagemonitor.configuration.ConfigurationOptionProvider create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java new file mode 100644 index 0000000000..5ee30dacce --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java @@ -0,0 +1,52 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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 co.elastic.apm.agent.jdbc.helper; + +import co.elastic.apm.agent.common.util.WildcardMatcher; +import co.elastic.apm.agent.tracer.configuration.WildcardMatcherValueConverter; +import org.stagemonitor.configuration.ConfigurationOption; +import org.stagemonitor.configuration.ConfigurationOptionProvider; +import org.stagemonitor.configuration.converter.DoubleValueConverter; +import org.stagemonitor.configuration.converter.ListValueConverter; +import org.stagemonitor.configuration.converter.StringValueConverter; + +import java.util.Arrays; +import java.util.List; + +public class JdbcConfiguration extends ConfigurationOptionProvider { + + private final ConfigurationOption> databaseMetaDataExclusionList = ConfigurationOption + .builder(new ListValueConverter(StringValueConverter.INSTANCE), List.class) + .key("exclude_from_getting_username") + .configurationCategory("Datastore") + .description("If any of these strings match part of the package or class name of the DatabaseMetaData instance, getUserName() won't be called" + + "\n" + + WildcardMatcher.DOCUMENTATION + ) + .tags("internal") + .dynamic(true) + .buildWithDefault(Arrays.asList( + "hikari" + )); + + public List getDatabaseMetaDataExclusionList() { + return databaseMetaDataExclusionList.get(); + } + +} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java index 7f249900d5..ca8f379e40 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java @@ -21,18 +21,21 @@ import co.elastic.apm.agent.sdk.internal.db.signature.Scanner; import co.elastic.apm.agent.sdk.internal.db.signature.SignatureParser; import co.elastic.apm.agent.tracer.AbstractSpan; +import co.elastic.apm.agent.tracer.GlobalTracer; import co.elastic.apm.agent.tracer.Span; import co.elastic.apm.agent.tracer.ElasticContext; import co.elastic.apm.agent.jdbc.JdbcFilter; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap; +import co.elastic.apm.agent.tracer.Tracer; import javax.annotation.Nullable; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.SQLException; import java.sql.Statement; +import java.util.List; import java.util.concurrent.Callable; import static co.elastic.apm.agent.jdbc.helper.JdbcGlobalState.metaDataMap; @@ -45,6 +48,11 @@ public class JdbcHelper { public static final String DB_SPAN_ACTION = "query"; private static final JdbcHelper INSTANCE = new JdbcHelper(); + private final JdbcConfiguration config; + + public JdbcHelper() { + this.config = GlobalTracer.get().getConfig(JdbcConfiguration.class); + } public static JdbcHelper get() { return INSTANCE; @@ -181,7 +189,7 @@ private ConnectionMetaData getConnectionMetaData(@Nullable Connection connection DatabaseMetaData metaData = connection.getMetaData(); connectionMetaData = ConnectionMetaData.parse(metaData.getURL()) .withConnectionInstance(safeGetCatalog(connection)) - .withConnectionUser("dummyuser") + .withConnectionUser(maybeGetUserName(metaData, config)) .build(); if (logger.isDebugEnabled()) { @@ -201,6 +209,17 @@ private ConnectionMetaData getConnectionMetaData(@Nullable Connection connection return connectionMetaData; } + static String maybeGetUserName(DatabaseMetaData metaData, JdbcConfiguration config) throws SQLException { + List exclusionList = config.getDatabaseMetaDataExclusionList(); + String classname = metaData.getClass().getName(); + for (String exclude : exclusionList) { + if (classname.contains(exclude)) { + return null; + } + } + return metaData.getUserName(); + } + @Nullable private String safeGetCatalog(Connection connection) { String catalog = null; diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/resources/META-INF/services/org.stagemonitor.configuration.ConfigurationOptionProvider b/apm-agent-plugins/apm-jdbc-plugin/src/main/resources/META-INF/services/org.stagemonitor.configuration.ConfigurationOptionProvider new file mode 100644 index 0000000000..38d62b1c55 --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/resources/META-INF/services/org.stagemonitor.configuration.ConfigurationOptionProvider @@ -0,0 +1 @@ +co.elastic.apm.agent.jdbc.helper.JdbcConfiguration diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java new file mode 100644 index 0000000000..2dde8be37f --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java @@ -0,0 +1,94 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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 co.elastic.apm.agent.jdbc.helper; + +import co.elastic.apm.agent.MockTracer; +import co.elastic.apm.agent.bci.ElasticApmAgent; +import co.elastic.apm.agent.configuration.SpyConfiguration; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import net.bytebuddy.agent.ByteBuddyAgent; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.stagemonitor.configuration.ConfigurationRegistry; + +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.sql.DatabaseMetaData; +import java.sql.SQLException; +import java.util.List; + +import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat; +import static org.mockito.Mockito.doReturn; + +public class JdbcGetUserNameExclusionTest { + + protected static ElasticApmTracer tracer; + protected static ConfigurationRegistry config; + protected static JdbcConfiguration jdbcconfig; + + @BeforeAll + @BeforeClass + public static synchronized void beforeAll() { + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); + tracer = mockInstrumentationSetup.getTracer(); + config = mockInstrumentationSetup.getConfig(); + assertThat(tracer.isRunning()).isTrue(); + ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install()); + } + + @Test + public void hasUsernameCorrectlyExcludes() throws SQLException { + DatabaseMetaData meta = (DatabaseMetaData) Proxy.newProxyInstance( + this.getClass().getClassLoader(), + new Class[] { DatabaseMetaData.class }, + new MetadataInvocationHandler()); + + assertThat(JdbcHelper.maybeGetUserName(meta, config.getConfig(JdbcConfiguration.class))).isEqualTo("testuser"); + + String classname = meta.getClass().getName(); + String excludeName = classname.substring(classname.indexOf('$')+1); + doReturn(List.of(excludeName)) + .when(config.getConfig(JdbcConfiguration.class)) + .getDatabaseMetaDataExclusionList(); + + assertThat(JdbcHelper.maybeGetUserName(meta, config.getConfig(JdbcConfiguration.class))).isEqualTo(null); + } + + @After + @AfterEach + public final void cleanUp() { + SpyConfiguration.reset(config); + } + + public class MetadataInvocationHandler implements InvocationHandler { + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if (method.getName().equals("getUserName")) { + return "testuser"; + } + return null; + } + } + +} From 9193fba5235efecd1b53f621e84bf03bb10b37f6 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Tue, 2 Apr 2024 10:10:11 +0100 Subject: [PATCH 3/3] changelog and test break fixed --- CHANGELOG.asciidoc | 1 + .../agent/jdbc/helper/JdbcConfiguration.java | 2 +- .../helper/JdbcGetUserNameExclusionTest.java | 21 ++----------------- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 35349f1556..6b92fe1bdb 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -39,6 +39,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: ===== Features * Differentiate Lambda URLs from API Gateway in AWS Lambda integration - {pull}3417[#3417] * Added lambda support for ELB triggers {pull}#3411[#3411] +* Add exclusion list option for calling DatabaseMetaData.getUserName - {pull}#3568[#3568] [[release-notes-1.x]] === Java Agent version 1.x diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java index 5ee30dacce..87dcfb976d 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcConfiguration.java @@ -39,7 +39,7 @@ public class JdbcConfiguration extends ConfigurationOptionProvider { "\n" + WildcardMatcher.DOCUMENTATION ) - .tags("internal") + .tags("internal","added[1.49.0]") .dynamic(true) .buildWithDefault(Arrays.asList( "hikari" diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java index 2dde8be37f..5ff0962629 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/helper/JdbcGetUserNameExclusionTest.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.jdbc.helper; +import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.MockTracer; import co.elastic.apm.agent.bci.ElasticApmAgent; import co.elastic.apm.agent.configuration.SpyConfiguration; @@ -40,22 +41,10 @@ import static co.elastic.apm.agent.testutils.assertions.Assertions.assertThat; import static org.mockito.Mockito.doReturn; -public class JdbcGetUserNameExclusionTest { +public class JdbcGetUserNameExclusionTest extends AbstractInstrumentationTest { - protected static ElasticApmTracer tracer; - protected static ConfigurationRegistry config; protected static JdbcConfiguration jdbcconfig; - @BeforeAll - @BeforeClass - public static synchronized void beforeAll() { - MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); - tracer = mockInstrumentationSetup.getTracer(); - config = mockInstrumentationSetup.getConfig(); - assertThat(tracer.isRunning()).isTrue(); - ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install()); - } - @Test public void hasUsernameCorrectlyExcludes() throws SQLException { DatabaseMetaData meta = (DatabaseMetaData) Proxy.newProxyInstance( @@ -74,12 +63,6 @@ public void hasUsernameCorrectlyExcludes() throws SQLException { assertThat(JdbcHelper.maybeGetUserName(meta, config.getConfig(JdbcConfiguration.class))).isEqualTo(null); } - @After - @AfterEach - public final void cleanUp() { - SpyConfiguration.reset(config); - } - public class MetadataInvocationHandler implements InvocationHandler { @Override