From e8e573c2ee5bdc22dda9dca67f453688a1b21367 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 13 Sep 2022 07:22:43 -0700 Subject: [PATCH] Add experimental span attribute job.system (#6586) * Add experimental span attribute job.system * spring-batch --- .../quartz-2.0/javaagent/build.gradle.kts | 5 ++ .../quartz/v2_0/QuartzSingletons.java | 8 ++- .../quartz/v2_0/QuartzTelemetryBuilder.java | 18 +++++++ .../quartz/v2_0/QuartzTest.java | 6 ++- .../quartz/v2_0/AbstractQuartzTest.java | 30 +++++------ .../javaagent/build.gradle.kts | 2 + .../spring/batch/job/JobSingletons.java | 24 +++++++-- .../src/test/groovy/SpringBatchTest.groovy | 50 +++++++++++++++++++ .../javaagent/build.gradle.kts | 3 ++ .../SpringSchedulingSingletons.java | 21 ++++++-- .../test/groovy/SpringSchedulingTest.groovy | 4 ++ 11 files changed, 145 insertions(+), 26 deletions(-) diff --git a/instrumentation/quartz-2.0/javaagent/build.gradle.kts b/instrumentation/quartz-2.0/javaagent/build.gradle.kts index 31f78f992836..a67a4b420265 100644 --- a/instrumentation/quartz-2.0/javaagent/build.gradle.kts +++ b/instrumentation/quartz-2.0/javaagent/build.gradle.kts @@ -18,3 +18,8 @@ dependencies { testImplementation(project(":instrumentation:quartz-2.0:testing")) } + +tasks.withType().configureEach { + // TODO run tests both with and without experimental span attributes + jvmArgs("-Dotel.instrumentation.quartz.experimental-span-attributes=true") +} diff --git a/instrumentation/quartz-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quartz/v2_0/QuartzSingletons.java b/instrumentation/quartz-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quartz/v2_0/QuartzSingletons.java index edeb75312a55..b86cf9ecf2d3 100644 --- a/instrumentation/quartz-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quartz/v2_0/QuartzSingletons.java +++ b/instrumentation/quartz-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quartz/v2_0/QuartzSingletons.java @@ -6,11 +6,17 @@ package io.opentelemetry.javaagent.instrumentation.quartz.v2_0; import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.quartz.v2_0.QuartzTelemetry; public final class QuartzSingletons { - public static final QuartzTelemetry TELEMETRY = QuartzTelemetry.create(GlobalOpenTelemetry.get()); + public static final QuartzTelemetry TELEMETRY = + QuartzTelemetry.builder(GlobalOpenTelemetry.get()) + .setCaptureExperimentalSpanAttributes( + ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.quartz.experimental-span-attributes", false)) + .build(); private QuartzSingletons() {} } diff --git a/instrumentation/quartz-2.0/library/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTelemetryBuilder.java b/instrumentation/quartz-2.0/library/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTelemetryBuilder.java index af84f746cfd9..a229de396be3 100644 --- a/instrumentation/quartz-2.0/library/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTelemetryBuilder.java +++ b/instrumentation/quartz-2.0/library/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTelemetryBuilder.java @@ -6,6 +6,7 @@ package io.opentelemetry.instrumentation.quartz.v2_0; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; @@ -24,6 +25,8 @@ public final class QuartzTelemetryBuilder { private final List> additionalExtractors = new ArrayList<>(); + private boolean captureExperimentalSpanAttributes; + QuartzTelemetryBuilder(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; } @@ -38,6 +41,17 @@ public QuartzTelemetryBuilder addAttributeExtractor( return this; } + /** + * Sets whether experimental attributes should be set to spans. These attributes may be changed or + * removed in the future, so only enable this if you know you do not require attributes filled by + * this instrumentation to be stable across versions + */ + public QuartzTelemetryBuilder setCaptureExperimentalSpanAttributes( + boolean captureExperimentalSpanAttributes) { + this.captureExperimentalSpanAttributes = captureExperimentalSpanAttributes; + return this; + } + /** * Returns a new {@link QuartzTelemetry} with the settings of this {@link QuartzTelemetryBuilder}. */ @@ -45,6 +59,10 @@ public QuartzTelemetry build() { InstrumenterBuilder instrumenter = Instrumenter.builder(openTelemetry, INSTRUMENTATION_NAME, new QuartzSpanNameExtractor()); + if (captureExperimentalSpanAttributes) { + instrumenter.addAttributesExtractor( + AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "quartz")); + } instrumenter.setErrorCauseExtractor(new QuartzErrorCauseExtractor()); instrumenter.addAttributesExtractor( CodeAttributesExtractor.create(new QuartzCodeAttributesGetter())); diff --git a/instrumentation/quartz-2.0/library/src/test/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTest.java b/instrumentation/quartz-2.0/library/src/test/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTest.java index ac89a24c96cb..78f172cc46c9 100644 --- a/instrumentation/quartz-2.0/library/src/test/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTest.java +++ b/instrumentation/quartz-2.0/library/src/test/java/io/opentelemetry/instrumentation/quartz/v2_0/QuartzTest.java @@ -16,7 +16,11 @@ class QuartzTest extends AbstractQuartzTest { @Override protected void configureScheduler(Scheduler scheduler) { - QuartzTelemetry.create(testing.getOpenTelemetry()).configure(scheduler); + QuartzTelemetry.builder(testing.getOpenTelemetry()) + // TODO run tests both with and without experimental span attributes + .setCaptureExperimentalSpanAttributes(true) + .build() + .configure(scheduler); } @Override diff --git a/instrumentation/quartz-2.0/testing/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/AbstractQuartzTest.java b/instrumentation/quartz-2.0/testing/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/AbstractQuartzTest.java index d0829f7cfd7e..85694614a21f 100644 --- a/instrumentation/quartz-2.0/testing/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/AbstractQuartzTest.java +++ b/instrumentation/quartz-2.0/testing/src/main/java/io/opentelemetry/instrumentation/quartz/v2_0/AbstractQuartzTest.java @@ -5,11 +5,12 @@ package io.opentelemetry.instrumentation.quartz.v2_0; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; import static org.quartz.JobBuilder.newJob; import static org.quartz.TriggerBuilder.newTrigger; import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.sdk.trace.data.StatusData; @@ -67,14 +68,12 @@ void successfulJob() throws Exception { .hasKind(SpanKind.INTERNAL) .hasNoParent() .hasStatus(StatusData.unset()) - .hasAttributesSatisfying( - attrs -> - assertThat(attrs) - .containsEntry( - SemanticAttributes.CODE_NAMESPACE, - SuccessfulJob.class.getName()) - .containsEntry( - SemanticAttributes.CODE_FUNCTION, "execute")), + .hasAttributesSatisfyingExactly( + equalTo(AttributeKey.stringKey("job.system"), "quartz"), + equalTo( + SemanticAttributes.CODE_NAMESPACE, + SuccessfulJob.class.getName()), + equalTo(SemanticAttributes.CODE_FUNCTION, "execute")), span -> span.hasName("child") .hasKind(SpanKind.INTERNAL) @@ -99,14 +98,11 @@ void failingJob() throws Exception { .hasNoParent() .hasStatus(StatusData.error()) .hasException(new IllegalStateException("Bad job")) - .hasAttributesSatisfying( - attrs -> - assertThat(attrs) - .containsEntry( - SemanticAttributes.CODE_NAMESPACE, - FailingJob.class.getName()) - .containsEntry( - SemanticAttributes.CODE_FUNCTION, "execute")))); + .hasAttributesSatisfyingExactly( + equalTo(AttributeKey.stringKey("job.system"), "quartz"), + equalTo( + SemanticAttributes.CODE_NAMESPACE, FailingJob.class.getName()), + equalTo(SemanticAttributes.CODE_FUNCTION, "execute")))); } private static Scheduler createScheduler(String name) throws Exception { diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/build.gradle.kts b/instrumentation/spring/spring-batch-3.0/javaagent/build.gradle.kts index 2147a8486272..8a264755b4a1 100644 --- a/instrumentation/spring/spring-batch-3.0/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-batch-3.0/javaagent/build.gradle.kts @@ -53,6 +53,8 @@ tasks { withType().configureEach { systemProperty("testLatestDeps", findProperty("testLatestDeps")) jvmArgs("-Dotel.instrumentation.spring-batch.enabled=true") + // TODO run tests both with and without experimental span attributes + jvmArgs("-Dotel.instrumentation.spring-batch.experimental-span-attributes=true") } } diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/job/JobSingletons.java b/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/job/JobSingletons.java index 9e8c4f6e024b..23199c6b7430 100644 --- a/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/job/JobSingletons.java +++ b/instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/job/JobSingletons.java @@ -8,15 +8,31 @@ import static io.opentelemetry.javaagent.instrumentation.spring.batch.SpringBatchInstrumentationConfig.instrumentationName; import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; +import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import org.springframework.batch.core.JobExecution; public class JobSingletons { - private static final Instrumenter INSTRUMENTER = - Instrumenter.builder( - GlobalOpenTelemetry.get(), instrumentationName(), JobSingletons::extractSpanName) - .buildInstrumenter(); + private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES = + ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.spring-batch.experimental-span-attributes", false); + + private static final Instrumenter INSTRUMENTER; + + static { + InstrumenterBuilder instrumenter = + Instrumenter.builder( + GlobalOpenTelemetry.get(), instrumentationName(), JobSingletons::extractSpanName); + if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) { + instrumenter.addAttributesExtractor( + AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "spring_batch")); + } + INSTRUMENTER = instrumenter.buildInstrumenter(); + } private static String extractSpanName(JobExecution jobExecution) { return "BatchJob " + jobExecution.getJobInstance().getJobName(); diff --git a/instrumentation/spring/spring-batch-3.0/javaagent/src/test/groovy/SpringBatchTest.groovy b/instrumentation/spring/spring-batch-3.0/javaagent/src/test/groovy/SpringBatchTest.groovy index d6148be5448a..d91d38ca6ba9 100644 --- a/instrumentation/spring/spring-batch-3.0/javaagent/src/test/groovy/SpringBatchTest.groovy +++ b/instrumentation/spring/spring-batch-3.0/javaagent/src/test/groovy/SpringBatchTest.groovy @@ -27,16 +27,21 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob taskletJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { name "BatchJob taskletJob.step" kind INTERNAL childOf span(0) + attributes {} } span(2) { name "BatchJob taskletJob.step.Tasklet" kind INTERNAL childOf span(1) + attributes {} } } } @@ -52,11 +57,15 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob taskletJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { name "BatchJob taskletJob.step" kind INTERNAL childOf span(0) + attributes {} } span(2) { name "BatchJob taskletJob.step.Tasklet" @@ -64,6 +73,7 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { childOf span(1) status ERROR errorEvent IllegalStateException, "fail" + attributes {} } } } @@ -79,36 +89,45 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob itemsAndTaskletJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { name "BatchJob itemsAndTaskletJob.itemStep" kind INTERNAL childOf span(0) + attributes {} } span(2) { name "BatchJob itemsAndTaskletJob.itemStep.Chunk" kind INTERNAL childOf span(1) + attributes {} } span(3) { name "BatchJob itemsAndTaskletJob.itemStep.Chunk" kind INTERNAL childOf span(1) + attributes {} } span(4) { name "BatchJob itemsAndTaskletJob.itemStep.Chunk" kind INTERNAL childOf span(1) + attributes {} } span(5) { name "BatchJob itemsAndTaskletJob.taskletStep" kind INTERNAL childOf span(0) + attributes {} } span(6) { name "BatchJob itemsAndTaskletJob.taskletStep.Tasklet" kind INTERNAL childOf span(5) + attributes {} } } } @@ -124,26 +143,33 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob flowJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { name "BatchJob flowJob.flowStep1" kind INTERNAL childOf span(0) + attributes {} } span(2) { name "BatchJob flowJob.flowStep1.Tasklet" kind INTERNAL childOf span(1) + attributes {} } span(3) { name "BatchJob flowJob.flowStep2" kind INTERNAL childOf span(0) + attributes {} } span(4) { name "BatchJob flowJob.flowStep2.Tasklet" kind INTERNAL childOf span(3) + attributes {} } } } @@ -159,26 +185,33 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob splitJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { name ~/BatchJob splitJob\.splitFlowStep[12]/ kind INTERNAL childOf span(0) + attributes {} } span(2) { name ~/BatchJob splitJob\.splitFlowStep[12]\.Tasklet/ kind INTERNAL childOf span(1) + attributes {} } span(3) { name ~/BatchJob splitJob\.splitFlowStep[12]/ kind INTERNAL childOf span(0) + attributes {} } span(4) { name ~/BatchJob splitJob\.splitFlowStep[12]\.Tasklet/ kind INTERNAL childOf span(3) + attributes {} } } } @@ -194,26 +227,33 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob decisionJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { name "BatchJob decisionJob.decisionStepStart" kind INTERNAL childOf span(0) + attributes {} } span(2) { name "BatchJob decisionJob.decisionStepStart.Tasklet" kind INTERNAL childOf span(1) + attributes {} } span(3) { name "BatchJob decisionJob.decisionStepLeft" kind INTERNAL childOf span(0) + attributes {} } span(4) { name "BatchJob decisionJob.decisionStepLeft.Tasklet" kind INTERNAL childOf span(3) + attributes {} } } } @@ -229,42 +269,52 @@ abstract class SpringBatchTest extends AgentInstrumentationSpecification { span(0) { name "BatchJob partitionedJob" kind INTERNAL + attributes { + "job.system" "spring_batch" + } } span(1) { def stepName = hasPartitionManagerStep() ? "partitionManagerStep" : "partitionWorkerStep" name "BatchJob partitionedJob.$stepName" kind INTERNAL childOf span(0) + attributes {} } span(2) { name ~/BatchJob partitionedJob.partitionWorkerStep:partition[01]/ kind INTERNAL childOf span(1) + attributes {} } span(3) { name ~/BatchJob partitionedJob.partitionWorkerStep:partition[01].Chunk/ kind INTERNAL childOf span(2) + attributes {} } span(4) { name ~/BatchJob partitionedJob.partitionWorkerStep:partition[01].Chunk/ kind INTERNAL childOf span(2) + attributes {} } span(5) { name ~/BatchJob partitionedJob.partitionWorkerStep:partition[01]/ kind INTERNAL childOf span(1) + attributes {} } span(6) { name ~/BatchJob partitionedJob.partitionWorkerStep:partition[01].Chunk/ kind INTERNAL childOf span(5) + attributes {} } span(7) { name ~/BatchJob partitionedJob.partitionWorkerStep:partition[01].Chunk/ kind INTERNAL childOf span(5) + attributes {} } } } diff --git a/instrumentation/spring/spring-scheduling-3.1/javaagent/build.gradle.kts b/instrumentation/spring/spring-scheduling-3.1/javaagent/build.gradle.kts index 6fb6eea422dd..91ee94cf095c 100644 --- a/instrumentation/spring/spring-scheduling-3.1/javaagent/build.gradle.kts +++ b/instrumentation/spring/spring-scheduling-3.1/javaagent/build.gradle.kts @@ -19,6 +19,9 @@ dependencies { } tasks.withType().configureEach { + // TODO run tests both with and without experimental span attributes + jvmArgs("-Dotel.instrumentation.spring-scheduling.experimental-span-attributes=true") + // required on jdk17 jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED") jvmArgs("-XX:+IgnoreUnrecognizedVMOptions") diff --git a/instrumentation/spring/spring-scheduling-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/scheduling/SpringSchedulingSingletons.java b/instrumentation/spring/spring-scheduling-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/scheduling/SpringSchedulingSingletons.java index b2b4cc8a9b89..ff863d657f65 100644 --- a/instrumentation/spring/spring-scheduling-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/scheduling/SpringSchedulingSingletons.java +++ b/instrumentation/spring/spring-scheduling-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/scheduling/SpringSchedulingSingletons.java @@ -6,24 +6,39 @@ package io.opentelemetry.javaagent.instrumentation.spring.scheduling; import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor; +import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; public final class SpringSchedulingSingletons { + private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES = + ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.spring-scheduling.experimental-span-attributes", false); + private static final Instrumenter INSTRUMENTER; static { SpringSchedulingCodeAttributesGetter codeAttributesGetter = new SpringSchedulingCodeAttributesGetter(); - INSTRUMENTER = + + InstrumenterBuilder builder = Instrumenter.builder( GlobalOpenTelemetry.get(), "io.opentelemetry.spring-scheduling-3.1", CodeSpanNameExtractor.create(codeAttributesGetter)) - .addAttributesExtractor(CodeAttributesExtractor.create(codeAttributesGetter)) - .buildInstrumenter(); + .addAttributesExtractor(CodeAttributesExtractor.create(codeAttributesGetter)); + + if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) { + builder.addAttributesExtractor( + AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "spring_scheduling")); + } + + INSTRUMENTER = builder.buildInstrumenter(); } public static Instrumenter instrumenter() { diff --git a/instrumentation/spring/spring-scheduling-3.1/javaagent/src/test/groovy/SpringSchedulingTest.groovy b/instrumentation/spring/spring-scheduling-3.1/javaagent/src/test/groovy/SpringSchedulingTest.groovy index 32f5a6077dd0..0b1f3cfbdeeb 100644 --- a/instrumentation/spring/spring-scheduling-3.1/javaagent/src/test/groovy/SpringSchedulingTest.groovy +++ b/instrumentation/spring/spring-scheduling-3.1/javaagent/src/test/groovy/SpringSchedulingTest.groovy @@ -26,6 +26,7 @@ class SpringSchedulingTest extends AgentInstrumentationSpecification { name "TriggerTask.run" hasNoParent() attributes { + "job.system" "spring_scheduling" "code.namespace" "TriggerTask" "code.function" "run" } @@ -49,6 +50,7 @@ class SpringSchedulingTest extends AgentInstrumentationSpecification { name "IntervalTask.run" hasNoParent() attributes { + "job.system" "spring_scheduling" "code.namespace" "IntervalTask" "code.function" "run" } @@ -71,6 +73,7 @@ class SpringSchedulingTest extends AgentInstrumentationSpecification { nameContains "LambdaTaskConfigurer\$\$Lambda\$" hasNoParent() attributes { + "job.system" "spring_scheduling" "code.namespace" { it.contains("LambdaTaskConfigurer\$\$Lambda\$") } "code.function" "run" } @@ -98,6 +101,7 @@ class SpringSchedulingTest extends AgentInstrumentationSpecification { name "EnhancedClassTaskConfig.run" hasNoParent() attributes { + "job.system" "spring_scheduling" "code.namespace" "EnhancedClassTaskConfig" "code.function" "run" }