From 930918ebeec2613af8a28f47f37ab87dbdaddeeb Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 11 Feb 2021 10:01:17 +0100 Subject: [PATCH 1/2] Add support for enabled in config Signed-off-by: Pavol Loffay --- .../org/hypertrace/agent/core/config/EnvironmentConfig.java | 5 +++++ .../org/hypertrace/agent/core/config/HypertraceConfig.java | 3 +++ .../hypertrace/agent/core/config/EnvironmentConfigTest.java | 3 +++ .../hypertrace/agent/core/config/HypertraceConfigTest.java | 3 +++ javaagent-core/src/test/resources/config.yaml | 1 + .../agent/otel/extensions/HypertraceAgentConfiguration.java | 3 +++ 6 files changed, 18 insertions(+) diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/config/EnvironmentConfig.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/config/EnvironmentConfig.java index 719210a34..7098dfacd 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/config/EnvironmentConfig.java +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/config/EnvironmentConfig.java @@ -36,6 +36,7 @@ private EnvironmentConfig() {} public static final String CONFIG_FILE_PROPERTY = HT_PREFIX + "config.file"; static final String SERVICE_NAME = HT_PREFIX + "service.name"; + static final String ENABLED = HT_PREFIX + "enabled"; static final String PROPAGATION_FORMATS = HT_PREFIX + "propagation.formats"; @@ -63,6 +64,10 @@ public static AgentConfig.Builder applyPropertiesAndEnvVars(AgentConfig.Builder if (serviceName != null) { builder.setServiceName(StringValue.newBuilder().setValue(serviceName).build()); } + String enabled = getProperty(ENABLED); + if (enabled != null) { + builder.setEnabled(BoolValue.newBuilder().setValue(Boolean.valueOf(enabled)).build()); + } Reporting.Builder reportingBuilder = applyReporting(builder.getReporting().toBuilder()); builder.setReporting(reportingBuilder); diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/config/HypertraceConfig.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/config/HypertraceConfig.java index bb96fe85a..0e8c9ea49 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/config/HypertraceConfig.java +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/config/HypertraceConfig.java @@ -162,6 +162,9 @@ private static AgentConfig.Builder applyDefaults(AgentConfig.Builder configBuild if (configBuilder.getServiceName().getValue().isEmpty()) { configBuilder.setServiceName(StringValue.newBuilder().setValue(DEFAULT_SERVICE_NAME).build()); } + if (!configBuilder.hasEnabled()) { + configBuilder.setEnabled(BoolValue.newBuilder().setValue(true).build()); + } Reporting.Builder reportingBuilder = applyReportingDefaults(configBuilder.getReporting().toBuilder()); diff --git a/javaagent-core/src/test/java/org/hypertrace/agent/core/config/EnvironmentConfigTest.java b/javaagent-core/src/test/java/org/hypertrace/agent/core/config/EnvironmentConfigTest.java index 40587a4d5..1a250aaa0 100644 --- a/javaagent-core/src/test/java/org/hypertrace/agent/core/config/EnvironmentConfigTest.java +++ b/javaagent-core/src/test/java/org/hypertrace/agent/core/config/EnvironmentConfigTest.java @@ -36,6 +36,7 @@ class EnvironmentConfigTest { @ClearSystemProperty(key = EnvironmentConfig.CAPTURE_HTTP_BODY_PREFIX + "request") @ClearSystemProperty(key = EnvironmentConfig.CAPTURE_BODY_MAX_SIZE_BYTES) @ClearSystemProperty(key = EnvironmentConfig.JAVAAGENT_FILTER_JAR_PATHS) + @ClearSystemProperty(key = EnvironmentConfig.ENABLED) public void systemProperties() { // when tests are run in parallel the env vars/sys props set it junit-pioneer are visible to // parallel tests @@ -48,11 +49,13 @@ public void systemProperties() { System.setProperty(EnvironmentConfig.PROPAGATION_FORMATS, "B3,TRACECONTEXT"); System.setProperty(EnvironmentConfig.CAPTURE_BODY_MAX_SIZE_BYTES, "512"); System.setProperty(EnvironmentConfig.JAVAAGENT_FILTER_JAR_PATHS, "/path1.jar,/path/2/jar.jar"); + System.setProperty(EnvironmentConfig.ENABLED, "false"); AgentConfig.Builder configBuilder = AgentConfig.newBuilder(); configBuilder.setServiceName(StringValue.newBuilder().setValue("foo")); AgentConfig agentConfig = EnvironmentConfig.applyPropertiesAndEnvVars(configBuilder).build(); + Assertions.assertEquals(false, agentConfig.getEnabled().getValue()); Assertions.assertEquals("foo", agentConfig.getServiceName().getValue()); Assertions.assertEquals( Arrays.asList(PropagationFormat.B3, PropagationFormat.TRACECONTEXT), diff --git a/javaagent-core/src/test/java/org/hypertrace/agent/core/config/HypertraceConfigTest.java b/javaagent-core/src/test/java/org/hypertrace/agent/core/config/HypertraceConfigTest.java index d549cf780..936a55982 100644 --- a/javaagent-core/src/test/java/org/hypertrace/agent/core/config/HypertraceConfigTest.java +++ b/javaagent-core/src/test/java/org/hypertrace/agent/core/config/HypertraceConfigTest.java @@ -36,6 +36,7 @@ public class HypertraceConfigTest { public void defaultValues() throws IOException { URL resource = getClass().getClassLoader().getResource("emptyconfig.yaml"); AgentConfig agentConfig = HypertraceConfig.load(resource.getPath()); + Assertions.assertTrue(agentConfig.getEnabled().getValue()); Assertions.assertEquals("unknown", agentConfig.getServiceName().getValue()); Assertions.assertEquals( HypertraceConfig.DEFAULT_REPORTING_ENDPOINT, @@ -69,6 +70,7 @@ public void defaultValues() throws IOException { Assertions.assertEquals( true, agentConfig.getDataCapture().getRpcBody().getResponse().getValue()); Assertions.assertTrue(agentConfig.hasJavaagent()); + Assertions.assertEquals(0, agentConfig.getJavaagent().getFilterJarPathsCount()); } @Test @@ -95,6 +97,7 @@ public void jsonConfig(@TempDir File tempFolder) throws IOException { private void assertConfig(AgentConfig agentConfig) { Assertions.assertEquals("service", agentConfig.getServiceName().getValue()); + Assertions.assertEquals(false, agentConfig.getEnabled().getValue()); Assertions.assertEquals( Arrays.asList(PropagationFormat.B3), agentConfig.getPropagationFormatsList()); Assertions.assertEquals( diff --git a/javaagent-core/src/test/resources/config.yaml b/javaagent-core/src/test/resources/config.yaml index 24962a3ed..ea1f5f8ef 100644 --- a/javaagent-core/src/test/resources/config.yaml +++ b/javaagent-core/src/test/resources/config.yaml @@ -1,5 +1,6 @@ # use snake case for newly added fields service_name: service +enabled: false propagationFormats: - B3 reporting: diff --git a/otel-extensions/src/main/java/org/hypertrace/agent/otel/extensions/HypertraceAgentConfiguration.java b/otel-extensions/src/main/java/org/hypertrace/agent/otel/extensions/HypertraceAgentConfiguration.java index 82c0574c0..6651d1492 100644 --- a/otel-extensions/src/main/java/org/hypertrace/agent/otel/extensions/HypertraceAgentConfiguration.java +++ b/otel-extensions/src/main/java/org/hypertrace/agent/otel/extensions/HypertraceAgentConfiguration.java @@ -41,11 +41,14 @@ public class HypertraceAgentConfiguration implements PropertySource { private static final String OTEL_DEFAULT_LOG_LEVEL = "io.opentelemetry.javaagent.slf4j.simpleLogger.defaultLogLevel"; + private static final String OTEL_ENABLED = "otel.javaagent.enabled"; + @Override public Map getProperties() { AgentConfig agentConfig = HypertraceConfig.get(); Map configProperties = new HashMap<>(); + configProperties.put(OTEL_ENABLED, String.valueOf(agentConfig.getEnabled().getValue())); configProperties.put(OTEL_TRACE_EXPORTER, "zipkin"); configProperties.put( OTEL_EXPORTER_ZIPKIN_SERVICE_NAME, agentConfig.getServiceName().getValue()); From 4cdb4a8dc5d89d3092bb7acc7f6929b24a1ba670 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 11 Feb 2021 10:33:51 +0100 Subject: [PATCH 2/2] smoke test Signed-off-by: Pavol Loffay --- .../SpringBootDiabledAgentSmokeTest.java | 75 +++++++++++++++++++ ...pringBootDisabledBodyCaptureSmokeTest.java | 9 ++- 2 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDiabledAgentSmokeTest.java diff --git a/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDiabledAgentSmokeTest.java b/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDiabledAgentSmokeTest.java new file mode 100644 index 000000000..8af24a34a --- /dev/null +++ b/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDiabledAgentSmokeTest.java @@ -0,0 +1,75 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.hypertrace.agent.smoketest; + +import java.io.IOException; +import okhttp3.Request; +import okhttp3.Response; +import org.awaitility.core.ConditionTimeoutException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.utility.MountableFile; + +public class SpringBootDiabledAgentSmokeTest extends AbstractSmokeTest { + + @Override + protected String getTargetImage(int jdk) { + return "ghcr.io/open-telemetry/java-test-containers:smoke-springboot-jdk" + + jdk + + "-20210209.550405798"; + } + + private GenericContainer app; + + @BeforeEach + void beforeEach() { + app = createAppUnderTest(8); + app.addEnv("HT_ENABLED", "false"); + app.withCopyFileToContainer( + MountableFile.forClasspathResource("/ht-config-all-disabled.yaml"), + "/etc/ht-config-all-disabled.yaml"); + app.withEnv("HT_CONFIG_FILE", "/etc/ht-config-all-disabled.yaml"); + app.start(); + } + + @AfterEach + void afterEach() { + if (app != null) { + app.stop(); + } + } + + @Test() + public void get() throws IOException { + // TODO test with multiple JDK (11, 14) + String url = String.format("http://localhost:%d/greeting", app.getMappedPort(8080)); + Request request = new Request.Builder().url(url).get().build(); + + try (Response response = client.newCall(request).execute()) { + Assertions.assertEquals(response.body().string(), "Hi!"); + } + + Assertions.assertThrows( + ConditionTimeoutException.class, + () -> { + waitForTraces(); + }); + } +} diff --git a/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDisabledBodyCaptureSmokeTest.java b/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDisabledBodyCaptureSmokeTest.java index 2f6850402..49f2fdddf 100644 --- a/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDisabledBodyCaptureSmokeTest.java +++ b/smoke-tests/src/test/java/org/hypertrace/agent/smoketest/SpringBootDisabledBodyCaptureSmokeTest.java @@ -36,7 +36,9 @@ public class SpringBootDisabledBodyCaptureSmokeTest extends AbstractSmokeTest { @Override protected String getTargetImage(int jdk) { - return "open-telemetry-docker-dev.bintray.io/java/smoke-springboot-jdk" + jdk + ":latest"; + return "ghcr.io/open-telemetry/java-test-containers:smoke-springboot-jdk" + + jdk + + "-20210209.550405798"; } private GenericContainer app; @@ -65,7 +67,9 @@ public void get() throws IOException { String url = String.format("http://localhost:%d/greeting", app.getMappedPort(8080)); Request request = new Request.Builder().url(url).get().build(); - Response response = client.newCall(request).execute(); + try (Response response = client.newCall(request).execute()) { + Assertions.assertEquals(response.body().string(), "Hi!"); + } Collection traces = waitForTraces(); Object currentAgentVersion = @@ -74,7 +78,6 @@ public void get() throws IOException { .getMainAttributes() .get(Attributes.Name.IMPLEMENTATION_VERSION); - Assertions.assertEquals(response.body().string(), "Hi!"); Assertions.assertEquals(1, countSpansByName(traces, "/greeting")); Assertions.assertEquals(1, countSpansByName(traces, "webcontroller.greeting")); Assertions.assertTrue(