From 23613da3e9d9e289dc9f1274e86d03aff408184f Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Tue, 10 Oct 2023 10:37:15 +0200 Subject: [PATCH] Made disable_send dynamic, exclude health & config requests. --- .../ApmServerConfigurationSource.java | 2 +- .../apm/agent/report/ApmServerClient.java | 11 ++++- .../agent/report/ApmServerHealthChecker.java | 2 +- .../agent/report/ReporterConfiguration.java | 7 +-- .../apm/agent/report/ApmServerClientTest.java | 47 ++++++++++--------- .../java/specs/MockServerStepDefinitions.java | 2 +- 6 files changed, 41 insertions(+), 30 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ApmServerConfigurationSource.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ApmServerConfigurationSource.java index 662810e4b7..6517e658da 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ApmServerConfigurationSource.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/ApmServerConfigurationSource.java @@ -165,7 +165,7 @@ String fetchConfig(final ConfigurationRegistry configurationRegistry) { } try { payloadSerializer.blockUntilReady(); - return apmServerClient.execute("/config/v1/agents", new ApmServerClient.ConnectionHandler() { + return apmServerClient.forceExecute("/config/v1/agents", new ApmServerClient.ConnectionHandler() { @Override public String withConnection(HttpURLConnection connection) throws IOException { try { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java index 1bc971d72f..eb92d25523 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java @@ -145,6 +145,9 @@ private static List shuffleUrls(List serverUrls) { @Nullable HttpURLConnection startRequest(String relativePath) throws IOException { + if(reporterConfiguration.isSendDisabled()) { + return null; + } URL url = appendPathToCurrentUrl(relativePath); if (url == null) { return null; @@ -247,6 +250,7 @@ void onConnectionError() { * If there's a connection error executing the request, * the request is retried with the next APM Server url. * The maximum amount of retries is the number of configured APM Server URLs. + * This method ignores the disable_send configuration. * * @param path the APM Server path * @param connectionHandler receives the {@link HttpURLConnection} and returns the result @@ -255,7 +259,7 @@ void onConnectionError() { * @throws Exception in case all retries yield an exception, the last will be thrown */ @Nullable - public V execute(String path, ConnectionHandler connectionHandler) throws Exception { + public V forceExecute(String path, ConnectionHandler connectionHandler) throws Exception { final List prioritizedUrlList = getPrioritizedUrlList(); if (prioritizedUrlList.isEmpty()) { return null; @@ -285,7 +289,10 @@ public V execute(String path, ConnectionHandler connectionHandler) throws throw previousException; } - public List executeForAllUrls(String path, ConnectionHandler connectionHandler) { + /** + * Executes the given requests for all server URLs, even if the disable_send configuration is set to true. + */ + public List forceExecuteForAllUrls(String path, ConnectionHandler connectionHandler) { List serverUrls = getServerUrls(); List results = new ArrayList<>(serverUrls.size()); for (URL serverUrl : serverUrls) { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java index 275ebd07d1..e27bec314a 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java @@ -64,7 +64,7 @@ public Future checkHealthAndGetMinVersion() { @Nullable @Override public Version call() { - List versions = apmServerClient.executeForAllUrls("/", new ApmServerClient.ConnectionHandler() { + List versions = apmServerClient.forceExecuteForAllUrls("/", new ApmServerClient.ConnectionHandler() { @Override public Version withConnection(HttpURLConnection connection) { try { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ReporterConfiguration.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ReporterConfiguration.java index 5a7965c317..e509511e50 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ReporterConfiguration.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ReporterConfiguration.java @@ -240,9 +240,6 @@ public String getApiKey() { * @return a list of APM Server URLs resulting from the combination of {@code server_url} and {@code server_urls} */ public List getServerUrls() { - if (disableSend.get()) { - return Collections.emptyList(); - } List calculatedUrlList; URL singleUrl = serverUrl.get(); List urlList = serverUrls.get(); @@ -267,6 +264,10 @@ public List getServerUrls() { return calculatedUrlList; } + public boolean isSendDisabled() { + return disableSend.get(); + } + public TimeDuration getServerTimeout() { return serverTimeout.get(); } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java index e469235bcc..7acf3f70f8 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java @@ -204,7 +204,7 @@ private static ConditionFactory awaitUpToOneSecond() { public void testInitialCurrentUrlIsFirstUrl() throws Exception { assertThat(Objects.requireNonNull(apmServerClient.getCurrentUrl()).getPort()).isEqualTo(apmServer1.port()); - apmServerClient.execute("/test", HttpURLConnection::getResponseCode); + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); apmServer1.verify(1, getRequestedFor(urlEqualTo("/test"))); apmServer2.verify(0, getRequestedFor(urlEqualTo("/test"))); @@ -215,7 +215,7 @@ public void testUseNextUrlOnError() throws Exception { apmServerClient.incrementAndGetErrorCount(0); assertThat(Objects.requireNonNull(apmServerClient.getCurrentUrl()).getPort()).isEqualTo(apmServer2.port()); - apmServerClient.execute("/test", HttpURLConnection::getResponseCode); + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); apmServer1.verify(0, getRequestedFor(urlEqualTo("/test"))); apmServer2.verify(1, getRequestedFor(urlEqualTo("/proxy/test"))); @@ -233,7 +233,7 @@ public void testWrapUrlsOnConsecutiveError() throws Exception { @Test public void testRetry() throws Exception { - assertThat(apmServerClient.execute("/test", conn -> new String(conn.getInputStream().readAllBytes()))).isEqualTo("hello from server 2"); + assertThat(apmServerClient.forceExecute("/test", conn -> new String(conn.getInputStream().readAllBytes()))).isEqualTo("hello from server 2"); assertThat(Objects.requireNonNull(apmServerClient.getCurrentUrl()).getPort()).isEqualTo(apmServer2.port()); apmServer1.verify(1, getRequestedFor(urlEqualTo("/test"))); apmServer2.verify(1, getRequestedFor(urlEqualTo("/proxy/test"))); @@ -242,7 +242,7 @@ public void testRetry() throws Exception { @Test public void testRetryFailure() { - assertThatThrownBy(() -> apmServerClient.execute("/not-found", URLConnection::getInputStream)) + assertThatThrownBy(() -> apmServerClient.forceExecute("/not-found", URLConnection::getInputStream)) .isInstanceOf(FileNotFoundException.class) .matches(t -> t.getSuppressed().length == 1, "should have a suppressed exception"); apmServer1.verify(1, getRequestedFor(urlEqualTo("/not-found"))); @@ -254,7 +254,7 @@ public void testRetryFailure() { @Test public void testExecuteSuccessfullyForAllUrls() { - apmServerClient.executeForAllUrls("/not-found", connection -> { + apmServerClient.forceExecuteForAllUrls("/not-found", connection -> { connection.getResponseCode(); return null; }); @@ -268,7 +268,7 @@ public void testExecuteSuccessfullyForAllUrls() { @Test public void testExecuteFailureForAllUrls() { // exception will only be logged, not thrown - apmServerClient.executeForAllUrls("/not-found", connection -> { + apmServerClient.forceExecuteForAllUrls("/not-found", connection -> { connection.getInputStream(); return null; }); @@ -320,17 +320,20 @@ public void testServerUrlSettingOverridesServerUrls() throws IOException { } @Test - public void testDisableSend() { - // We have to go through that because the disable_send config is non-dynamic - ConfigurationRegistry localConfig = SpyConfiguration.createSpyConfig( - Objects.requireNonNull(ConfigSources.fromClasspath("test.elasticapm.disable-send.properties", ClassLoader.getSystemClassLoader())) - ); - final ElasticApmTracer tracer = new ElasticApmTracerBuilder() - .reporter(new MockReporter()) - .configurationRegistry(localConfig) - .buildAndStart(); - List updatedServerUrls = tracer.getApmServerClient().getServerUrls(); - assertThat(updatedServerUrls).isEmpty(); + public void testDisableSend() throws Exception { + doReturn(true).when(reporterConfiguration).isSendDisabled(); + + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); + assertThat(apmServerClient.startRequest("/test")).isNull(); + apmServer1.verify(1, getRequestedFor(urlEqualTo("/test"))); + + doReturn(false).when(reporterConfiguration).isSendDisabled(); + + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); + HttpURLConnection request = apmServerClient.startRequest("/test"); + assertThat(request).isNotNull(); + request.getResponseCode(); + apmServer1.verify(3, getRequestedFor(urlEqualTo("/test"))); } @Test @@ -351,7 +354,7 @@ public void testWithEmptyServerUrlList() { client.start(Collections.emptyList()); Exception exception = null; try { - client.execute("/irrelevant", connection -> null); + client.forceExecute("/irrelevant", connection -> null); } catch (Exception e) { exception = e; } @@ -376,13 +379,13 @@ public void testSupportUnsampledTransactions() { public void testApiKeyRotation() throws Exception { doReturn("token1").when(reporterConfiguration).getApiKey(); - apmServerClient.execute("/test", HttpURLConnection::getResponseCode); + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); apmServer1.verify(1, getRequestedFor(urlEqualTo("/test")) .withHeader("Authorization", equalTo("ApiKey token1"))); doReturn("token2").when(reporterConfiguration).getApiKey(); - apmServerClient.execute("/test", HttpURLConnection::getResponseCode); + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); apmServer1.verify(1, getRequestedFor(urlEqualTo("/test")) .withHeader("Authorization", equalTo("ApiKey token2"))); } @@ -392,13 +395,13 @@ public void testApiKeyRotation() throws Exception { public void testSecretTokenRotation() throws Exception { doReturn("token1").when(reporterConfiguration).getSecretToken(); - apmServerClient.execute("/test", HttpURLConnection::getResponseCode); + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); apmServer1.verify(1, getRequestedFor(urlEqualTo("/test")) .withHeader("Authorization", equalTo("Bearer token1"))); doReturn("token2").when(reporterConfiguration).getSecretToken(); - apmServerClient.execute("/test", HttpURLConnection::getResponseCode); + apmServerClient.forceExecute("/test", HttpURLConnection::getResponseCode); apmServer1.verify(1, getRequestedFor(urlEqualTo("/test")) .withHeader("Authorization", equalTo("Bearer token2"))); } diff --git a/apm-agent-core/src/test/java/specs/MockServerStepDefinitions.java b/apm-agent-core/src/test/java/specs/MockServerStepDefinitions.java index 9fb47fddbd..4c93e90775 100644 --- a/apm-agent-core/src/test/java/specs/MockServerStepDefinitions.java +++ b/apm-agent-core/src/test/java/specs/MockServerStepDefinitions.java @@ -97,7 +97,7 @@ public void executeRequest() throws IOException { ApmServerClient apmServerClient = new ApmServerClient(config); apmServerClient.start(); try { - apmServerClient.execute("/", new ApmServerClient.ConnectionHandler<>() { + apmServerClient.forceExecute("/", new ApmServerClient.ConnectionHandler<>() { @Nullable @Override public Object withConnection(HttpURLConnection connection) throws IOException {