Fix some instrumentation scope names (#7632)

open-telemetry · Jan 24, 2023 · adbd966 · adbd966
1 parent bb092bf
commit adbd966
Show file tree

Hide file tree

Showing 22 changed files with 96 additions and 48 deletions.
diff --git a/instrumentation/azure-core/azure-core-1.14/javaagent/src/test/java/AzureSdkTest.java b/instrumentation/azure-core/azure-core-1.14/javaagent/src/test/java/AzureSdkTest.java
@@ -46,7 +46,7 @@ void testSpan() {
     Context context = TracerProxy.start("hello", Context.NONE);
     TracerProxy.end(200, null, context);
 
-    testing.waitAndAssertTraces(
+    testing.waitAndAssertTracesWithoutScopeVersionVerification(
         trace ->
             trace.hasSpansSatisfyingExactly(
                 span ->

diff --git a/instrumentation/azure-core/azure-core-1.19/javaagent/src/test/java/AzureSdkTest.java b/instrumentation/azure-core/azure-core-1.19/javaagent/src/test/java/AzureSdkTest.java
@@ -46,7 +46,7 @@ void testSpan() {
     Context context = TracerProxy.start("hello", Context.NONE);
     TracerProxy.end(200, null, context);
 
-    testing.waitAndAssertTraces(
+    testing.waitAndAssertTracesWithoutScopeVersionVerification(
         trace ->
             trace.hasSpansSatisfyingExactly(
                 span ->

diff --git a/...ntation/couchbase/couchbase-3.1.6/javaagent/src/test/groovy/CouchbaseClient316Test.groovy b/...ntation/couchbase/couchbase-3.1.6/javaagent/src/test/groovy/CouchbaseClient316Test.groovy
@@ -57,7 +57,7 @@ class CouchbaseClient316Test extends AgentInstrumentationSpecification {
     }
 
     then:
-    assertTraces(1) {
+    assertTracesWithoutScopeVersionVerification(1) {
       trace(0, 2) {
         span(0) {
           name(~/.*get/)

diff --git a/...umentation/couchbase/couchbase-3.1/javaagent/src/test/groovy/CouchbaseClient31Test.groovy b/...umentation/couchbase/couchbase-3.1/javaagent/src/test/groovy/CouchbaseClient31Test.groovy
@@ -66,7 +66,7 @@ class CouchbaseClient31Test extends AgentInstrumentationSpecification {
     }
 
     then:
-    assertTraces(1) {
+    assertTracesWithoutScopeVersionVerification(1) {
       trace(0, 2) {
         span(0) {
           name(~/.*get/)

diff --git a/...umentation/couchbase/couchbase-3.2/javaagent/src/test/groovy/CouchbaseClient32Test.groovy b/...umentation/couchbase/couchbase-3.2/javaagent/src/test/groovy/CouchbaseClient32Test.groovy
@@ -59,7 +59,7 @@ class CouchbaseClient32Test extends AgentInstrumentationSpecification {
     }
 
     then:
-    assertTraces(1) {
+    assertTracesWithoutScopeVersionVerification(1) {
       trace(0, 2) {
         span(0) {
           name(~/.*get/)

diff --git a/...java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/Hibernate43Singletons.java b/...java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/Hibernate43Singletons.java
@@ -12,7 +12,8 @@
 public final class Hibernate43Singletons {
 
   private static final Instrumenter<HibernateOperation, Void> INSTANCE =
-      HibernateInstrumenterFactory.createInstrumenter("io.opentelemetry.hibernate-4.3");
+      HibernateInstrumenterFactory.createInstrumenter(
+          "io.opentelemetry.hibernate-procedure-call-4.3");
 
   public static Instrumenter<HibernateOperation, Void> instrumenter() {
     return INSTANCE;

diff --git a/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfSingletons.java b/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfSingletons.java
@@ -12,7 +12,7 @@
 public final class CxfSingletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.cxf-jaxrs-3.2");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-2.0-cxf-3.2");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseySingletons.java b/...src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseySingletons.java
@@ -12,7 +12,7 @@
 public final class JerseySingletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jersey-2.0");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-2.0-jersey-2.0");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/Resteasy30Singletons.java b/...main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/Resteasy30Singletons.java
@@ -12,7 +12,7 @@
 public final class Resteasy30Singletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.resteasy-3.0");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-2.0-resteasy-3.0");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/Resteasy31Singletons.java b/...main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/Resteasy31Singletons.java
@@ -12,7 +12,7 @@
 public final class Resteasy31Singletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.resteasy-3.1");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-2.0-resteasy-3.1");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/jaxrs/v3_0/JaxrsAnnotationsSingletons.java b/...ava/io/opentelemetry/javaagent/instrumentation/jaxrs/v3_0/JaxrsAnnotationsSingletons.java
@@ -12,7 +12,7 @@
 public final class JaxrsAnnotationsSingletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-annotations-3.0");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-3.0-annotations");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v3_0/JerseySingletons.java b/...src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v3_0/JerseySingletons.java
@@ -12,7 +12,7 @@
 public final class JerseySingletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jersey-2.0");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-3.0-jersey-3.0");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...c/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v3_0/ResteasySingletons.java b/...c/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v3_0/ResteasySingletons.java
@@ -12,7 +12,7 @@
 public final class ResteasySingletons {
 
   private static final Instrumenter<HandlerData, Void> INSTANCE =
-      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.resteasy-6.0");
+      JaxrsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxrs-3.0-resteasy-6.0");
 
   public static Instrumenter<HandlerData, Void> instrumenter() {
     return INSTANCE;

diff --git a/...rc/main/java/io/opentelemetry/javaagent/instrumentation/jaxws/jws/v1_1/JwsSingletons.java b/...rc/main/java/io/opentelemetry/javaagent/instrumentation/jaxws/jws/v1_1/JwsSingletons.java
@@ -12,7 +12,7 @@
 public final class JwsSingletons {
 
   private static final Instrumenter<JaxWsRequest, Void> INSTANCE =
-      JaxWsInstrumenterFactory.createInstrumenter("io.opentelemetry.jws-1.1");
+      JaxWsInstrumenterFactory.createInstrumenter("io.opentelemetry.jaxws-jws-api-1.1");
 
   public static Instrumenter<JaxWsRequest, Void> instrumenter() {
     return INSTANCE;

diff --git a/...ent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java b/...ent/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/LogbackTest.java
@@ -139,9 +139,7 @@ private static void test(
       LogRecordData log = testing.logRecords().get(0);
       assertThat(log)
           .hasBody("xyz: 123")
-          // TODO (trask) why is version "" instead of null?
-          .hasInstrumentationScope(
-              InstrumentationScopeInfo.builder(expectedLoggerName).setVersion("").build())
+          .hasInstrumentationScope(InstrumentationScopeInfo.builder(expectedLoggerName).build())
           .hasSeverity(expectedSeverity)
           .hasSeverityText(expectedSeverityText);
       if (logException) {
@@ -201,8 +199,7 @@ void testMdc() {
     LogRecordData log = getLogRecords().get(0);
     assertThat(log)
         .hasBody("xyz: 123")
-        // TODO (trask) why is version "" instead of null?
-        .hasInstrumentationScope(InstrumentationScopeInfo.builder("abc").setVersion("").build())
+        .hasInstrumentationScope(InstrumentationScopeInfo.builder("abc").build())
         .hasSeverity(Severity.INFO)
         .hasSeverityText("INFO")
         // TODO (trask) convert to hasAttributesSatisfyingExactly once that's available for logs

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/vertx/kafka/v3_6/VertxKafkaSingletons.java b/...ava/io/opentelemetry/javaagent/instrumentation/vertx/kafka/v3_6/VertxKafkaSingletons.java
@@ -15,7 +15,7 @@
 
 public final class VertxKafkaSingletons {
 
-  private static final String INSTRUMENTATION_NAME = "io.opentelemetry.vertx-kafka-client-3.5";
+  private static final String INSTRUMENTATION_NAME = "io.opentelemetry.vertx-kafka-client-3.6";
 
   private static final Instrumenter<ConsumerRecords<?, ?>, Void> BATCH_PROCESS_INSTRUMENTER;
   private static final Instrumenter<ConsumerRecord<?, ?>, Void> PROCESS_INSTRUMENTER;

diff --git a/...src/main/groovy/io/opentelemetry/instrumentation/test/InstrumentationSpecification.groovy b/...src/main/groovy/io/opentelemetry/instrumentation/test/InstrumentationSpecification.groovy
@@ -94,7 +94,7 @@ abstract class InstrumentationSpecification extends Specification {
    * 20 seconds, then times out.
    */
   List<List<SpanData>> waitForTraces(int numberOfTraces) {
-    TelemetryDataUtil.waitForTraces({ testRunner().getExportedSpans() }, numberOfTraces)
+    TelemetryDataUtil.waitForTraces({ testRunner().getExportedSpans() }, numberOfTraces, true)
   }
 
   void ignoreTracesAndClear(int numberOfTraces) {
@@ -109,9 +109,20 @@ abstract class InstrumentationSpecification extends Specification {
       options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
     @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST)
     final Closure spec) {
-    InMemoryExporterAssert.assertTraces({ testRunner().getExportedSpans() }, size, spec)
+    InMemoryExporterAssert.assertTraces({ testRunner().getExportedSpans() }, size, spec, true)
   }
 
+  void assertTracesWithoutScopeVersionVerification(
+    final int size,
+    @ClosureParams(
+      value = SimpleType,
+      options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
+    @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST)
+    final Closure spec) {
+    InMemoryExporterAssert.assertTraces({ testRunner().getExportedSpans() }, size, spec, false)
+  }
+
+
   /**
    * Runs the provided {@code callback} inside the scope of an INTERNAL span with name {@code
    * spanName}.

diff --git a/...c/main/groovy/io/opentelemetry/instrumentation/test/asserts/InMemoryExporterAssert.groovy b/...c/main/groovy/io/opentelemetry/instrumentation/test/asserts/InMemoryExporterAssert.groovy
@@ -32,9 +32,10 @@ class InMemoryExporterAssert {
 
   static void assertTraces(Supplier<List<SpanData>> spanSupplier, int expectedSize,
                            @ClosureParams(value = SimpleType, options = ['io.opentelemetry.instrumentation.test.asserts.ListWriterAssert'])
-                           @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST) Closure spec) {
+                           @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST) Closure spec,
+                           boolean verifyScopeVersion) {
     try {
-      def traces = TelemetryDataUtil.waitForTraces(spanSupplier, expectedSize)
+      def traces = TelemetryDataUtil.waitForTraces(spanSupplier, expectedSize, verifyScopeVersion)
       assert traces.size() == expectedSize
       def asserter = new InMemoryExporterAssert(traces, spanSupplier)
       def clone = (Closure) spec.clone()

diff --git a/...mon/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java b/...mon/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java
@@ -62,10 +62,10 @@ public final List<List<SpanData>> traces() {
     return TelemetryDataUtil.groupTraces(getExportedSpans());
   }
 
-  public final List<List<SpanData>> waitForTraces(int numberOfTraces) {
+  public final List<List<SpanData>> waitForTraces(int numberOfTraces, boolean verifyScopeVersion) {
     try {
       return TelemetryDataUtil.waitForTraces(
-          this::getExportedSpans, numberOfTraces, 20, TimeUnit.SECONDS);
+          this::getExportedSpans, numberOfTraces, 20, TimeUnit.SECONDS, verifyScopeVersion);
     } catch (TimeoutException | InterruptedException e) {
       throw new AssertionError("Error waiting for " + numberOfTraces + " traces", e);
     }
@@ -75,7 +75,19 @@ public final List<List<SpanData>> waitForTraces(int numberOfTraces) {
   @SuppressWarnings("varargs")
   public final void waitAndAssertSortedTraces(
       Comparator<List<SpanData>> traceComparator, Consumer<TraceAssert>... assertions) {
-    waitAndAssertTraces(traceComparator, Arrays.asList(assertions));
+    waitAndAssertTraces(traceComparator, Arrays.asList(assertions), true);
+  }
+
+  @SafeVarargs
+  @SuppressWarnings("varargs")
+  public final void waitAndAssertTracesWithoutScopeVersionVerification(
+      Consumer<TraceAssert>... assertions) {
+    waitAndAssertTracesWithoutScopeVersionVerification(Arrays.asList(assertions));
+  }
+
+  public final <T extends Consumer<TraceAssert>>
+      void waitAndAssertTracesWithoutScopeVersionVerification(Iterable<T> assertions) {
+    waitAndAssertTraces(null, assertions, false);
   }
 
   @SafeVarargs
@@ -85,28 +97,33 @@ public final void waitAndAssertTraces(Consumer<TraceAssert>... assertions) {
   }
 
   public final <T extends Consumer<TraceAssert>> void waitAndAssertTraces(Iterable<T> assertions) {
-    waitAndAssertTraces(null, assertions);
+    waitAndAssertTraces(null, assertions, true);
   }
 
   private <T extends Consumer<TraceAssert>> void waitAndAssertTraces(
-      @Nullable Comparator<List<SpanData>> traceComparator, Iterable<T> assertions) {
+      @Nullable Comparator<List<SpanData>> traceComparator,
+      Iterable<T> assertions,
+      boolean verifyScopeVersion) {
     List<T> assertionsList = new ArrayList<>();
     assertions.forEach(assertionsList::add);
 
     try {
-      await().untilAsserted(() -> doAssertTraces(traceComparator, assertionsList));
+      await()
+          .untilAsserted(() -> doAssertTraces(traceComparator, assertionsList, verifyScopeVersion));
     } catch (ConditionTimeoutException e) {
       // Don't throw this failure since the stack is the awaitility thread, causing confusion.
       // Instead, just assert one more time on the test thread, which will fail with a better stack
       // trace.
       // TODO(anuraaga): There is probably a better way to do this.
-      doAssertTraces(traceComparator, assertionsList);
+      doAssertTraces(traceComparator, assertionsList, verifyScopeVersion);
     }
   }
 
   private <T extends Consumer<TraceAssert>> void doAssertTraces(
-      @Nullable Comparator<List<SpanData>> traceComparator, List<T> assertionsList) {
-    List<List<SpanData>> traces = waitForTraces(assertionsList.size());
+      @Nullable Comparator<List<SpanData>> traceComparator,
+      List<T> assertionsList,
+      boolean verifyScopeVersion) {
+    List<List<SpanData>> traces = waitForTraces(assertionsList.size(), verifyScopeVersion);
     if (traceComparator != null) {
       traces.sort(traceComparator);
     }

diff --git a/...rc/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java b/...rc/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java
@@ -113,7 +113,7 @@ public void clearData() {
    * 20 seconds, then times out.
    */
   public List<List<SpanData>> waitForTraces(int numberOfTraces) {
-    return testRunner.waitForTraces(numberOfTraces);
+    return testRunner.waitForTraces(numberOfTraces, true);
   }
 
   @SafeVarargs
@@ -123,6 +123,13 @@ public final void waitAndAssertSortedTraces(
     testRunner.waitAndAssertSortedTraces(traceComparator, assertions);
   }
 
+  @SafeVarargs
+  @SuppressWarnings("varargs")
+  public final void waitAndAssertTracesWithoutScopeVersionVerification(
+      Consumer<TraceAssert>... assertions) {
+    testRunner.waitAndAssertTracesWithoutScopeVersionVerification(assertions);
+  }
+
   @SafeVarargs
   @SuppressWarnings("varargs")
   public final void waitAndAssertTraces(Consumer<TraceAssert>... assertions) {

diff --git a/...common/src/main/java/io/opentelemetry/instrumentation/testing/util/TelemetryDataUtil.java b/...common/src/main/java/io/opentelemetry/instrumentation/testing/util/TelemetryDataUtil.java
@@ -46,13 +46,18 @@ public static List<List<SpanData>> groupTraces(List<SpanData> spans) {
     return traces;
   }
 
-  public static List<List<SpanData>> waitForTraces(Supplier<List<SpanData>> supplier, int number)
+  public static List<List<SpanData>> waitForTraces(
+      Supplier<List<SpanData>> supplier, int number, boolean verifyScopeVersion)
       throws InterruptedException, TimeoutException {
-    return waitForTraces(supplier, number, 20, TimeUnit.SECONDS);
+    return waitForTraces(supplier, number, 20, TimeUnit.SECONDS, verifyScopeVersion);
   }
 
   public static List<List<SpanData>> waitForTraces(
-      Supplier<List<SpanData>> supplier, int number, long timeout, TimeUnit unit)
+      Supplier<List<SpanData>> supplier,
+      int number,
+      long timeout,
+      TimeUnit unit,
+      boolean verifyScopeVersion)
       throws InterruptedException, TimeoutException {
     long startTime = System.nanoTime();
     List<List<SpanData>> allTraces = groupTraces(supplier.get());
@@ -74,14 +79,16 @@ public static List<List<SpanData>> waitForTraces(
               + " total trace(s): "
               + allTraces);
     }
-    // TODO (trask) is there a better location for this assertion?
-    for (List<SpanData> trace : completeTraces) {
-      for (SpanData span : trace) {
-        if (!span.getInstrumentationScopeInfo().getName().equals("test")) {
-          assertThat(span.getInstrumentationScopeInfo().getVersion())
-              .as(
-                  "Instrumentation version was empty; make sure that the instrumentation name matches the gradle module name")
-              .isNotNull();
+    if (verifyScopeVersion) {
+      // TODO (trask) is there a better location for this assertion?
+      for (List<SpanData> trace : completeTraces) {
+        for (SpanData span : trace) {
+          if (!span.getInstrumentationScopeInfo().getName().equals("test")) {
+            assertThat(span.getInstrumentationScopeInfo().getVersion())
+                .as(
+                    "Instrumentation version was empty; make sure that the instrumentation name matches the gradle module name")
+                .isNotNull();
+          }
         }
       }
     }

diff --git a/...n/src/main/java/io/opentelemetry/javaagent/testing/common/AgentTestingExporterAccess.java b/...n/src/main/java/io/opentelemetry/javaagent/testing/common/AgentTestingExporterAccess.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.testing.common;
 
+import static com.google.common.base.Strings.emptyToNull;
 import static io.opentelemetry.api.common.AttributeKey.booleanArrayKey;
 import static io.opentelemetry.api.common.AttributeKey.doubleArrayKey;
 import static io.opentelemetry.api.common.AttributeKey.longArrayKey;
@@ -189,7 +190,9 @@ public static List<SpanData> getExportedSpans() {
                           fromProto(resource.getAttributesList())))
                   .setInstrumentationScopeInfo(
                       InstrumentationScopeInfo.builder(instrumentationScope.getName())
-                          .setVersion(instrumentationScope.getVersion())
+                          // emptyToNull since they are the same at protobuf layer,
+                          // and allows for simpler verification of InstrumentationScope
+                          .setVersion(emptyToNull(instrumentationScope.getVersion()))
                           .build())
                   .setName(span.getName())
                   .setStartEpochNanos(span.getStartTimeUnixNano())
@@ -267,7 +270,9 @@ public static List<MetricData> getExportedMetrics() {
                   io.opentelemetry.sdk.resources.Resource.create(
                       fromProto(resource.getAttributesList())),
                   InstrumentationScopeInfo.builder(instrumentationScope.getName())
-                      .setVersion(instrumentationScope.getVersion())
+                      // emptyToNull since they are the same at protobuf layer,
+                      // and allows for simpler verification of InstrumentationScope
+                      .setVersion(emptyToNull(instrumentationScope.getVersion()))
                       .build()));
         }
       }
@@ -308,7 +313,9 @@ public static List<LogRecordData> getExportedLogRecords() {
                   io.opentelemetry.sdk.resources.Resource.create(
                       fromProto(resource.getAttributesList())),
                   InstrumentationScopeInfo.builder(instrumentationScope.getName())
-                      .setVersion(instrumentationScope.getVersion())
+                      // emptyToNull since they are the same at protobuf layer,
+                      // and allows for simpler verification of InstrumentationScope
+                      .setVersion(emptyToNull(instrumentationScope.getVersion()))
                       .build()));
         }
       }