Skip to content

Commit

Permalink
Made disable_send dynamic, exclude health & config requests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@JonasKunz
JonasKunz committed Oct 10, 2023
1 parent 61dfa35 commit 23613da
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 30 deletions.
2 changes: 1 addition & 1 deletion ...t-core/src/main/java/co/elastic/apm/agent/configuration/ApmServerConfigurationSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ String fetchConfig(final ConfigurationRegistry configurationRegistry) {
}
try {
payloadSerializer.blockUntilReady();
return apmServerClient.execute("/config/v1/agents", new ApmServerClient.ConnectionHandler<String>() {
return apmServerClient.forceExecute("/config/v1/agents", new ApmServerClient.ConnectionHandler<String>() {
@Override
public String withConnection(HttpURLConnection connection) throws IOException {
try {
Expand Down
11 changes: 9 additions & 2 deletions apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ private static List<URL> shuffleUrls(List<URL> serverUrls) {

@Nullable
HttpURLConnection startRequest(String relativePath) throws IOException {
if(reporterConfiguration.isSendDisabled()) {
return null;
}
URL url = appendPathToCurrentUrl(relativePath);
if (url == null) {
return null;
Expand Down Expand Up @@ -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
Expand All @@ -255,7 +259,7 @@ void onConnectionError() {
* @throws Exception in case all retries yield an exception, the last will be thrown
*/
@Nullable
public <V> V execute(String path, ConnectionHandler<V> connectionHandler) throws Exception {
public <V> V forceExecute(String path, ConnectionHandler<V> connectionHandler) throws Exception {
final List<URL> prioritizedUrlList = getPrioritizedUrlList();
if (prioritizedUrlList.isEmpty()) {
return null;
Expand Down Expand Up @@ -285,7 +289,10 @@ public <V> V execute(String path, ConnectionHandler<V> connectionHandler) throws
throw previousException;
}

public <T> List<T> executeForAllUrls(String path, ConnectionHandler<T> connectionHandler) {
/**
* Executes the given requests for all server URLs, even if the disable_send configuration is set to true.
*/
public <T> List<T> forceExecuteForAllUrls(String path, ConnectionHandler<T> connectionHandler) {
List<URL> serverUrls = getServerUrls();
List<T> results = new ArrayList<>(serverUrls.size());
for (URL serverUrl : serverUrls) {
Expand Down
2 changes: 1 addition & 1 deletion apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public Future<Version> checkHealthAndGetMinVersion() {
@Nullable
@Override
public Version call() {
List<Version> versions = apmServerClient.executeForAllUrls("/", new ApmServerClient.ConnectionHandler<Version>() {
List<Version> versions = apmServerClient.forceExecuteForAllUrls("/", new ApmServerClient.ConnectionHandler<Version>() {
@Override
public Version withConnection(HttpURLConnection connection) {
try {
Expand Down
7 changes: 4 additions & 3 deletions apm-agent-core/src/main/java/co/elastic/apm/agent/report/ReporterConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<URL> getServerUrls() {
if (disableSend.get()) {
return Collections.emptyList();
}
List<URL> calculatedUrlList;
URL singleUrl = serverUrl.get();
List<URL> urlList = serverUrls.get();
Expand All @@ -267,6 +264,10 @@ public List<URL> getServerUrls() {
return calculatedUrlList;
}

public boolean isSendDisabled() {
return disableSend.get();
}

public TimeDuration getServerTimeout() {
return serverTimeout.get();
}
Expand Down
47 changes: 25 additions & 22 deletions apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
Expand All @@ -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")));
Expand All @@ -233,7 +233,7 @@ public void testWrapUrlsOnConsecutiveError() throws Exception {

@Test
public void testRetry() throws Exception {
assertThat(apmServerClient.<String>execute("/test", conn -> new String(conn.getInputStream().readAllBytes()))).isEqualTo("hello from server 2");
assertThat(apmServerClient.<String>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")));
Expand All @@ -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")));
Expand All @@ -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;
});
Expand All @@ -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;
});
Expand Down Expand Up @@ -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<URL> 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
Expand All @@ -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;
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand Down
2 changes: 1 addition & 1 deletion apm-agent-core/src/test/java/specs/MockServerStepDefinitions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 23613da

Please sign in to comment.