Merge pull request #267 from solarwinds/cc/NH-91749

NH-91749: use the new extended span processor
solarwinds · Sep 30, 2024 · 2aa7bee · 2aa7bee
2 parents 6392f24 + a697040
commit 2aa7bee
Show file tree

Hide file tree

Showing 14 changed files with 106 additions and 126 deletions.
diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml
@@ -207,6 +207,16 @@ jobs:
           cd smoke-tests
           ./gradlew build -x test
 
+      - name: Build webmvc jar
+        run: |
+          cd smoke-tests
+          ./gradlew :spring-boot-webmvc:build
+
+      - name: Build webmvc image
+        run: |
+          cd smoke-tests/spring-boot-webmvc
+          docker image build --tag smt:webmvc .
+
       - name: Docker login
         run: echo "${{ secrets.GITHUB_TOKEN }}" | docker login ghcr.io -u $GITHUB_USERNAME --password-stdin
 

diff --git a/custom/lambda/build.gradle b/custom/lambda/build.gradle
@@ -34,6 +34,7 @@ dependencies {
 
   compileOnly("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api:${versions.opentelemetryJavaagent}")
   compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-extension-api:${versions.opentelemetryJavaagentAlpha}")
+  compileOnly project(path: ":bootstrap")
 
   testImplementation project(path: ":custom:shared")
   testImplementation "org.json:json:${versions.json}"

diff --git a/...main/java/com/solarwinds/opentelemetry/extensions/InboundMeasurementMetricsGenerator.java b/...main/java/com/solarwinds/opentelemetry/extensions/InboundMeasurementMetricsGenerator.java
@@ -31,11 +31,11 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.trace.ReadWriteSpan;
 import io.opentelemetry.sdk.trace.ReadableSpan;
-import io.opentelemetry.sdk.trace.SpanProcessor;
 import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor;
 import io.opentelemetry.semconv.SemanticAttributes;
 
-public class InboundMeasurementMetricsGenerator implements SpanProcessor {
+public class InboundMeasurementMetricsGenerator implements ExtendedSpanProcessor {
   private LongHistogram responseTime;
 
   private static final Logger logger = LoggerFactory.getLogger();
@@ -109,4 +109,18 @@ public void onEnd(ReadableSpan span) {
   public boolean isEndRequired() {
     return true;
   }
+
+  @Override
+  public void onEnding(ReadWriteSpan span) {
+    SpanData spanData = span.toSpanData();
+    final SpanContext parentSpanContext = spanData.getParentSpanContext();
+    if (!parentSpanContext.isValid() || parentSpanContext.isRemote()) {
+      span.setAttribute(TRANSACTION_NAME_KEY, TransactionNameManager.getTransactionName(spanData));
+    }
+  }
+
+  @Override
+  public boolean isOnEndingRequired() {
+    return true;
+  }
 }
diff --git a/custom/lambda/src/main/java/com/solarwinds/opentelemetry/extensions/LambdaAgentListener.java b/custom/lambda/src/main/java/com/solarwinds/opentelemetry/extensions/LambdaAgentListener.java
@@ -23,7 +23,6 @@
 import com.solarwinds.joboe.logging.Logger;
 import com.solarwinds.joboe.logging.LoggerFactory;
 import com.solarwinds.joboe.sampling.SettingsManager;
-import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.javaagent.extension.AgentListener;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
@@ -42,14 +41,6 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk openTelemetrySdk) {
       SettingsManager.initialize(
           new AwsLambdaSettingsFetcher(new FileSettingsReader("/tmp/solarwinds-apm-settings.json")),
           SamplingConfigProvider.getSamplingConfiguration());
-
-      TransactionNameManager.setRuntimeNameGenerator(
-          spanData ->
-              new TransactionNameManager.TransactionNameResult(
-                  spanData
-                      .getAttributes()
-                      .get(AttributeKey.stringKey(SharedNames.TRANSACTION_NAME_KEY)),
-                  true));
       logger.info("Successfully submitted SolarwindsAPM OpenTelemetry extensions settings");
     } else {
       logger.info("SolarwindsAPM OpenTelemetry extensions is disabled");

diff --git a/.../shared/src/main/java/com/solarwinds/opentelemetry/extensions/TransactionNameManager.java b/.../shared/src/main/java/com/solarwinds/opentelemetry/extensions/TransactionNameManager.java
@@ -36,10 +36,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.regex.Pattern;
-import lombok.Getter;
-import lombok.RequiredArgsConstructor;
-import lombok.Setter;
-import lombok.Value;
 
 public class TransactionNameManager {
   private static final Logger logger = LoggerFactory.getLogger();
@@ -64,11 +60,6 @@ public class TransactionNameManager {
 
   private static NamingScheme namingScheme = new DefaultNamingScheme(null);
 
-  @Getter @Setter
-  private static RuntimeNameGenerator runtimeNameGenerator =
-      (SpanData spanData) ->
-          new TransactionNameResult(CustomTransactionNameDict.get(spanData.getTraceId()), false);
-
   static {
     customTransactionNamePattern = getTransactionNamePattern();
     addNameCountChangeListener();
@@ -128,12 +119,7 @@ static String[] parseTransactionNamePattern(String pattern) {
    * @return transaction name
    */
   public static String getTransactionName(SpanData spanData) {
-    TransactionNameResult transactionNameResult = buildTransactionName(spanData);
-    String transactionName = transactionNameResult.name;
-
-    if (transactionNameResult.isLambda()) {
-      return transactionName;
-    }
+    String transactionName = buildTransactionName(spanData);
 
     if (transactionName != null) {
       Boolean domainPrefixedTransactionName =
@@ -203,34 +189,34 @@ static String transformTransactionName(String inputTransactionName) {
     return transactionName;
   }
 
-  static TransactionNameResult buildTransactionName(SpanData spanData) {
+  static String buildTransactionName(SpanData spanData) {
     Attributes spanAttributes = spanData.getAttributes();
-    TransactionNameResult transactionNameResult = runtimeNameGenerator.generateName(spanData);
+    String custName = CustomTransactionNameDict.get(spanData.getTraceId());
 
-    if (transactionNameResult.name != null) {
-      logger.trace(String.format("Using custom transaction name -> %s", transactionNameResult));
-      return transactionNameResult;
+    if (custName != null) {
+      logger.trace(String.format("Using custom transaction name -> %s", custName));
+      return custName;
     }
 
     String name = namingScheme.createName(spanAttributes);
     if (name != null && !name.isEmpty()) {
       logger.trace(String.format("Using scheme derived transaction name -> %s", name));
-      return new TransactionNameResult(name, false);
+      return name;
     }
 
     // use HandlerName which may be injected by some MVC instrumentations (currently only Spring
     // MVC)
     String handlerName = spanAttributes.get(AttributeKey.stringKey("HandlerName"));
     if (handlerName != null) {
       logger.trace(String.format("Using HandlerName(%s) as the transaction name", handlerName));
-      return new TransactionNameResult(handlerName, false);
+      return handlerName;
     }
 
     // use "http.route"
     String httpRoute = spanAttributes.get(SemanticAttributes.HTTP_ROUTE);
     if (httpRoute != null) {
       logger.trace(String.format("Using http.route (%s) as the transaction name", httpRoute));
-      return new TransactionNameResult(httpRoute, false);
+      return httpRoute;
     }
 
     // get transaction name from url
@@ -246,7 +232,7 @@ static TransactionNameResult buildTransactionName(SpanData spanData) {
             String.format(
                 "Using custom configure pattern to extract transaction name: (%s)",
                 transactionName));
-        return new TransactionNameResult(transactionName, false);
+        return transactionName;
       }
     }
 
@@ -261,12 +247,12 @@ static TransactionNameResult buildTransactionName(SpanData spanData) {
       logger.trace(
           String.format(
               "Using token name pattern to extract transaction name: (%s)", transactionNameByUrl));
-      return new TransactionNameResult(transactionNameByUrl, false);
+      return transactionNameByUrl;
     }
 
     String spanName = spanData.getName();
     logger.trace(String.format("Using span name as the transaction name: (%s)", spanName));
-    return new TransactionNameResult(spanName, false);
+    return spanName;
   }
 
   /**
@@ -379,16 +365,4 @@ static void reset() {
     URL_TRANSACTION_NAME_CACHE.invalidateAll();
     maxNameCount = DEFAULT_MAX_NAME_COUNT;
   }
-
-  @Value
-  @RequiredArgsConstructor
-  public static class TransactionNameResult {
-    String name;
-    boolean lambda;
-  }
-
-  @FunctionalInterface
-  public interface RuntimeNameGenerator {
-    TransactionNameResult generateName(SpanData spanData);
-  }
 }
diff --git a/smoke-tests/k6/basic.js b/smoke-tests/k6/basic.js
@@ -552,6 +552,8 @@ function silence(fn) {
 }
 
 export default function () {
+  silence(verify_that_span_data_is_persisted_0)
+
   if (`${__ENV.LAMBDA}` === "true") {
     const request_count = (measurement) => check(measurement, {"request_count": mrs => mrs.value > 0})
     const tracecount = (measurement) => check(measurement, {"tracecount": mrs => mrs.value > 0})
@@ -585,7 +587,6 @@ export default function () {
       })
     silence(verify_logs_export)
     silence(verify_that_specialty_path_is_not_sampled)
-    silence(verify_that_span_data_is_persisted_0)
 
     silence(verify_that_span_data_is_persisted)
     silence(verify_that_trace_is_persisted)

diff --git a/smoke-tests/src/test/java/com/solarwinds/LambdaTest.java b/smoke-tests/src/test/java/com/solarwinds/LambdaTest.java
@@ -16,32 +16,32 @@
 
 package com.solarwinds;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
-
 import com.solarwinds.agents.Agent;
-import com.solarwinds.agents.SwoAgentResolver;
 import com.solarwinds.agents.SwoLambdaAgentResolver;
 import com.solarwinds.config.Configs;
 import com.solarwinds.config.TestConfig;
 import com.solarwinds.containers.K6Container;
 import com.solarwinds.containers.PetClinicRestContainer;
 import com.solarwinds.containers.PostgresContainer;
+import com.solarwinds.containers.SpringBootWebMvcContainer;
 import com.solarwinds.results.ResultsCollector;
 import com.solarwinds.util.LogStreamAnalyzer;
 import com.solarwinds.util.NamingConventions;
-import java.io.IOException;
-import java.nio.file.Files;
-import java.util.List;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.containers.Network;
-import org.testcontainers.containers.output.OutputFrame;
 import org.testcontainers.containers.output.Slf4jLogConsumer;
 
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
 @EnabledIfEnvironmentVariable(named = "LAMBDA", matches = "true")
 public class LambdaTest {
   private static final Network NETWORK = Network.newNetwork();
@@ -64,34 +64,37 @@ static void runTestConfig() {
         .forEach(
             agent -> {
               try {
-                runAppOnce(config, agent);
+                runAppOnce(agent);
               } catch (Exception e) {
                 fail("Unhandled exception in " + config.name(), e);
               }
             });
   }
 
-  static void runAppOnce(TestConfig config, Agent agent) throws Exception {
+  static void runAppOnce(Agent agent) throws Exception {
     GenericContainer<?> postgres = new PostgresContainer(NETWORK).build();
     postgres.start();
 
-    GenericContainer<?> petClinic = new PetClinicRestContainer(new SwoLambdaAgentResolver(), NETWORK,
-        agent, namingConventions).build();
+    GenericContainer<?> webMvc = new SpringBootWebMvcContainer(new SwoLambdaAgentResolver(), NETWORK, agent).build();
+    webMvc.start();
+
+    GenericContainer<?> petClinic = new PetClinicRestContainer(new SwoLambdaAgentResolver(), NETWORK, agent).build();
     petClinic.start();
     petClinic.followOutput(logStreamAnalyzer);
 
-    GenericContainer<?> k6 = new K6Container(NETWORK, agent, config, namingConventions).build();
+    GenericContainer<?> k6 = new K6Container(NETWORK, agent, namingConventions).build();
     k6.start();
     k6.followOutput(new Slf4jLogConsumer(LoggerFactory.getLogger("k6")));
 
     petClinic.execInContainer("kill", "1");
+    webMvc.execInContainer("kill", "1");
     postgres.stop();
   }
 
   @Test
   void assertThatRequestCountMetricIsReported() throws IOException {
     String resultJson = new String(
-        Files.readAllBytes(namingConventions.local.k6Results(Configs.E2E.config.agents().get(0))));
+            Files.readAllBytes(namingConventions.local.k6Results(Configs.LAMBDA_E2E.config.agents().get(0))));
 
     double passes = ResultsCollector.read(resultJson, "$.root_group.checks.['request_count'].passes");
     assertTrue(passes > 1, "Expects a count > 1 ");
@@ -100,7 +103,7 @@ void assertThatRequestCountMetricIsReported() throws IOException {
   @Test
   void assertThatTraceCountMetricIsReported() throws IOException {
     String resultJson = new String(
-        Files.readAllBytes(namingConventions.local.k6Results(Configs.E2E.config.agents().get(0))));
+            Files.readAllBytes(namingConventions.local.k6Results(Configs.LAMBDA_E2E.config.agents().get(0))));
 
     double passes = ResultsCollector.read(resultJson, "$.root_group.checks.['tracecount'].passes");
     assertTrue(passes > 1, "Expects a count > 1 ");
@@ -109,7 +112,7 @@ void assertThatTraceCountMetricIsReported() throws IOException {
   @Test
   void assertThatSampleCountMetricIsReported() throws IOException {
     String resultJson = new String(
-        Files.readAllBytes(namingConventions.local.k6Results(Configs.E2E.config.agents().get(0))));
+            Files.readAllBytes(namingConventions.local.k6Results(Configs.LAMBDA_E2E.config.agents().get(0))));
 
     double passes = ResultsCollector.read(resultJson, "$.root_group.checks.['samplecount'].passes");
     assertTrue(passes > 1, "Expects a count > 1 ");
@@ -118,7 +121,7 @@ void assertThatSampleCountMetricIsReported() throws IOException {
   @Test
   void assertThatResponseTimeMetricIsReported() throws IOException {
     String resultJson = new String(
-        Files.readAllBytes(namingConventions.local.k6Results(Configs.E2E.config.agents().get(0))));
+            Files.readAllBytes(namingConventions.local.k6Results(Configs.LAMBDA_E2E.config.agents().get(0))));
 
     double passes = ResultsCollector.read(resultJson, "$.root_group.checks.['response_time'].passes");
     assertTrue(passes > 1, "Expects a count > 1 ");
@@ -127,10 +130,10 @@ void assertThatResponseTimeMetricIsReported() throws IOException {
   @Test
   void assertThatCustomTransactionNameTakesEffect() throws IOException {
     String resultJson = new String(
-        Files.readAllBytes(namingConventions.local.k6Results(Configs.E2E.config.agents().get(0))));
+            Files.readAllBytes(namingConventions.local.k6Results(Configs.LAMBDA_E2E.config.agents().get(0))));
 
     double passes = ResultsCollector.read(resultJson, "$.root_group.checks.['transaction-name'].passes");
-    assertTrue(passes > 1, "Expects a count > 1 ");
+    assertTrue(passes > 1, "Environment based transaction naming is broken ");
   }
 
   @Test
@@ -144,4 +147,11 @@ void assertThatJDBCInstrumentationIsApplied() {
     Boolean actual = logStreamAnalyzer.getAnswer().get("Applying instrumentation: sw-jdbc");
     assertTrue(actual, "sw-jdbc instrumentation is not applied");
   }
+
+  @Test
+  void assertSDKTransactionNaming() throws IOException {
+    String resultJson = new String(Files.readAllBytes(namingConventions.local.k6Results(Configs.LAMBDA_E2E.config.agents().get(0))));
+    double passes = ResultsCollector.read(resultJson, "$.root_group.checks.['custom transaction name'].passes");
+    assertTrue(passes > 1, "SDK transaction naming is broken");
+  }
 }
diff --git a/smoke-tests/src/test/java/com/solarwinds/SmokeTest.java b/smoke-tests/src/test/java/com/solarwinds/SmokeTest.java
@@ -72,14 +72,14 @@ static void runTestConfig() {
                 .forEach(
                         agent -> {
                             try {
-                                runAppOnce(config, agent);
+                                runAppOnce(agent);
                             } catch (Exception e) {
                                 fail("Unhandled exception in " + config.name(), e);
                             }
                         });
     }
 
-    static void runAppOnce(TestConfig config, Agent agent) throws Exception {
+    static void runAppOnce(Agent agent) throws Exception {
         GenericContainer<?> webMvc = new SpringBootWebMvcContainer(new SwoAgentResolver(), NETWORK, agent).build();
         webMvc.start();
         webMvc.followOutput(logStreamAnalyzer);
@@ -91,11 +91,11 @@ static void runAppOnce(TestConfig config, Agent agent) throws Exception {
         GenericContainer<?> postgres = new PostgresContainer(NETWORK).build();
         postgres.start();
 
-        GenericContainer<?> petClinic = new PetClinicRestContainer(new SwoAgentResolver(), NETWORK, agent, namingConventions).build();
+        GenericContainer<?> petClinic = new PetClinicRestContainer(new SwoAgentResolver(), NETWORK, agent).build();
         petClinic.start();
         petClinic.followOutput(logStreamAnalyzer);
 
-        GenericContainer<?> k6 = new K6Container(NETWORK, agent, config, namingConventions).build();
+        GenericContainer<?> k6 = new K6Container(NETWORK, agent, namingConventions).build();
         k6.start();
         k6.followOutput(new Slf4jLogConsumer(LoggerFactory.getLogger("k6")), OutputFrame.OutputType.STDOUT);