From bb2e2993478305e2021dfc4f60e1731690137b2a Mon Sep 17 00:00:00 2001 From: RashmiRam Date: Fri, 8 May 2020 11:29:26 +0530 Subject: [PATCH 1/9] Used BatchSpansProcessor instead of SimpleSpansProcessor BatchSpansProcessor won't get blocked by the exporter. Fixes #368 Signed-off-by: RashmiRam --- .../java/io/opentelemetry/auto/tooling/TracerInstaller.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java index 530770ea1c76..fd763effb859 100644 --- a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java +++ b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java @@ -22,7 +22,7 @@ import io.opentelemetry.sdk.contrib.auto.config.SpanExporterFactory; import io.opentelemetry.sdk.metrics.export.IntervalMetricReader; import io.opentelemetry.sdk.metrics.export.MetricExporter; -import io.opentelemetry.sdk.trace.export.SimpleSpansProcessor; +import io.opentelemetry.sdk.trace.export.BatchSpansProcessor; import io.opentelemetry.sdk.trace.export.SpanExporter; import java.io.File; import java.net.MalformedURLException; @@ -69,7 +69,7 @@ private static synchronized void installExportersFromJar(final String exporterJa if (spanExporterFactory != null) { final SpanExporter spanExporter = spanExporterFactory.fromConfig(config); OpenTelemetrySdk.getTracerProvider() - .addSpanProcessor(SimpleSpansProcessor.create(spanExporter)); + .addSpanProcessor(BatchSpansProcessor.create(spanExporter)); log.info("Installed span exporter: " + spanExporter.getClass().getName()); } else { log.warn("No matching providers in jar " + exporterJar); From 7e73a82538a731cfe44cba818d5670f646cec37d Mon Sep 17 00:00:00 2001 From: RashmiRam Date: Sat, 9 May 2020 01:02:04 +0530 Subject: [PATCH 2/9] Enable BatchSpanProcessor to use env vars for configuration --- .../java/io/opentelemetry/auto/tooling/TracerInstaller.java | 2 +- .../io/opentelemetry/smoketest/AbstractSmokeTest.groovy | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java index fd763effb859..1420f7be8c9d 100644 --- a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java +++ b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java @@ -69,7 +69,7 @@ private static synchronized void installExportersFromJar(final String exporterJa if (spanExporterFactory != null) { final SpanExporter spanExporter = spanExporterFactory.fromConfig(config); OpenTelemetrySdk.getTracerProvider() - .addSpanProcessor(BatchSpansProcessor.create(spanExporter)); + .addSpanProcessor(BatchSpansProcessor.create(spanExporter, BatchSpansProcessor.Config.loadFromDefaultSources())); // Loads configuration for SpansProcessor from env vars if exists. log.info("Installed span exporter: " + spanExporter.getClass().getName()); } else { log.warn("No matching providers in jar " + exporterJar); diff --git a/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy b/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy index 46bba7026162..a3119c578c36 100644 --- a/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy +++ b/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy @@ -70,6 +70,11 @@ abstract class AbstractSmokeTest extends Specification { processBuilder.environment().put("JAVA_HOME", System.getProperty("java.home")) processBuilder.environment().put("DD_API_KEY", API_KEY) +// // Setting configuration variables of batch span processor through env vars +// // This config is to immediately flush a batch of 1 span with delay of 10ms + processBuilder.environment().put("OTEL_BSP_MAX_EXPORT_BATCH", "1") + processBuilder.environment().put("OTEL_BSP_SCHEDULE_DELAY", "10") + processBuilder.redirectErrorStream(true) logfile = new File("${buildDirectory}/reports/testProcess.${this.getClass().getName()}.log") processBuilder.redirectOutput(ProcessBuilder.Redirect.to(logfile)) From c4383bab4ad139efe67f20b153078e9c95e1d2b6 Mon Sep 17 00:00:00 2001 From: RashmiRam Date: Sat, 9 May 2020 01:48:39 +0530 Subject: [PATCH 3/9] Formatted the code as per the style guidelines --- .../io/opentelemetry/auto/tooling/TracerInstaller.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java index 1420f7be8c9d..db6fd8fdb4d0 100644 --- a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java +++ b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java @@ -69,7 +69,12 @@ private static synchronized void installExportersFromJar(final String exporterJa if (spanExporterFactory != null) { final SpanExporter spanExporter = spanExporterFactory.fromConfig(config); OpenTelemetrySdk.getTracerProvider() - .addSpanProcessor(BatchSpansProcessor.create(spanExporter, BatchSpansProcessor.Config.loadFromDefaultSources())); // Loads configuration for SpansProcessor from env vars if exists. + .addSpanProcessor( + BatchSpansProcessor.create( + spanExporter, + BatchSpansProcessor.Config + .loadFromDefaultSources())); // Loads configuration for SpansProcessor from + // env vars if exists. log.info("Installed span exporter: " + spanExporter.getClass().getName()); } else { log.warn("No matching providers in jar " + exporterJar); From ff724f481a9aadd3bb6f5d5d93d0bceb335f70bb Mon Sep 17 00:00:00 2001 From: RashmiRam Date: Mon, 11 May 2020 10:47:23 +0530 Subject: [PATCH 4/9] Removed the comment --- .../java/io/opentelemetry/auto/tooling/TracerInstaller.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java index db6fd8fdb4d0..22d2266ce40a 100644 --- a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java +++ b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/TracerInstaller.java @@ -71,10 +71,7 @@ private static synchronized void installExportersFromJar(final String exporterJa OpenTelemetrySdk.getTracerProvider() .addSpanProcessor( BatchSpansProcessor.create( - spanExporter, - BatchSpansProcessor.Config - .loadFromDefaultSources())); // Loads configuration for SpansProcessor from - // env vars if exists. + spanExporter, BatchSpansProcessor.Config.loadFromDefaultSources())); log.info("Installed span exporter: " + spanExporter.getClass().getName()); } else { log.warn("No matching providers in jar " + exporterJar); From 4d04de7c661ecc6245ec317644d286bb40c2afd1 Mon Sep 17 00:00:00 2001 From: RashmiRam Date: Tue, 12 May 2020 15:00:12 +0530 Subject: [PATCH 5/9] Updated the timeout for play & spring boot tests to wait for batchspanprocessor to finish exporting --- .../groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy | 2 +- .../io/opentelemetry/smoketest/SpringBootSmokeTest.groovy | 2 +- .../io/opentelemetry/smoketest/AbstractSmokeTest.groovy | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy b/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy index c711a253db83..55422e690798 100644 --- a/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy +++ b/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy @@ -45,7 +45,7 @@ class PlaySmokeTest extends AbstractServerSmokeTest { setup: def spanCounter = new SpanCounter(logfile, [ (HTTP_REQUEST_SPAN): 2, - ], 10000) + ], 20000) String url = "http://localhost:$httpPort/welcome?id=$n" def request = new Request.Builder().url(url).get().build() diff --git a/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy b/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy index c4593ea463e4..f94d8f843444 100644 --- a/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy +++ b/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy @@ -39,7 +39,7 @@ class SpringBootSmokeTest extends AbstractServerSmokeTest { def spanCounter = new SpanCounter(logfile, [ (HANDLER_SPAN): 1, (SERVLET_SPAN): 1, - ], 10000) + ], 20000) String url = "http://localhost:${httpPort}/greeting" def request = new Request.Builder().url(url).get().build() diff --git a/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy b/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy index a3119c578c36..ae653bf012c3 100644 --- a/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy +++ b/smoke-tests/src/main/groovy/io/opentelemetry/smoketest/AbstractSmokeTest.groovy @@ -70,8 +70,8 @@ abstract class AbstractSmokeTest extends Specification { processBuilder.environment().put("JAVA_HOME", System.getProperty("java.home")) processBuilder.environment().put("DD_API_KEY", API_KEY) -// // Setting configuration variables of batch span processor through env vars -// // This config is to immediately flush a batch of 1 span with delay of 10ms + // Setting configuration variables of batch span processor through env vars + // This config is to immediately flush a batch of 1 span with delay of 10ms processBuilder.environment().put("OTEL_BSP_MAX_EXPORT_BATCH", "1") processBuilder.environment().put("OTEL_BSP_SCHEDULE_DELAY", "10") From 4b3b64bb442b359843089a05bcbeb0373e2dbe21 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 14 May 2020 13:58:14 -0700 Subject: [PATCH 6/9] Update smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy --- .../test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy b/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy index 55422e690798..f137285cd8e2 100644 --- a/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy +++ b/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy @@ -45,7 +45,7 @@ class PlaySmokeTest extends AbstractServerSmokeTest { setup: def spanCounter = new SpanCounter(logfile, [ (HTTP_REQUEST_SPAN): 2, - ], 20000) + ], 30000) String url = "http://localhost:$httpPort/welcome?id=$n" def request = new Request.Builder().url(url).get().build() From d40a649e904c38e9e72b38d2c3049a017b25b33f Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 14 May 2020 13:58:21 -0700 Subject: [PATCH 7/9] Update smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy --- .../io/opentelemetry/smoketest/SpringBootSmokeTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy b/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy index f94d8f843444..a0e9c43f2f8a 100644 --- a/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy +++ b/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy @@ -39,7 +39,7 @@ class SpringBootSmokeTest extends AbstractServerSmokeTest { def spanCounter = new SpanCounter(logfile, [ (HANDLER_SPAN): 1, (SERVLET_SPAN): 1, - ], 20000) + ], 30000) String url = "http://localhost:${httpPort}/greeting" def request = new Request.Builder().url(url).get().build() From 499838be5abe15e33d179b14e1a6c27569bc0699 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sat, 16 May 2020 11:55:19 -0700 Subject: [PATCH 8/9] Remove unnecessarily verbose debug logging --- .../auto/bootstrap/instrumentation/java/concurrent/State.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/java/concurrent/State.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/java/concurrent/State.java index 38a1dc2ba52f..ff12a51ee68b 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/java/concurrent/State.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/java/concurrent/State.java @@ -37,7 +37,7 @@ private State() {} public void setParentSpan(final Span parentSpan) { final boolean result = parentSpanRef.compareAndSet(null, parentSpan); - if (!result) { + if (!result && parentSpanRef.get() != parentSpan) { log.debug( "Failed to set parent span because another parent span is already set {}: new: {}, old: {}", this, From df978011abccd6086a4c58c8c726df448f46ca59 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sat, 16 May 2020 12:13:45 -0700 Subject: [PATCH 9/9] Revert timeout change --- .../test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy | 2 +- .../io/opentelemetry/smoketest/SpringBootSmokeTest.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy b/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy index f137285cd8e2..c711a253db83 100644 --- a/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy +++ b/smoke-tests/play/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy @@ -45,7 +45,7 @@ class PlaySmokeTest extends AbstractServerSmokeTest { setup: def spanCounter = new SpanCounter(logfile, [ (HTTP_REQUEST_SPAN): 2, - ], 30000) + ], 10000) String url = "http://localhost:$httpPort/welcome?id=$n" def request = new Request.Builder().url(url).get().build() diff --git a/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy b/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy index a0e9c43f2f8a..c4593ea463e4 100644 --- a/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy +++ b/smoke-tests/springboot/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy @@ -39,7 +39,7 @@ class SpringBootSmokeTest extends AbstractServerSmokeTest { def spanCounter = new SpanCounter(logfile, [ (HANDLER_SPAN): 1, (SERVLET_SPAN): 1, - ], 30000) + ], 10000) String url = "http://localhost:${httpPort}/greeting" def request = new Request.Builder().url(url).get().build()