From 547682df1cdf0e8fd43cdae081b47b6dc1e265a0 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 3 Apr 2023 15:33:14 +0200 Subject: [PATCH 01/14] implement + test --- .../apm/agent/report/ApmServerClient.java | 15 +++++- .../report/serialize/DslJsonSerializer.java | 29 ++++++++---- .../apm/agent/report/ApmServerClientTest.java | 47 ++++++++++++++++--- .../serialize/DslJsonSerializerTest.java | 45 ++++++++++++++---- 4 files changed, 112 insertions(+), 24 deletions(-) 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 e40b247ed8..0be5257cd0 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 @@ -72,6 +72,7 @@ public class ApmServerClient { private static final Version VERSION_7_4 = Version.of("7.4.0"); private static final Version VERSION_8_0 = Version.of("8.0.0"); private static final Version VERSION_8_6 = Version.of("8.6.0"); + private static final Version VERSION_8_7_1 = Version.of("8.7.1"); private final ReporterConfiguration reporterConfiguration; @Nullable @@ -117,6 +118,11 @@ private void setServerUrls(List serverUrls) { this.errorCount.set(0); } + public boolean isServerVersionReady() { + Future localValue = apmServerVersion; + return localValue != null && localValue.isDone(); + } + private static List shuffleUrls(List serverUrls) { // shuffling the URL list helps to distribute the load across the apm servers // when there are multiple agents, they should not all start connecting to the same apm server @@ -339,10 +345,17 @@ public boolean supportsLogsEndpoint() { return isAtLeast(VERSION_8_6); } + public boolean supportsActivationMethod() { + if (!isServerVersionReady()) { + return true; + } + return isAtLeast(VERSION_8_7_1); + } + public boolean supportsKeepingUnsampledTransaction() { // Method is called from application threads thus we have to return fast to avoid blocking application threads // When server version is not known we assume that it's a 7.x or earlier and keep sending unsampled. - if (apmServerVersion != null && !apmServerVersion.isDone()) { + if (!isServerVersionReady()) { return true; } return isLowerThan(VERSION_8_0); diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java index 7b3e8fc10c..3f45e9319d 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java @@ -87,6 +87,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; import static com.dslplatform.json.JsonWriter.ARRAY_END; import static com.dslplatform.json.JsonWriter.ARRAY_START; @@ -184,10 +185,10 @@ public void appendMetaDataNdJsonToStream() throws UninitializedException { jw.writeByte(NEW_LINE); } - static void serializeMetadata(MetaData metaData, JsonWriter metadataJW, boolean supportsConfiguredAndDetectedHostname) { + static void serializeMetadata(MetaData metaData, JsonWriter metadataJW, boolean supportsConfiguredAndDetectedHostname, boolean supportsAgentActivationMethod) { StringBuilder metadataReplaceBuilder = new StringBuilder(); metadataJW.writeByte(JsonWriter.OBJECT_START); - serializeService(metaData.getService(), metadataReplaceBuilder, metadataJW); + serializeService(metaData.getService(), metadataReplaceBuilder, metadataJW, supportsAgentActivationMethod); metadataJW.writeByte(COMMA); serializeProcess(metaData.getProcess(), metadataReplaceBuilder, metadataJW); metadataJW.writeByte(COMMA); @@ -233,9 +234,19 @@ private void assertMetaDataReady() throws UninitializedException { */ @Override public void blockUntilReady() throws Exception { - if (serializedMetaData == null) { + + boolean supportsActivationMethod = apmServerClient.supportsActivationMethod(); + Boolean serializedActivationMethodValue = serializedActivationMethod.get(); + boolean updateRequired = serializedActivationMethodValue == null || supportsActivationMethod != serializedActivationMethodValue.booleanValue(); + + if (serializedMetaData == null || updateRequired) { + serializedActivationMethod.set(supportsActivationMethod); + JsonWriter metadataJW = new DslJson<>(new DslJson.Settings<>()).newWriter(4096); - serializeMetadata(metaData.get(5, TimeUnit.SECONDS), metadataJW, apmServerClient.supportsConfiguredAndDetectedHostname()); + MetaData meta = metaData.get(5, TimeUnit.SECONDS); + boolean supportsConfiguredAndDetectedHostname = apmServerClient.supportsConfiguredAndDetectedHostname(); + + serializeMetadata(meta, metadataJW, supportsConfiguredAndDetectedHostname, true); serializedMetaData = metadataJW.toByteArray(); } } @@ -447,7 +458,7 @@ public String toString() { return jw.toString(); } - private static void serializeService(final Service service, final StringBuilder replaceBuilder, final JsonWriter jw) { + private static void serializeService(final Service service, final StringBuilder replaceBuilder, final JsonWriter jw, boolean supportsAgentActivationMethod) { writeFieldName("service", jw); jw.writeByte(JsonWriter.OBJECT_START); @@ -457,7 +468,7 @@ private static void serializeService(final Service service, final StringBuilder final Agent agent = service.getAgent(); if (agent != null) { - serializeAgent(agent, replaceBuilder, jw); + serializeAgent(agent, replaceBuilder, jw, supportsAgentActivationMethod); } final Language language = service.getLanguage(); @@ -536,10 +547,12 @@ private static void serializeService(@Nullable String name, @Nullable String ver serializeService(name, version, null, replaceBuilder, jw); } - private static void serializeAgent(final Agent agent, final StringBuilder replaceBuilder, final JsonWriter jw) { + private static void serializeAgent(final Agent agent, final StringBuilder replaceBuilder, final JsonWriter jw, boolean supportsAgentActivationMethod) { writeFieldName("agent", jw); jw.writeByte(JsonWriter.OBJECT_START); - writeField("activation_method", agent.getActivationMethod(), replaceBuilder, jw); + if (supportsAgentActivationMethod) { + writeField("activation_method", agent.getActivationMethod(), replaceBuilder, jw); + } writeField("name", agent.getName(), replaceBuilder, jw); writeField("ephemeral_id", agent.getEphemeralId(), replaceBuilder, jw); writeLastField("version", agent.getVersion(), replaceBuilder, jw); 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 c268a10c5f..94e9243bf4 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 @@ -50,6 +50,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -400,16 +401,48 @@ private void stubServerVersion(@Nullable String version){ @Test public void testSupportLogsEnpoint() { - testSupportLogsEndpoint(null, false); - testSupportLogsEndpoint("8.5.99", false); - testSupportLogsEndpoint("8.6.0", true); - testSupportLogsEndpoint("9.0.0", true); + String feature = "logs endpoint"; + Callable featureMethod = () -> apmServerClient.supportsLogsEndpoint(); + + testSupportedFeature(feature, featureMethod,null, false); + testSupportedFeature(feature, featureMethod,"8.5.99", false); + testSupportedFeature(feature, featureMethod,"8.6.0", true); + testSupportedFeature(feature, featureMethod,"9.0.0", true); + } + + @Test + public void testSupportsActivationMethod() { + String feature = "agent activation method"; + Callable featureMethod = () -> apmServerClient.supportsActivationMethod(); + + testSupportedFeature(feature, featureMethod, null, true); + testSupportedFeature(feature, featureMethod, "8.6.99", false); + testSupportedFeature(feature, featureMethod, "8.7.0", false); + testSupportedFeature(feature, featureMethod, "8.7.1", true); + testSupportedFeature(feature, featureMethod, "9.0.0", true); + } + + @Test + public void testSupportsSendingUnsampledTransactions() { + String feature = "keep unsampled transactions"; + Callable featureMethod = () -> apmServerClient.supportsKeepingUnsampledTransaction(); + + testSupportedFeature(feature, featureMethod, null, true); + testSupportedFeature(feature, featureMethod, "7.99.99", true); + testSupportedFeature(feature, featureMethod, "8.0.0", false); + testSupportedFeature(feature, featureMethod, "9.0.0", false); } - private void testSupportLogsEndpoint(@Nullable String version, boolean expected) { + private void testSupportedFeature(String feature, Callable featureMethod, @Nullable String version, boolean expected) { stubServerVersion(version); - assertThat(apmServerClient.supportsLogsEndpoint()) - .describedAs("logs endpoint for version %s is expected to be %s", expected ? "supported": "not supported") + Boolean result; + try { + result = featureMethod.call(); + } catch (Exception e) { + throw new RuntimeException(e); + } + assertThat(result) + .describedAs("%s for version '%s' is expected to be %s", feature, version, expected ? "supported" : "not supported") .isEqualTo(expected); } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index 56d5e92dc5..fbcc40c663 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -81,6 +81,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.function.Function; @@ -1067,7 +1068,7 @@ void testSystemInfo_configuredHostname(boolean supportsConfiguredAndDetectedHost String platform = "test-platform"; MetaData metaData = createMetaData(new SystemInfo(arc, "configured", "detected", platform)); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), supportsConfiguredAndDetectedHostname); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), supportsConfiguredAndDetectedHostname, true); serializer.appendMetadataToStream(); JsonNode system = readJsonString(serializer.toString()).get("system"); @@ -1092,7 +1093,7 @@ void testSystemInfo_detectedHostname(boolean supportsConfiguredAndDetectedHostna String platform = "test-platform"; MetaData metaData = createMetaData(new SystemInfo(arc, null, "detected", platform)); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), supportsConfiguredAndDetectedHostname); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), supportsConfiguredAndDetectedHostname, true); serializer.appendMetadataToStream(); JsonNode system = readJsonString(serializer.toString()).get("system"); @@ -1117,7 +1118,7 @@ void testSystemInfo_nullHostname(boolean supportsConfiguredAndDetectedHostname) String platform = "test-platform"; MetaData metaData = createMetaData(new SystemInfo(arc, null, null, platform)); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), supportsConfiguredAndDetectedHostname); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), supportsConfiguredAndDetectedHostname, true); serializer.appendMetadataToStream(); JsonNode system = readJsonString(serializer.toString()).get("system"); @@ -1144,7 +1145,7 @@ void testCloudProviderInfoWithNullObjectFields() throws Exception { cloudProviderInfo.setInstance(null); cloudProviderInfo.setService(null); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true, true); serializer.appendMetadataToStream(); JsonNode jsonCloud = readJsonString(serializer.toString()).get("cloud"); @@ -1171,7 +1172,7 @@ void testCloudProviderInfoWithNullNameFields() throws Exception { Objects.requireNonNull(cloudProviderInfo.getProject()).setName(null); Objects.requireNonNull(cloudProviderInfo.getInstance()).setName(null); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true, true); serializer.appendMetadataToStream(); JsonNode jsonCloud = readJsonString(serializer.toString()).get("cloud"); @@ -1204,7 +1205,7 @@ void testCloudProviderInfoWithNullNameAndIdFields() throws Exception { instance.setName(null); instance.setId(null); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true, true); serializer.appendMetadataToStream(); JsonNode jsonCloud = readJsonString(serializer.toString()).get("cloud"); @@ -1228,7 +1229,7 @@ void testCloudProviderInfoWithNullIdFields() throws Exception { Objects.requireNonNull(cloudProviderInfo.getInstance()).setId(null); Objects.requireNonNull(cloudProviderInfo.getAccount()).setId(null); - DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true); + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true, true); serializer.appendMetadataToStream(); JsonNode jsonCloud = readJsonString(serializer.toString()).get("cloud"); @@ -1248,6 +1249,30 @@ void testCloudProviderInfoWithNullIdFields() throws Exception { assertThat(jsonCloudProject.get("name").asText()).isEqualTo("projectName"); } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testActivationMethod(boolean supportsActivationMethod) throws Exception { + + MetaData metaData = createMetaData(); + + DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true, supportsActivationMethod); + serializer.appendMetadataToStream(); + + JsonNode service = readJsonString(serializer.toString()).get("service"); + + assertThat(service).isNotNull(); + + JsonNode activationMethod = service.get("agent").get("activation_method"); + + if (!supportsActivationMethod) { + assertThat(activationMethod).isNull(); + } else { + assertThat(activationMethod.isTextual()).isTrue(); + assertThat(activationMethod.asText()).isEqualTo("unknown"); + } + + } + private MetaData createMetaData() throws Exception { return createMetaData(SystemInfo.create("hostname", 0, mock(ServerlessConfiguration.class))); } @@ -1256,7 +1281,11 @@ private MetaData createMetaData(SystemInfo system) throws Exception { Service service = new Service().withAgent(new Agent("name", "version")).withName("name"); final ProcessInfo processInfo = new ProcessInfo("title"); processInfo.getArgv().add("test"); - return MetaDataMock.create(processInfo, service, system, createCloudProviderInfo(), new HashMap<>(0), createFaaSMetaDataExtension()).get(); + try { + return MetaDataMock.create(processInfo, service, system, createCloudProviderInfo(), new HashMap<>(0), createFaaSMetaDataExtension()).get(); + } catch (InterruptedException|ExecutionException e) { + throw new RuntimeException(e); + } } private CloudProviderInfo createCloudProviderInfo() { From 3d7ed9b0aec5ee06dcd8c341c38577af023ac39d Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 3 Apr 2023 16:24:02 +0200 Subject: [PATCH 02/14] missin nullable annotation --- .../main/java/co/elastic/apm/agent/impl/metadata/Agent.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java index dc5692c8a8..144142f2a1 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java @@ -24,6 +24,7 @@ import co.elastic.apm.agent.tracer.GlobalTracer; import co.elastic.apm.agent.util.PrivilegedActionUtils; +import javax.annotation.Nullable; import java.lang.management.ManagementFactory; import java.util.List; import java.util.UUID; @@ -61,7 +62,7 @@ public Agent(String name, String version) { this(name, version, UUID.randomUUID().toString(), null); } - public Agent(String name, String version, String ephemeralId, CoreConfiguration coreConfiguration) { + public Agent(String name, String version, String ephemeralId, @Nullable CoreConfiguration coreConfiguration) { this.name = name; this.version = version; this.ephemeralId = ephemeralId; @@ -99,7 +100,7 @@ public String getActivationMethod() { return activationMethod; } - private String getActivationMethod(CoreConfiguration coreConfiguration) { + private String getActivationMethod(@Nullable CoreConfiguration coreConfiguration) { ActivationMethod activation = ActivationMethod.UNKNOWN; if (coreConfiguration != null) { activation = coreConfiguration.getActivationMethod(); From 993333e2233201d5a564c154e4c3932812d08d38 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 3 Apr 2023 16:26:17 +0200 Subject: [PATCH 03/14] finish impl + testing --- .../report/serialize/DslJsonSerializer.java | 4 +- .../apm/agent/impl/metadata/MetaDataMock.java | 23 +++++++- .../serialize/DslJsonSerializerTest.java | 52 +++++++++++++++++-- 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java index 3f45e9319d..6aab6e3461 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java @@ -116,6 +116,8 @@ public class DslJsonSerializer implements PayloadSerializer { @Nullable private byte[] serializedMetaData; + private final AtomicReference serializedActivationMethod = new AtomicReference<>(null); + public DslJsonSerializer(StacktraceConfiguration stacktraceConfiguration, ApmServerClient apmServerClient, final Future metaData) { this.stacktraceConfiguration = stacktraceConfiguration; this.apmServerClient = apmServerClient; @@ -246,7 +248,7 @@ public void blockUntilReady() throws Exception { MetaData meta = metaData.get(5, TimeUnit.SECONDS); boolean supportsConfiguredAndDetectedHostname = apmServerClient.supportsConfiguredAndDetectedHostname(); - serializeMetadata(meta, metadataJW, supportsConfiguredAndDetectedHostname, true); + serializeMetadata(meta, metadataJW, supportsConfiguredAndDetectedHostname, supportsActivationMethod); serializedMetaData = metadataJW.toByteArray(); } } diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/metadata/MetaDataMock.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/metadata/MetaDataMock.java index 1fa9917216..6de3f37f85 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/metadata/MetaDataMock.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/impl/metadata/MetaDataMock.java @@ -24,6 +24,8 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import static org.mockito.Mockito.spy; + public class MetaDataMock { /** @@ -33,7 +35,7 @@ public class MetaDataMock { */ public static Future create(ProcessInfo process, Service service, SystemInfo system, @Nullable CloudProviderInfo cloudProviderInfo, Map globalLabels, @Nullable FaaSMetaDataExtension faaSMetaDataExtension) { - return new NoWaitFuture(new MetaData(process, service, system, cloudProviderInfo, globalLabels, faaSMetaDataExtension)); + return create(new MetaData(process, service, system, cloudProviderInfo, globalLabels, faaSMetaDataExtension)); } /** @@ -42,7 +44,7 @@ public static Future create(ProcessInfo process, Service service, Syst * @return a mock future, already containing the medata info */ public static Future create() { - return new NoWaitFuture<>(new MetaData( + return create(new MetaData( new ProcessInfo("test-process"), new Service(), new SystemInfo("x64", "localhost", null, "platform"), @@ -52,6 +54,23 @@ public static Future create() { )); } + public static Future create(MetaData value) { + return new NoWaitFuture<>(value); + } + + /** + * @return a metadata spy with some default values + */ + public static MetaData createDefaultMock() { + return spy(new MetaData( + new ProcessInfo("test-process"), + new Service(), + new SystemInfo("x64", "localhost", null, "platform"), + null, + Collections.emptyMap(), + null)); + } + private static class NoWaitFuture implements Future { @Nullable diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index fbcc40c663..fe9d6a800d 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -67,6 +67,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.stagemonitor.configuration.ConfigurationRegistry; @@ -1258,19 +1259,60 @@ void testActivationMethod(boolean supportsActivationMethod) throws Exception { DslJsonSerializer.serializeMetadata(metaData, serializer.getJsonWriter(), true, supportsActivationMethod); serializer.appendMetadataToStream(); - JsonNode service = readJsonString(serializer.toString()).get("service"); + checkMetadataActivationMethod(serializer.toString(), supportsActivationMethod ? "unknown" : null); + } + + @ParameterizedTest + @CsvSource(delimiter = '|', value = { + "true | false", // likely when going from "version unknown" to "known unsupported version" + "false | true", // unlikey to happen in practice, but worth testing anyway + }) + void testActivationMethodMetadataUpdate(boolean value1, boolean value2 ) throws Exception { + // because metadata is serialized only once, we need to ensure it's properly updated whenever needed + // in particular when going from 'apm server version unknown' to 'apm server version known' + + StacktraceConfiguration stacktraceConfiguration = mock(StacktraceConfiguration.class); + apmServerClient = mock(ApmServerClient.class); + doReturn(value1).when(apmServerClient).supportsActivationMethod(); + Service service = mock(Service.class); + Agent agent = new Agent("java-test", "1.0.0"); + doReturn(agent).when(service).getAgent(); + MetaData mockMetada = MetaDataMock.createDefaultMock(); + doReturn(service).when(mockMetada).getService(); + + serializer = new DslJsonSerializer(stacktraceConfiguration, apmServerClient, MetaDataMock.create(mockMetada)); + serializer.blockUntilReady(); + serializer.appendMetadataToStream(); + + checkMetadataActivationMethod(serializer.toString(), value1 ? "unknown": null); + serializer.jw.reset(); + + doReturn(value2).when(apmServerClient).supportsActivationMethod(); + + serializer.blockUntilReady(); + serializer.appendMetadataToStream(); + + checkMetadataActivationMethod(serializer.toString(), value2 ? "unknown": null); + } + + private void checkMetadataActivationMethod(String json, @Nullable String expectedValue) { + JsonNode root = readJsonString(json); + + JsonNode service = root.get("service"); assertThat(service).isNotNull(); - JsonNode activationMethod = service.get("agent").get("activation_method"); + JsonNode agent = service.get("agent"); + assertThat(agent).isNotNull(); - if (!supportsActivationMethod) { + JsonNode activationMethod = agent.get("activation_method"); + + if (expectedValue == null) { assertThat(activationMethod).isNull(); } else { assertThat(activationMethod.isTextual()).isTrue(); - assertThat(activationMethod.asText()).isEqualTo("unknown"); + assertThat(activationMethod.asText()).isEqualTo(expectedValue); } - } private MetaData createMetaData() throws Exception { From 4c829343abe0d63ed149bee147eca6a4d2ae6b29 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Mon, 3 Apr 2023 16:57:05 +0200 Subject: [PATCH 04/14] revert useless change --- .../apm/agent/report/serialize/DslJsonSerializerTest.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java index fe9d6a800d..c768ecc8bc 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/serialize/DslJsonSerializerTest.java @@ -1323,11 +1323,7 @@ private MetaData createMetaData(SystemInfo system) throws Exception { Service service = new Service().withAgent(new Agent("name", "version")).withName("name"); final ProcessInfo processInfo = new ProcessInfo("title"); processInfo.getArgv().add("test"); - try { - return MetaDataMock.create(processInfo, service, system, createCloudProviderInfo(), new HashMap<>(0), createFaaSMetaDataExtension()).get(); - } catch (InterruptedException|ExecutionException e) { - throw new RuntimeException(e); - } + return MetaDataMock.create(processInfo, service, system, createCloudProviderInfo(), new HashMap<>(0), createFaaSMetaDataExtension()).get(); } private CloudProviderInfo createCloudProviderInfo() { From d10d8024c727e8164805031c1161e9a050f00659 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 4 Apr 2023 13:25:24 +0200 Subject: [PATCH 05/14] fix tests & reuse common test code --- .../apm/agent/test/JavaExecutable.java | 43 ++++++++++++++++ apm-agent-core/pom.xml | 8 +++ .../agent/configuration/ActivationTypeIT.java | 50 ++++--------------- .../CommonsExecAsyncInstrumentationTest.java | 16 +----- .../jakartaee-simple-webapp/pom.xml | 7 +++ .../webapp/JakartaExecuteCmdServlet.java | 17 ++----- .../co/elastic/apm/test/TestAppContainer.java | 10 +--- .../apm/testapp/RuntimeAttachTestIT.java | 13 +---- integration-tests/simple-webapp/pom.xml | 7 +++ .../co/elastic/webapp/ExecuteCmdServlet.java | 17 ++----- 10 files changed, 88 insertions(+), 100 deletions(-) create mode 100644 apm-agent-common/src/test/java/co/elastic/apm/agent/test/JavaExecutable.java diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/JavaExecutable.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/JavaExecutable.java new file mode 100644 index 0000000000..f2f2a4ba9d --- /dev/null +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/JavaExecutable.java @@ -0,0 +1,43 @@ +/* + * 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.test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +public class JavaExecutable { + + private JavaExecutable() { + + } + + /** + * @return absolute path to current Java executable + */ + public static String getBinaryPath() { + boolean isWindows = System.getProperty("os.name").startsWith("Windows"); + String executable = isWindows ? "java.exe" : "java"; + Path path = Paths.get(System.getProperty("java.home"), "bin", executable); + if (!Files.isExecutable(path)) { + throw new IllegalStateException("unable to find java path " + path); + } + return path.toAbsolutePath().toString(); + } +} diff --git a/apm-agent-core/pom.xml b/apm-agent-core/pom.xml index 30d382eb91..f0fc2e88be 100644 --- a/apm-agent-core/pom.xml +++ b/apm-agent-core/pom.xml @@ -203,6 +203,14 @@ testcontainers test + + + ${project.groupId} + apm-agent-common + ${project.version} + test-jar + test + diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java index 58cd3b2fd8..be67de1481 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java @@ -18,6 +18,8 @@ */ package co.elastic.apm.agent.configuration; +import co.elastic.apm.agent.test.AgentFileAccessor; +import co.elastic.apm.agent.test.JavaExecutable; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -36,6 +38,7 @@ import java.net.ServerSocket; import java.net.Socket; import java.net.SocketException; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -59,14 +62,11 @@ public class ActivationTypeIT { @BeforeAll public static void setUp() throws IOException { - ElasticAgentJarFileLocation = getJarPath("elastic-apm-agent", false); - assertThat(ElasticAgentJarFileLocation).isNotNull(); - ElasticAgentAttachJarFileLocation = getJarPath("apm-agent-attach", false); - assertThat(ElasticAgentAttachJarFileLocation).isNotNull(); - ElasticAgentAttachTestJarFileLocation = getJarPath("apm-agent-attach", true); - assertThat(ElasticAgentAttachTestJarFileLocation).isNotNull(); - ElasticAgentAttachCliJarFileLocation = getJarPath("apm-agent-attach-cli", false); - assertThat(ElasticAgentAttachCliJarFileLocation).isNotNull(); + + ElasticAgentJarFileLocation = AgentFileAccessor.getPathToJavaagent().toAbsolutePath().toString(); + ElasticAgentAttachJarFileLocation = AgentFileAccessor.getPathToAttacher().toAbsolutePath().toString(); + ElasticAgentAttachTestJarFileLocation = AgentFileAccessor.getArtifactPath(Path.of("apm-agent-attach"), "-tests", ".jar").toAbsolutePath().toString(); + ElasticAgentAttachCliJarFileLocation = AgentFileAccessor.getPathToAttacher().toAbsolutePath().toString(); } public MockServer startServer() throws IOException { @@ -78,34 +78,6 @@ public MockServer startServer() throws IOException { return server; } - private static String getJarPath(String project, boolean findTestJar) { - File rootDir = new File(".."); - File projectDir = new File(rootDir, project); - assertThat(projectDir.exists()).isTrue(); - assertThat(projectDir.isDirectory()).isTrue(); - File targetDir = new File(projectDir, "target"); - assertThat(targetDir.exists()).isTrue(); - assertThat(targetDir.isDirectory()).isTrue(); - String jarName = null; - for (String file : targetDir.list()) { - if (file.matches("^"+project+".*"+".jar$") - && !file.contains("-sources") && !file.contains("-slim")) { - if (!findTestJar && file.contains("-tests")) { - continue; - } - if (findTestJar && !file.contains("-tests")) { - continue; - } - File jarNameFile = new File(targetDir, file); - assertThat(jarNameFile.exists()).isTrue(); - assertThat(jarNameFile.isDirectory()).isFalse(); - assertThat(jarNameFile.canRead()).isTrue(); - jarName = jarNameFile.getPath(); - } - } - return jarName; - } - @Test public void testSelfAttach() throws Exception { try (MockServer server = startServer()) { @@ -196,12 +168,12 @@ public void executeCommandInNewThread(ProcessBuilder pb, ActivationHandler handl if (serviceName != null) { ProcessBuilder pbAttach; if ("fleet".equals(activationMethod)) { - pbAttach = new ProcessBuilder("java", + pbAttach = new ProcessBuilder(JavaExecutable.getBinaryPath().toString(), "-jar", ElasticAgentAttachCliJarFileLocation, "--include-vmarg", serviceName, "-C", "activation_method=FLEET"); } else { - pbAttach = new ProcessBuilder("java", + pbAttach = new ProcessBuilder(JavaExecutable.getBinaryPath().toString(), "-jar", ElasticAgentAttachCliJarFileLocation, "--include-vmarg", serviceName); } @@ -325,7 +297,7 @@ public void executeCommand() throws IOException, InterruptedException { public void init() { command.clear(); - addOption("java"); + addOption(JavaExecutable.getBinaryPath()); addOption("-Xmx32m"); addOption("-classpath"); addOption(Classpath); diff --git a/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/CommonsExecAsyncInstrumentationTest.java b/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/CommonsExecAsyncInstrumentationTest.java index 480a11833b..616a6f69f2 100644 --- a/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/CommonsExecAsyncInstrumentationTest.java +++ b/apm-agent-plugins/apm-process-plugin/src/test/java/co/elastic/apm/agent/process/CommonsExecAsyncInstrumentationTest.java @@ -21,15 +21,13 @@ import co.elastic.apm.agent.AbstractInstrumentationTest; import co.elastic.apm.agent.impl.transaction.AbstractSpan; import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.test.JavaExecutable; import org.apache.commons.exec.CommandLine; import org.apache.commons.exec.DefaultExecuteResultHandler; import org.apache.commons.exec.DefaultExecutor; import org.apache.commons.exec.ExecuteException; import org.junit.jupiter.api.Test; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -103,7 +101,7 @@ public void onProcessFailed(ExecuteException e) { } }; - new DefaultExecutor().execute(new CommandLine(getJavaBinaryPath()).addArgument("-version"), handler); + new DefaultExecutor().execute(new CommandLine(JavaExecutable.getBinaryPath()).addArgument("-version"), handler); handler.waitFor(); assertThat(future.isCompletedExceptionally()) @@ -113,16 +111,6 @@ public void onProcessFailed(ExecuteException e) { return future; } - private static String getJavaBinaryPath() { - boolean isWindows = System.getProperty("os.name").startsWith("Windows"); - String executable = isWindows ? "java.exe" : "java"; - Path path = Paths.get(System.getProperty("java.home"), "bin", executable); - if (!Files.isExecutable(path)) { - throw new IllegalStateException("unable to find java path"); - } - return path.toAbsolutePath().toString(); - } - private static void startTransaction() { Transaction transaction = tracer.startRootTransaction(CommonsExecAsyncInstrumentationTest.class.getClassLoader()); diff --git a/integration-tests/jakartaee-simple-webapp/pom.xml b/integration-tests/jakartaee-simple-webapp/pom.xml index d7abcda244..0786858acc 100644 --- a/integration-tests/jakartaee-simple-webapp/pom.xml +++ b/integration-tests/jakartaee-simple-webapp/pom.xml @@ -56,6 +56,13 @@ apm-agent-api ${project.version} + + + ${project.groupId} + apm-agent-common + ${project.version} + test-jar + diff --git a/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java b/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java index 6fbd6e57f0..bc85646a74 100644 --- a/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java +++ b/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java @@ -18,15 +18,14 @@ */ package co.elastic.webapp; +import co.elastic.apm.agent.test.JavaExecutable; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; + import java.io.IOException; import java.io.PrintWriter; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Arrays; public class JakartaExecuteCmdServlet extends HttpServlet { @@ -38,7 +37,7 @@ private enum Variant { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException { - String[] cmd = new String[]{getJavaBinaryPath(), "-version"}; + String[] cmd = new String[]{JavaExecutable.getBinaryPath(), "-version"}; String variant = req.getParameter("variant"); Variant v = variant != null ? Variant.valueOf(variant) : Variant.WAIT_FOR; @@ -76,14 +75,4 @@ private static void writeMsg(PrintWriter writer, String msg, Object... msgArgs) writer.println(String.format(msg, msgArgs)); } - private static String getJavaBinaryPath() { - boolean isWindows = System.getProperty("os.name").startsWith("Windows"); - String executable = isWindows ? "java.exe" : "java"; - Path path = Paths.get(System.getProperty("java.home"), "bin", executable); - if (!Files.isExecutable(path)) { - throw new IllegalStateException("unable to find java path"); - } - return path.toAbsolutePath().toString(); - } - } diff --git a/integration-tests/main-app-test/src/test/java/co/elastic/apm/test/TestAppContainer.java b/integration-tests/main-app-test/src/test/java/co/elastic/apm/test/TestAppContainer.java index f2b407a162..4e03df2b5f 100644 --- a/integration-tests/main-app-test/src/test/java/co/elastic/apm/test/TestAppContainer.java +++ b/integration-tests/main-app-test/src/test/java/co/elastic/apm/test/TestAppContainer.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.test; +import co.elastic.apm.agent.test.JavaExecutable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testcontainers.containers.GenericContainer; @@ -151,20 +152,13 @@ private static String debuggerArgument(int port, String hostname) { } private boolean probeDebugger(int port) { - boolean isWindows = System.getProperty("os.name").startsWith("Windows"); - String executable = isWindows ? "java.exe" : "java"; - Path path = Paths.get(System.getProperty("java.home"), "bin", executable); - if (!Files.isExecutable(path)) { - throw new IllegalStateException("unable to find java path"); - } - // the most straightforward way to probe for an active debugger listening on port is to start another JVM // with the debug options and check the process exit status. Trying to probe for open network port messes with // the debugger and makes IDEA stop it. The only downside of this is that the debugger will first attach to this // probe JVM, then the one running in a docker container we are aiming to debug. try { Process process = new ProcessBuilder() - .command(path.toAbsolutePath().toString(), debuggerArgument(port, "localhost"), "-version") + .command(JavaExecutable.getBinaryPath().toString(), debuggerArgument(port, "localhost"), "-version") .start(); process.waitFor(5, TimeUnit.SECONDS); return process.exitValue() == 0; diff --git a/integration-tests/runtime-attach/runtime-attach-test/src/test/java/co/elastic/apm/testapp/RuntimeAttachTestIT.java b/integration-tests/runtime-attach/runtime-attach-test/src/test/java/co/elastic/apm/testapp/RuntimeAttachTestIT.java index d479e06711..a5653f2129 100644 --- a/integration-tests/runtime-attach/runtime-attach-test/src/test/java/co/elastic/apm/testapp/RuntimeAttachTestIT.java +++ b/integration-tests/runtime-attach/runtime-attach-test/src/test/java/co/elastic/apm/testapp/RuntimeAttachTestIT.java @@ -19,6 +19,7 @@ package co.elastic.apm.testapp; import co.elastic.apm.agent.test.AgentFileAccessor; +import co.elastic.apm.agent.test.JavaExecutable; import com.sun.tools.attach.VirtualMachine; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -317,7 +318,7 @@ private Path getAppJar() { private ProcessHandle startForkedJvm(Path executableJar, List args) { ArrayList cmd = new ArrayList<>(); - cmd.add(getJavaBinaryPath()); + cmd.add(JavaExecutable.getBinaryPath()); cmd.add("-jar"); cmd.add(executableJar.toString()); cmd.addAll(args); @@ -337,16 +338,6 @@ private ProcessHandle startForkedJvm(Path executableJar, List args) { } } - private static String getJavaBinaryPath() { - boolean isWindows = System.getProperty("os.name").startsWith("Windows"); - String executable = isWindows ? "java.exe" : "java"; - Path path = Paths.get(System.getProperty("java.home"), "bin", executable); - if (!Files.isExecutable(path)) { - throw new IllegalStateException("unable to find java path"); - } - return path.toAbsolutePath().toString(); - } - private static void terminateJvm(ProcessHandle jvm) { int retryCount = 5; while (jvm.isAlive() && retryCount-- > 0) { diff --git a/integration-tests/simple-webapp/pom.xml b/integration-tests/simple-webapp/pom.xml index d29c190232..cff9506ca3 100644 --- a/integration-tests/simple-webapp/pom.xml +++ b/integration-tests/simple-webapp/pom.xml @@ -57,6 +57,13 @@ 1.30.0 + + + ${project.groupId} + apm-agent-common + ${project.version} + test-jar + diff --git a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java index c6638f6544..7ffcec0a82 100644 --- a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java +++ b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java @@ -18,15 +18,14 @@ */ package co.elastic.webapp; +import co.elastic.apm.agent.test.JavaExecutable; + import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.PrintWriter; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Arrays; public class ExecuteCmdServlet extends HttpServlet { @@ -38,7 +37,7 @@ private enum Variant { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException { - String[] cmd = new String[]{getJavaBinaryPath(), "-version"}; + String[] cmd = new String[]{JavaExecutable.getBinaryPath(), "-version"}; String variant = req.getParameter("variant"); Variant v = variant != null ? Variant.valueOf(variant) : Variant.WAIT_FOR; @@ -76,14 +75,4 @@ private static void writeMsg(PrintWriter writer, String msg, Object... msgArgs) writer.println(String.format(msg, msgArgs)); } - private static String getJavaBinaryPath() { - boolean isWindows = System.getProperty("os.name").startsWith("Windows"); - String executable = isWindows ? "java.exe" : "java"; - Path path = Paths.get(System.getProperty("java.home"), "bin", executable); - if (!Files.isExecutable(path)) { - throw new IllegalStateException("unable to find java path"); - } - return path.toAbsolutePath().toString(); - } - } From 39b037e28d24cac798c1821719180b256e2a54cb Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 4 Apr 2023 13:49:28 +0200 Subject: [PATCH 06/14] simplify a bit by using a synchronized method --- .../report/serialize/DslJsonSerializer.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java index 6aab6e3461..d641ecb27a 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java @@ -115,8 +115,7 @@ public class DslJsonSerializer implements PayloadSerializer { private final Future metaData; @Nullable private byte[] serializedMetaData; - - private final AtomicReference serializedActivationMethod = new AtomicReference<>(null); + private boolean serializedActivationMethod; public DslJsonSerializer(StacktraceConfiguration stacktraceConfiguration, ApmServerClient apmServerClient, final Future metaData) { this.stacktraceConfiguration = stacktraceConfiguration; @@ -235,22 +234,20 @@ private void assertMetaDataReady() throws UninitializedException { * @throws Exception if blocking was interrupted, or timed out or an error occurred in the underlying implementation */ @Override - public void blockUntilReady() throws Exception { - + public synchronized void blockUntilReady() throws Exception { boolean supportsActivationMethod = apmServerClient.supportsActivationMethod(); - Boolean serializedActivationMethodValue = serializedActivationMethod.get(); - boolean updateRequired = serializedActivationMethodValue == null || supportsActivationMethod != serializedActivationMethodValue.booleanValue(); + if (null != serializedMetaData && serializedActivationMethod == supportsActivationMethod) { + return; + } - if (serializedMetaData == null || updateRequired) { - serializedActivationMethod.set(supportsActivationMethod); + serializedActivationMethod = supportsActivationMethod; - JsonWriter metadataJW = new DslJson<>(new DslJson.Settings<>()).newWriter(4096); - MetaData meta = metaData.get(5, TimeUnit.SECONDS); - boolean supportsConfiguredAndDetectedHostname = apmServerClient.supportsConfiguredAndDetectedHostname(); + JsonWriter metadataJW = new DslJson<>(new DslJson.Settings<>()).newWriter(4096); + MetaData meta = metaData.get(5, TimeUnit.SECONDS); + boolean supportsConfiguredAndDetectedHostname = apmServerClient.supportsConfiguredAndDetectedHostname(); - serializeMetadata(meta, metadataJW, supportsConfiguredAndDetectedHostname, supportsActivationMethod); - serializedMetaData = metadataJW.toByteArray(); - } + serializeMetadata(meta, metadataJW, supportsConfiguredAndDetectedHostname, supportsActivationMethod); + serializedMetaData = metadataJW.toByteArray(); } private static void serializeGlobalLabels(ArrayList globalLabelKeys, ArrayList globalLabelValues, From ab022da81c640f344906b13db4ea5e7db90d71fc Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 4 Apr 2023 14:28:36 +0200 Subject: [PATCH 07/14] upgrade animal sniffer plugin --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 3f466752ff..aee0feaa31 100644 --- a/pom.xml +++ b/pom.xml @@ -139,7 +139,7 @@ 5.1.1 - 1.17 + 1.23 1.16.3 From 2889cccb14ec7f1d05f78075223bce98b1e1cd01 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 4 Apr 2023 15:51:20 +0200 Subject: [PATCH 08/14] revert animal sniffer + ignore on test code --- integration-tests/pom.xml | 2 ++ pom.xml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index c25a6cdb77..4bd4fd1123 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -32,6 +32,8 @@ true ${project.basedir}/.. + + true diff --git a/pom.xml b/pom.xml index aee0feaa31..3f466752ff 100644 --- a/pom.xml +++ b/pom.xml @@ -139,7 +139,7 @@ 5.1.1 - 1.23 + 1.17 1.16.3 From 72302e0bd12b149638db816e0a9b90dfd0bc2272 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 4 Apr 2023 16:28:22 +0200 Subject: [PATCH 09/14] skip javadoc for integration tests --- integration-tests/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index 4bd4fd1123..44ddb0af75 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -34,6 +34,7 @@ ${project.basedir}/.. true + true From 118d2ec409cfd1b8ad9822ba043377ba1e561e45 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Tue, 4 Apr 2023 17:35:11 +0200 Subject: [PATCH 10/14] make common module java7 compatible --- apm-agent-common/pom.xml | 3 ++ .../common/util/ProcessExecutionUtilTest.java | 20 +++++++---- .../util/ResourceExtractionUtilTest.java | 36 +++++++++++++------ .../apm/agent/test/AgentFileAccessor.java | 26 +++++++++----- 4 files changed, 60 insertions(+), 25 deletions(-) diff --git a/apm-agent-common/pom.xml b/apm-agent-common/pom.xml index 695aa24866..18ed92614d 100644 --- a/apm-agent-common/pom.xml +++ b/apm-agent-common/pom.xml @@ -13,6 +13,9 @@ true ${project.basedir}/.. + + + 7 diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java index db75662c20..b1b1056671 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java @@ -19,7 +19,10 @@ package co.elastic.apm.agent.common.util; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; @@ -34,7 +37,12 @@ class ProcessExecutionUtilTest { @Test void testQuietExecutionOfNonExistingCommand() { final AtomicReference cmdOutputRef = new AtomicReference<>(); - assertDoesNotThrow(() -> cmdOutputRef.set(ProcessExecutionUtil.executeCommand(List.of("does", "not", "exist")))); + assertDoesNotThrow(new Executable() { + @Override + public void execute() throws Throwable { + cmdOutputRef.set(ProcessExecutionUtil.executeCommand(Arrays.asList("does", "not", "exist"))); + } + }); ProcessExecutionUtil.CommandOutput commandOutput = cmdOutputRef.get(); assertThat(commandOutput.getExitCode()).isLessThan(0); assertThat(commandOutput.getExceptionThrown()).isNotNull(); @@ -47,9 +55,9 @@ void testTimeout() { // other options: bash -c "sleep 0.2" (bash not guaranteed) // powershell -nop -c "& {sleep -m 2}" (too long to start) // timeout /NOBREAK /T 1 (output redirection fails) - cmd = List.of("ping", "192.0.2.1", "-n", "1", "-w", "200"); + cmd = Arrays.asList("ping", "192.0.2.1", "-n", "1", "-w", "200"); } else { - cmd = List.of("sleep", "0.2"); + cmd = Arrays.asList("sleep", "0.2"); } ProcessExecutionUtil.CommandOutput commandOutput = ProcessExecutionUtil.executeCommand(cmd, 100); System.out.println("commandOutput = " + commandOutput); @@ -67,8 +75,8 @@ void testTimeout() { @Test void cmdAsString() { - assertThat(ProcessExecutionUtil.cmdAsString(List.of())).isEqualTo("\"\""); - assertThat(ProcessExecutionUtil.cmdAsString(List.of("one"))).isEqualTo("\"one\""); - assertThat(ProcessExecutionUtil.cmdAsString(List.of("one", "two"))).isEqualTo("\"one two\""); + assertThat(ProcessExecutionUtil.cmdAsString(Collections.emptyList())).isEqualTo("\"\""); + assertThat(ProcessExecutionUtil.cmdAsString(Arrays.asList("one"))).isEqualTo("\"one\""); + assertThat(ProcessExecutionUtil.cmdAsString(Arrays.asList("one", "two"))).isEqualTo("\"one two\""); } } diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java index 17d79921a6..2e9fb7c0bb 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java @@ -18,10 +18,12 @@ */ package co.elastic.apm.agent.common.util; +import org.assertj.core.api.ThrowableAssert; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -29,6 +31,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -60,21 +63,31 @@ void exportResourceToDirectoryIdempotence(@TempDir Path tmpDir) throws Exception } @Test - void testContentDoesNotMatch(@TempDir Path tmpDir) throws Exception { + void testContentDoesNotMatch(final @TempDir Path tmpDir) throws Exception { Path tmp = ResourceExtractionUtil.extractResourceToDirectory("test.txt", "test", ".tmp", tmpDir); - Files.writeString(tmp, "changed"); - assertThatThrownBy(() -> ResourceExtractionUtil.extractResourceToDirectory("test.txt", "test", ".tmp", tmpDir)) + Files.write(tmp, "changed".getBytes(StandardCharsets.UTF_8)); + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + @Override + public void call() throws Throwable { + ResourceExtractionUtil.extractResourceToDirectory("test.txt", "test", ".tmp", tmpDir); + } + }) .isInstanceOf(IllegalStateException.class); } @Test - void exportResourceToDirectory_throwExceptionIfNotFound(@TempDir Path tmpDir) { - assertThatThrownBy(() -> ResourceExtractionUtil.extractResourceToDirectory("nonexist", "nonexist", ".tmp", tmpDir)) + void exportResourceToDirectory_throwExceptionIfNotFound(final @TempDir Path tmpDir) { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + @Override + public void call() throws Throwable { + ResourceExtractionUtil.extractResourceToDirectory("nonexist", "nonexist", ".tmp", tmpDir); + } + }) .hasMessage("nonexist not found"); } @Test - void exportResourceToDirectoryInMultipleThreads(@TempDir Path tmpDir) throws InterruptedException, ExecutionException { + void exportResourceToDirectoryInMultipleThreads(final @TempDir Path tmpDir) throws InterruptedException, ExecutionException { final int nbThreads = 10; final ExecutorService executorService = Executors.newFixedThreadPool(nbThreads); final CountDownLatch countDownLatch = new CountDownLatch(nbThreads); @@ -82,10 +95,13 @@ void exportResourceToDirectoryInMultipleThreads(@TempDir Path tmpDir) throws Int final String tempFileNamePrefix = UUID.randomUUID().toString(); for (int i = 0; i < nbThreads; i++) { - futureList.add(executorService.submit(() -> { - countDownLatch.countDown(); - countDownLatch.await(); - return ResourceExtractionUtil.extractResourceToDirectory("test.txt", tempFileNamePrefix, ".tmp", tmpDir); + futureList.add(executorService.submit(new Callable() { + @Override + public Path call() throws Exception { + countDownLatch.countDown(); + countDownLatch.await(); + return ResourceExtractionUtil.extractResourceToDirectory("test.txt", tempFileNamePrefix, ".tmp", tmpDir); + } })); } diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java index 0f3906447b..30affc0c40 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java @@ -19,10 +19,12 @@ package co.elastic.apm.agent.test; import java.io.IOException; +import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Objects; +import java.util.regex.Pattern; /** * Provides access to packaged agent artifacts @@ -49,21 +51,21 @@ public static Path getPathToJavaagent(Variant agentBuild) { break; } return getArtifactPath( - Path.of(project), + Paths.get(project), "", ".jar"); } public static Path getPathToAttacher() { return getArtifactPath( - Path.of("apm-agent-attach-cli"), + Paths.get("apm-agent-attach-cli"), "", ".jar"); } public static Path getPathToSlimAttacher() { return getArtifactPath( - Path.of("apm-agent-attach-cli"), + Paths.get("apm-agent-attach-cli"), "-slim", ".jar"); } @@ -100,14 +102,20 @@ public static Path getArtifactPath(Path modulePath, String artifactSuffix, Strin throw new IllegalStateException(errorMsg); } - return Files.find(targetFolder, 1, (path, attr) -> path.getFileName().toString() - .matches(artifactName + "-\\d\\.\\d+\\.\\d+(\\.RC\\d+)?(-SNAPSHOT)?" + artifactSuffix + extension)) - .findFirst() - .map(Path::toAbsolutePath) - .orElseThrow(() -> new IllegalStateException(errorMsg)); - } catch (IOException e) { + Pattern pattern = Pattern.compile(artifactName + "-\\d\\.\\d+\\.\\d+(\\.RC\\d+)?(-SNAPSHOT)?" + artifactSuffix + extension); + try (DirectoryStream stream = Files.newDirectoryStream(targetFolder)) { + for (Path filePath : stream) { + if (pattern.matcher(filePath.getFileName().getFileName().toString()).matches()) { + return filePath.toAbsolutePath(); + } + } + } + + } catch (IOException e){ throw new IllegalStateException(e); } + + throw new IllegalStateException(); } } From fc953817b51c796285558da5cd66396689bd47d1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Wed, 5 Apr 2023 09:15:39 +0200 Subject: [PATCH 11/14] revert changes to bare minimum --- apm-agent-common/pom.xml | 3 --- .../apm/agent/test/AgentFileAccessor.java | 26 +++++++------------ .../jakartaee-simple-webapp/pom.xml | 7 ----- .../webapp/JakartaExecuteCmdServlet.java | 17 +++++++++--- integration-tests/simple-webapp/pom.xml | 7 ----- .../co/elastic/webapp/ExecuteCmdServlet.java | 17 +++++++++--- 6 files changed, 37 insertions(+), 40 deletions(-) diff --git a/apm-agent-common/pom.xml b/apm-agent-common/pom.xml index 18ed92614d..695aa24866 100644 --- a/apm-agent-common/pom.xml +++ b/apm-agent-common/pom.xml @@ -13,9 +13,6 @@ true ${project.basedir}/.. - - - 7 diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java index 30affc0c40..beb65eca5b 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java @@ -19,12 +19,10 @@ package co.elastic.apm.agent.test; import java.io.IOException; -import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Objects; -import java.util.regex.Pattern; /** * Provides access to packaged agent artifacts @@ -51,21 +49,21 @@ public static Path getPathToJavaagent(Variant agentBuild) { break; } return getArtifactPath( - Paths.get(project), + Path.of("elastic-apm-agent"), "", ".jar"); } public static Path getPathToAttacher() { return getArtifactPath( - Paths.get("apm-agent-attach-cli"), + Path.of("apm-agent-attach-cli"), "", ".jar"); } public static Path getPathToSlimAttacher() { return getArtifactPath( - Paths.get("apm-agent-attach-cli"), + Path.of("apm-agent-attach-cli"), "-slim", ".jar"); } @@ -102,20 +100,14 @@ public static Path getArtifactPath(Path modulePath, String artifactSuffix, Strin throw new IllegalStateException(errorMsg); } - Pattern pattern = Pattern.compile(artifactName + "-\\d\\.\\d+\\.\\d+(\\.RC\\d+)?(-SNAPSHOT)?" + artifactSuffix + extension); - try (DirectoryStream stream = Files.newDirectoryStream(targetFolder)) { - for (Path filePath : stream) { - if (pattern.matcher(filePath.getFileName().getFileName().toString()).matches()) { - return filePath.toAbsolutePath(); - } - } - } - - } catch (IOException e){ + return Files.find(targetFolder, 1, (path, attr) -> path.getFileName().toString() + .matches(artifactName + "-\\d\\.\\d+\\.\\d+(\\.RC\\d+)?(-SNAPSHOT)?" + artifactSuffix + extension)) + .findFirst() + .map(Path::toAbsolutePath) + .orElseThrow(() -> new IllegalStateException(errorMsg)); + } catch (IOException e) { throw new IllegalStateException(e); } - - throw new IllegalStateException(); } } diff --git a/integration-tests/jakartaee-simple-webapp/pom.xml b/integration-tests/jakartaee-simple-webapp/pom.xml index 0786858acc..d7abcda244 100644 --- a/integration-tests/jakartaee-simple-webapp/pom.xml +++ b/integration-tests/jakartaee-simple-webapp/pom.xml @@ -56,13 +56,6 @@ apm-agent-api ${project.version} - - - ${project.groupId} - apm-agent-common - ${project.version} - test-jar - diff --git a/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java b/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java index bc85646a74..6fbd6e57f0 100644 --- a/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java +++ b/integration-tests/jakartaee-simple-webapp/src/main/java/co/elastic/webapp/JakartaExecuteCmdServlet.java @@ -18,14 +18,15 @@ */ package co.elastic.webapp; -import co.elastic.apm.agent.test.JavaExecutable; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; - import java.io.IOException; import java.io.PrintWriter; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; public class JakartaExecuteCmdServlet extends HttpServlet { @@ -37,7 +38,7 @@ private enum Variant { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException { - String[] cmd = new String[]{JavaExecutable.getBinaryPath(), "-version"}; + String[] cmd = new String[]{getJavaBinaryPath(), "-version"}; String variant = req.getParameter("variant"); Variant v = variant != null ? Variant.valueOf(variant) : Variant.WAIT_FOR; @@ -75,4 +76,14 @@ private static void writeMsg(PrintWriter writer, String msg, Object... msgArgs) writer.println(String.format(msg, msgArgs)); } + private static String getJavaBinaryPath() { + boolean isWindows = System.getProperty("os.name").startsWith("Windows"); + String executable = isWindows ? "java.exe" : "java"; + Path path = Paths.get(System.getProperty("java.home"), "bin", executable); + if (!Files.isExecutable(path)) { + throw new IllegalStateException("unable to find java path"); + } + return path.toAbsolutePath().toString(); + } + } diff --git a/integration-tests/simple-webapp/pom.xml b/integration-tests/simple-webapp/pom.xml index cff9506ca3..d29c190232 100644 --- a/integration-tests/simple-webapp/pom.xml +++ b/integration-tests/simple-webapp/pom.xml @@ -57,13 +57,6 @@ 1.30.0 - - - ${project.groupId} - apm-agent-common - ${project.version} - test-jar - diff --git a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java index 7ffcec0a82..c6638f6544 100644 --- a/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java +++ b/integration-tests/simple-webapp/src/main/java/co/elastic/webapp/ExecuteCmdServlet.java @@ -18,14 +18,15 @@ */ package co.elastic.webapp; -import co.elastic.apm.agent.test.JavaExecutable; - import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.PrintWriter; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; public class ExecuteCmdServlet extends HttpServlet { @@ -37,7 +38,7 @@ private enum Variant { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException { - String[] cmd = new String[]{JavaExecutable.getBinaryPath(), "-version"}; + String[] cmd = new String[]{getJavaBinaryPath(), "-version"}; String variant = req.getParameter("variant"); Variant v = variant != null ? Variant.valueOf(variant) : Variant.WAIT_FOR; @@ -75,4 +76,14 @@ private static void writeMsg(PrintWriter writer, String msg, Object... msgArgs) writer.println(String.format(msg, msgArgs)); } + private static String getJavaBinaryPath() { + boolean isWindows = System.getProperty("os.name").startsWith("Windows"); + String executable = isWindows ? "java.exe" : "java"; + Path path = Paths.get(System.getProperty("java.home"), "bin", executable); + if (!Files.isExecutable(path)) { + throw new IllegalStateException("unable to find java path"); + } + return path.toAbsolutePath().toString(); + } + } From 8f290aedb8d25edb4a3305cd083a17e331563f14 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Wed, 5 Apr 2023 09:22:45 +0200 Subject: [PATCH 12/14] revert more stuff --- .../common/util/ProcessExecutionUtilTest.java | 20 ++++------- .../util/ResourceExtractionUtilTest.java | 36 ++++++------------- 2 files changed, 16 insertions(+), 40 deletions(-) diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java index b1b1056671..db75662c20 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ProcessExecutionUtilTest.java @@ -19,10 +19,7 @@ package co.elastic.apm.agent.common.util; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.function.Executable; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; @@ -37,12 +34,7 @@ class ProcessExecutionUtilTest { @Test void testQuietExecutionOfNonExistingCommand() { final AtomicReference cmdOutputRef = new AtomicReference<>(); - assertDoesNotThrow(new Executable() { - @Override - public void execute() throws Throwable { - cmdOutputRef.set(ProcessExecutionUtil.executeCommand(Arrays.asList("does", "not", "exist"))); - } - }); + assertDoesNotThrow(() -> cmdOutputRef.set(ProcessExecutionUtil.executeCommand(List.of("does", "not", "exist")))); ProcessExecutionUtil.CommandOutput commandOutput = cmdOutputRef.get(); assertThat(commandOutput.getExitCode()).isLessThan(0); assertThat(commandOutput.getExceptionThrown()).isNotNull(); @@ -55,9 +47,9 @@ void testTimeout() { // other options: bash -c "sleep 0.2" (bash not guaranteed) // powershell -nop -c "& {sleep -m 2}" (too long to start) // timeout /NOBREAK /T 1 (output redirection fails) - cmd = Arrays.asList("ping", "192.0.2.1", "-n", "1", "-w", "200"); + cmd = List.of("ping", "192.0.2.1", "-n", "1", "-w", "200"); } else { - cmd = Arrays.asList("sleep", "0.2"); + cmd = List.of("sleep", "0.2"); } ProcessExecutionUtil.CommandOutput commandOutput = ProcessExecutionUtil.executeCommand(cmd, 100); System.out.println("commandOutput = " + commandOutput); @@ -75,8 +67,8 @@ void testTimeout() { @Test void cmdAsString() { - assertThat(ProcessExecutionUtil.cmdAsString(Collections.emptyList())).isEqualTo("\"\""); - assertThat(ProcessExecutionUtil.cmdAsString(Arrays.asList("one"))).isEqualTo("\"one\""); - assertThat(ProcessExecutionUtil.cmdAsString(Arrays.asList("one", "two"))).isEqualTo("\"one two\""); + assertThat(ProcessExecutionUtil.cmdAsString(List.of())).isEqualTo("\"\""); + assertThat(ProcessExecutionUtil.cmdAsString(List.of("one"))).isEqualTo("\"one\""); + assertThat(ProcessExecutionUtil.cmdAsString(List.of("one", "two"))).isEqualTo("\"one two\""); } } diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java index 2e9fb7c0bb..17d79921a6 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/common/util/ResourceExtractionUtilTest.java @@ -18,12 +18,10 @@ */ package co.elastic.apm.agent.common.util; -import org.assertj.core.api.ThrowableAssert; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -31,7 +29,6 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; -import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -63,31 +60,21 @@ void exportResourceToDirectoryIdempotence(@TempDir Path tmpDir) throws Exception } @Test - void testContentDoesNotMatch(final @TempDir Path tmpDir) throws Exception { + void testContentDoesNotMatch(@TempDir Path tmpDir) throws Exception { Path tmp = ResourceExtractionUtil.extractResourceToDirectory("test.txt", "test", ".tmp", tmpDir); - Files.write(tmp, "changed".getBytes(StandardCharsets.UTF_8)); - assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { - @Override - public void call() throws Throwable { - ResourceExtractionUtil.extractResourceToDirectory("test.txt", "test", ".tmp", tmpDir); - } - }) + Files.writeString(tmp, "changed"); + assertThatThrownBy(() -> ResourceExtractionUtil.extractResourceToDirectory("test.txt", "test", ".tmp", tmpDir)) .isInstanceOf(IllegalStateException.class); } @Test - void exportResourceToDirectory_throwExceptionIfNotFound(final @TempDir Path tmpDir) { - assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { - @Override - public void call() throws Throwable { - ResourceExtractionUtil.extractResourceToDirectory("nonexist", "nonexist", ".tmp", tmpDir); - } - }) + void exportResourceToDirectory_throwExceptionIfNotFound(@TempDir Path tmpDir) { + assertThatThrownBy(() -> ResourceExtractionUtil.extractResourceToDirectory("nonexist", "nonexist", ".tmp", tmpDir)) .hasMessage("nonexist not found"); } @Test - void exportResourceToDirectoryInMultipleThreads(final @TempDir Path tmpDir) throws InterruptedException, ExecutionException { + void exportResourceToDirectoryInMultipleThreads(@TempDir Path tmpDir) throws InterruptedException, ExecutionException { final int nbThreads = 10; final ExecutorService executorService = Executors.newFixedThreadPool(nbThreads); final CountDownLatch countDownLatch = new CountDownLatch(nbThreads); @@ -95,13 +82,10 @@ void exportResourceToDirectoryInMultipleThreads(final @TempDir Path tmpDir) thro final String tempFileNamePrefix = UUID.randomUUID().toString(); for (int i = 0; i < nbThreads; i++) { - futureList.add(executorService.submit(new Callable() { - @Override - public Path call() throws Exception { - countDownLatch.countDown(); - countDownLatch.await(); - return ResourceExtractionUtil.extractResourceToDirectory("test.txt", tempFileNamePrefix, ".tmp", tmpDir); - } + futureList.add(executorService.submit(() -> { + countDownLatch.countDown(); + countDownLatch.await(); + return ResourceExtractionUtil.extractResourceToDirectory("test.txt", tempFileNamePrefix, ".tmp", tmpDir); })); } From c53e0b9a70e619725dfe1f08d9cee1fbff588650 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Wed, 5 Apr 2023 09:22:53 +0200 Subject: [PATCH 13/14] revert more stuff --- .../apm/agent/test/AgentFileAccessor.java | 20 ++++++++----------- integration-tests/pom.xml | 3 --- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java index beb65eca5b..3b5e5fad9e 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/test/AgentFileAccessor.java @@ -30,8 +30,13 @@ public class AgentFileAccessor { public enum Variant { - STANDARD, - JAVA8_BUILD + STANDARD("elastic-apm-agent"), + JAVA8_BUILD("elastic-apm-agent-java8"); + + private String artifact; + Variant(String artifact){ + this.artifact = artifact; + } } public static Path getPathToJavaagent() { @@ -39,17 +44,8 @@ public static Path getPathToJavaagent() { } public static Path getPathToJavaagent(Variant agentBuild) { - String project = "elastic-apm-agent"; - switch (agentBuild) { - case STANDARD: - project = "elastic-apm-agent"; - break; - case JAVA8_BUILD: - project = "elastic-apm-agent-java8"; - break; - } return getArtifactPath( - Path.of("elastic-apm-agent"), + Path.of(agentBuild.artifact), "", ".jar"); } diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index 44ddb0af75..c25a6cdb77 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -32,9 +32,6 @@ true ${project.basedir}/.. - - true - true From ac802f2ae21ff95fd551f13bb473ebd8896f6e50 Mon Sep 17 00:00:00 2001 From: Sylvain Juge Date: Wed, 5 Apr 2023 15:24:33 +0200 Subject: [PATCH 14/14] rewrite some activation method integ tests --- .../apm/agent/impl/metadata/Agent.java | 4 +- .../agent/configuration/ActivationTypeIT.java | 301 +++++++----------- 2 files changed, 117 insertions(+), 188 deletions(-) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java index 144142f2a1..ccd913592d 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/metadata/Agent.java @@ -100,7 +100,7 @@ public String getActivationMethod() { return activationMethod; } - private String getActivationMethod(@Nullable CoreConfiguration coreConfiguration) { + private static String getActivationMethod(@Nullable CoreConfiguration coreConfiguration) { ActivationMethod activation = ActivationMethod.UNKNOWN; if (coreConfiguration != null) { activation = coreConfiguration.getActivationMethod(); @@ -118,6 +118,7 @@ private String getActivationMethod(@Nullable CoreConfiguration coreConfiguration return activation.toReferenceString(); } + @Nullable private static String getAgentJarFilename() { String agentLocation = PrivilegedActionUtils.getProtectionDomain(GlobalTracer.class).getCodeSource().getLocation().getFile(); if (agentLocation != null) { @@ -132,6 +133,7 @@ private static String getAgentJarFilename() { } } + @Nullable private static String getElasticJavaagentOnTheCommandline() { String agentJarFile = getAgentJarFilename(); if (agentJarFile != null) { diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java index be67de1481..537a8bf552 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/configuration/ActivationTypeIT.java @@ -20,24 +20,25 @@ import co.elastic.apm.agent.test.AgentFileAccessor; import co.elastic.apm.agent.test.JavaExecutable; +import co.elastic.apm.agent.testutils.TestPort; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.sun.net.httpserver.HttpContext; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.parallel.Execution; -import org.junit.jupiter.api.parallel.ExecutionMode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.annotation.Nullable; import java.io.BufferedReader; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.PrintWriter; -import java.lang.ref.Cleaner; -import java.net.ServerSocket; -import java.net.Socket; -import java.net.SocketException; +import java.net.InetSocketAddress; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; @@ -45,24 +46,20 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; -@Execution(ExecutionMode.CONCURRENT) public class ActivationTypeIT { - // Activation is about how an external config or process activates the agent, - // so tests here spawn a full JVM with an agent to test the activation method - private static final int TIMEOUT_IN_SECONDS = 200; private static String ElasticAgentAttachJarFileLocation; private static String ElasticAgentAttachTestJarFileLocation; private static String ElasticAgentAttachCliJarFileLocation; private static String ElasticAgentJarFileLocation; - private static final Cleaner MockCleaner = Cleaner.create(); @BeforeAll public static void setUp() throws IOException { - ElasticAgentJarFileLocation = AgentFileAccessor.getPathToJavaagent().toAbsolutePath().toString(); ElasticAgentAttachJarFileLocation = AgentFileAccessor.getPathToAttacher().toAbsolutePath().toString(); ElasticAgentAttachTestJarFileLocation = AgentFileAccessor.getArtifactPath(Path.of("apm-agent-attach"), "-tests", ".jar").toAbsolutePath().toString(); @@ -72,9 +69,7 @@ public static void setUp() throws IOException { public MockServer startServer() throws IOException { final MockServer server = new MockServer(); server.start(); - assertThat(server.waitUntilStarted(500)).isTrue(); assertThat(server.port()).isGreaterThan(0); - MockCleaner.register(server, () -> server.stop()); return server; } @@ -92,7 +87,7 @@ public void testSelfAttach() throws Exception { } @Test - public void testCLIAttach() throws Exception { + public void testJavaAgentAttach() throws Exception { try (MockServer server = startServer()) { JvmAgentProcess proc = new JvmAgentProcess(server, "JavaAgentCLI", "co.elastic.apm.agent.configuration.ActivationTestExampleApp", @@ -148,14 +143,18 @@ public void fakeTestLambdaAttach() throws Exception { } static class ExternalProcess { + @Nullable volatile Process child; boolean debug = true; private static void pauseSeconds(int seconds) { - try {Thread.sleep(seconds*1_000L);} catch (InterruptedException e) {} + try { + Thread.sleep(seconds * 1_000L); + } catch (InterruptedException e) { + } } - public void executeCommandInNewThread(ProcessBuilder pb, ActivationHandler handler, String activationMethod, String serviceName) throws IOException, InterruptedException { + public void executeCommandInNewThread(ProcessBuilder pb, String activationMethod, MockServer mockServer, @Nullable String serviceName) throws IOException { ExternalProcess spawnedProcess = new ExternalProcess(); new Thread(() -> { try { @@ -168,33 +167,58 @@ public void executeCommandInNewThread(ProcessBuilder pb, ActivationHandler handl if (serviceName != null) { ProcessBuilder pbAttach; if ("fleet".equals(activationMethod)) { - pbAttach = new ProcessBuilder(JavaExecutable.getBinaryPath().toString(), + pbAttach = new ProcessBuilder(JavaExecutable.getBinaryPath(), "-jar", ElasticAgentAttachCliJarFileLocation, "--include-vmarg", serviceName, "-C", "activation_method=FLEET"); } else { - pbAttach = new ProcessBuilder(JavaExecutable.getBinaryPath().toString(), + pbAttach = new ProcessBuilder(JavaExecutable.getBinaryPath(), "-jar", ElasticAgentAttachCliJarFileLocation, "--include-vmarg", serviceName); } executeCommandSynchronously(pbAttach); } - waitForActivationMethod(handler, TIMEOUT_IN_SECONDS*1000); - assertThat(handler.found()).isTrue(); - terminate(); - spawnedProcess.terminate(); - } - private static void waitForActivationMethod(ActivationHandler handler, long timeoutInMillis) { - long start = System.currentTimeMillis(); - while (System.currentTimeMillis() - start < timeoutInMillis) { - if (handler.found()) { - return; + ObjectMapper objectMapper = new ObjectMapper(); + await().until(() -> !mockServer.getReceivedBodyLines().isEmpty()); + + List jsonLines = mockServer.getReceivedBodyLines().stream() + .map(line -> { + try { + return objectMapper.readTree(line); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + ).collect(Collectors.toList()); + + String foundServiceName = null; + String foundActivationMethod = null; + for (JsonNode jsonLine : jsonLines) { + JsonNode service = jsonLine.get("service"); + if (service != null) { + JsonNode name = service.get("name"); + if (name != null && name.isTextual()) { + foundServiceName = name.asText(); + } + JsonNode agent = service.get("agent"); + if (agent != null) { + JsonNode am = agent.get("activation_method"); + if (am != null && am.isTextual()) { + foundActivationMethod = am.asText(); + } + } } - try {Thread.sleep(5);} catch (InterruptedException e) {} } - } + if (serviceName != null) { + assertThat(foundServiceName).isEqualTo(serviceName); + } + assertThat(foundActivationMethod).isEqualTo(activationMethod); + + terminate(); + spawnedProcess.terminate(); + } private void terminate() { if (child != null) { @@ -210,7 +234,7 @@ private void terminate() { public void executeCommandSynchronously(ProcessBuilder pb) throws IOException { if (debug) { - System.out.println("Executing command: "+ Arrays.toString(pb.command().toArray())); + System.out.println("Executing command: " + Arrays.toString(pb.command().toArray())); } pb.redirectErrorStream(true); Process childProcess = pb.start(); @@ -239,7 +263,10 @@ public void executeCommandSynchronously(ProcessBuilder pb) throws IOException { //Cleanup as well as I can boolean exited = false; - try {exited = childProcess.waitFor(3, TimeUnit.SECONDS);}catch (InterruptedException e) {} + try { + exited = childProcess.waitFor(3, TimeUnit.SECONDS); + } catch (InterruptedException e) { + } if (!exited) { childProcess.destroy(); pauseSeconds(1); @@ -263,7 +290,7 @@ static class JvmAgentProcess extends ExternalProcess { String serviceName; String targetClass; String activationMethod; - Map env; + Map env = new HashMap<>(); boolean attachRemotely; List targetParams = new ArrayList<>(); @@ -271,28 +298,24 @@ public void attachRemotely(boolean attachRemotely1) { this.attachRemotely = attachRemotely1; } - public JvmAgentProcess(MockServer server, String serviceName1, String targetClass1, String activationMethod1) { - apmServer = server; - serviceName = serviceName1; - targetClass = targetClass1; - activationMethod = activationMethod1; + public JvmAgentProcess(MockServer server, String serviceName, String targetClass, String activationMethod) { + this.apmServer = server; + this.serviceName = serviceName; + this.targetClass = targetClass; + this.activationMethod = activationMethod; init(); } public void addEnv(String key, String value) { - if(env == null) { - env = new HashMap<>(); - } env.put(key, value); } public void prependToClasspath(String location) { - command.set(3, location+System.getProperty("path.separator")+command.get(3)); + command.set(3, location + System.getProperty("path.separator") + command.get(3)); } - public void executeCommand() throws IOException, InterruptedException { - executeCommandInNewThread(buildProcess(), apmServer.getHandler(), - activationMethod, attachRemotely? serviceName : null); + public void executeCommand() throws IOException { + executeCommandInNewThread(buildProcess(), activationMethod, apmServer, attachRemotely ? serviceName : null); } public void init() { @@ -301,29 +324,24 @@ public void init() { addOption("-Xmx32m"); addOption("-classpath"); addOption(Classpath); - addAgentOption("server_url=http://localhost:"+apmServer.port()); + addAgentOption("server_url=http://localhost:" + apmServer.port()); for (String keyEqualsValue : TestAgentParams) { addAgentOption(keyEqualsValue); } - addAgentOption("service_name="+serviceName); - apmServer.getHandler().setActivationToWaitFor(serviceName, activationMethod); + addAgentOption("service_name=" + serviceName); } public void addAgentOption(String keyEqualsValue) { - command.add("-Delastic.apm."+keyEqualsValue); + command.add("-Delastic.apm." + keyEqualsValue); } public void addOption(String option) { command.add(option); } - public void addTargetParam(String param) { - targetParams.add(param); - } - private ProcessBuilder buildProcess() { command.add(targetClass); - for (String param :targetParams) { + for (String param : targetParams) { command.add(param); } ProcessBuilder pb = new ProcessBuilder(command); @@ -337,160 +355,69 @@ private ProcessBuilder buildProcess() { } - static class ActivationHandler { - - private volatile String serviceNameToWaitFor; - private volatile String activationMethodToWaitFor; - private volatile boolean found; - private final ObjectMapper objectMapper = new ObjectMapper(); - - public boolean found() { - return found; - } - - public void setActivationToWaitFor(String serviceNameToWaitFor1, String activationMethodToWaitFor1) { - this.serviceNameToWaitFor = serviceNameToWaitFor1; - this.activationMethodToWaitFor = activationMethodToWaitFor1; - found = false; - } - - public void handle(String line) { - try { - report(line); - } catch (JsonProcessingException e) { - e.printStackTrace(); - } - } - - private void report(String line) throws JsonProcessingException { - System.out.println("MockServer line read: "+line); - JsonNode messageRootNode = objectMapper.readTree(line); - JsonNode metadataNode = messageRootNode.get("metadata"); - if (metadataNode != null) { - JsonNode serviceNode = metadataNode.get("service"); - if (serviceNode != null) { - String name = serviceNode.get("name").asText(); - JsonNode agentNode = serviceNode.get("agent"); - if (agentNode != null) { - JsonNode activationNode = agentNode.get("activation_method"); - if(activationNode != null) { - String activationMethod = activationNode.asText(); - if (name.equals(serviceNameToWaitFor) && activationMethod.equals(activationMethodToWaitFor)) { - found = true; - } - } - } - } - } - } - - } - - class MockServer implements AutoCloseable { + static class MockServer implements AutoCloseable { - private static final String HTTP_HEADER ="HTTP/1.0 200 OK\nContent-Type: text/html; charset=utf-8\nServer: MockApmServer\n\n"; + private static final Logger log = LoggerFactory.getLogger(MockServer.class); - private volatile ServerSocket server; - private volatile boolean keepGoing = true; - private final ActivationHandler handler = new ActivationHandler(); + @Nullable + private HttpServer httpServer; + private List requestBodyLines = new ArrayList<>(); public MockServer() { } - public ActivationHandler getHandler() { - return handler; - } - public void stop() { - keepGoing = false; - try { - if (this.server != null) { - this.server.close(); - } - } catch (IOException e) { - System.out.println("MockApmServer: Unsuccessfully called stop(), stack trace follows, error is:"+e.getLocalizedMessage()); - e.printStackTrace(System.out); + if (httpServer == null) { + return; } + httpServer.stop(0); + httpServer = null; } public int port() { - if (this.server != null) { - return this.server.getLocalPort(); - } else { + if (httpServer == null) { return -1; } + return httpServer.getAddress().getPort(); } - public boolean waitUntilStarted(long timeoutInMillis) { - long start = System.currentTimeMillis(); - while((System.currentTimeMillis() - start < timeoutInMillis) && server == null) { - try {Thread.sleep(1);} catch (InterruptedException e) {} + public void start() throws IOException { + if (httpServer != null) { + throw new IllegalStateException("Ooops, you can't start this instance more than once"); } - return server != null; + + int serverPort = TestPort.getAvailableRandomPort(); + httpServer = HttpServer.create(new InetSocketAddress(serverPort), 0); + HttpContext context = httpServer.createContext("/"); + context.setHandler(httpHandler()); + httpServer.start(); + + log.info("starting mock APM server on port {}", serverPort); } - public void start() throws IOException { - if (this.server != null) { - throw new IOException("MockApmServer: Ooops, you can't start this instance more than once"); - } - new Thread(() -> { - try { - _start(); - } catch (Exception e) { - e.printStackTrace(); - } - }).start(); + public List getReceivedBodyLines() { + return requestBodyLines; } - private synchronized void _start() throws IOException { - if (this.server != null) { - throw new IOException("MockApmServer: Ooops, you can't start this instance more than once"); - } - this.server = new ServerSocket(0); - System.out.println("MockApmServer: Successfully called start(), now listening for requests on port "+this.server.getLocalPort()); - while(keepGoing) { - try(Socket client = this.server.accept()) { - while(!client.isClosed() && !client.isInputShutdown() && !client.isOutputShutdown()) { - try (BufferedReader clientInput = new BufferedReader(new InputStreamReader(client.getInputStream()))) { - String line = clientInput.readLine(); - if(line == null) { - //hmmm, try again - try {Thread.sleep(10);} catch (InterruptedException e) {} - line = clientInput.readLine(); - if (line == null) { - clientInput.close(); - break; - } - } - if (line.startsWith("GET /exit")) { - keepGoing = false; - } - while ( (line = clientInput.readLine()) != null) { - if (line.strip().startsWith("{")) { - try { - handler.handle(line.strip()); - } catch (Throwable e) { - //ignore, the report() is responsible to have log it - } - } - } - PrintWriter outputToClient = new PrintWriter(client.getOutputStream()); - outputToClient.println(HTTP_HEADER); - outputToClient.println("{}"); - outputToClient.flush(); - outputToClient.close(); - } catch (IOException e) { - if (!e.getMessage().equals("Connection reset")) { - e.printStackTrace(); - } + @NotNull + private HttpHandler httpHandler() { + return exchange -> { + + InputStream requestBody = exchange.getRequestBody(); + if (requestBody != null) { + try (BufferedReader reader = new BufferedReader(new InputStreamReader(requestBody))) { + String line = reader.readLine(); + if (!line.isEmpty()) { + requestBodyLines.add(line); } } - } catch (SocketException e) { - //ignore, we exit regardless and stop() at the end of the method } - } - stop(); + + String response = "{}"; + exchange.sendResponseHeaders(200, response.getBytes().length); + exchange.getResponseBody().write(response.getBytes()); + }; } @Override