NH-91749: use the new extended span processor to leverage mutating sp…

…an before it's ended

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}"

custom/lambda/src/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;
+  }
 }

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");

custom/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);
-  }
 }

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)

smoke-tests/src/test/java/com/solarwinds/LambdaTest.java

@@ -27,6 +27,7 @@
 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;
@@ -75,6 +76,9 @@ static void runAppOnce(TestConfig config, Agent agent) throws Exception {
     GenericContainer<?> postgres = new PostgresContainer(NETWORK).build();
     postgres.start();
 
+    GenericContainer<?> webMvc = new SpringBootWebMvcContainer(new SwoLambdaAgentResolver(), NETWORK, agent).build();
+    webMvc.start();
+
     GenericContainer<?> petClinic = new PetClinicRestContainer(new SwoLambdaAgentResolver(), NETWORK,
         agent, namingConventions).build();
     petClinic.start();
@@ -85,6 +89,7 @@ static void runAppOnce(TestConfig config, Agent agent) throws Exception {
     k6.followOutput(new Slf4jLogConsumer(LoggerFactory.getLogger("k6")));
 
     petClinic.execInContainer("kill", "1");
+    webMvc.execInContainer("kill", "1");
     postgres.stop();
   }
 
@@ -130,7 +135,7 @@ void assertThatCustomTransactionNameTakesEffect() throws IOException {
         Files.readAllBytes(namingConventions.local.k6Results(Configs.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 +149,11 @@ void assertThatJDBCInstrumentationIsApplied() {
     Boolean actual = logStreamAnalyzer.getAnswer().get("Applying instrumentation: sw-jdbc");
     assertTrue(actual, "sw-jdbc instrumentation is not applied");
   }
+
+  @Test
+  void assertTransactionNaming() throws IOException {
+    String resultJson = new String(Files.readAllBytes(namingConventions.local.k6Results(Configs.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");
+  }
 }

smoke-tests/src/test/java/com/solarwinds/containers/SpringBootWebMvcContainer.java

@@ -18,7 +18,6 @@
 
 import com.solarwinds.agents.Agent;
 import com.solarwinds.agents.AgentResolver;
-import com.solarwinds.util.NamingConventions;
 import org.jetbrains.annotations.NotNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -33,6 +32,7 @@
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
 public class SpringBootWebMvcContainer implements Container {
 
@@ -51,6 +51,27 @@ public SpringBootWebMvcContainer(AgentResolver agentResolver, Network network, A
 
   public GenericContainer<?> build() {
     Path agentPath = agentResolver.resolve(this.agent).orElseThrow();
+    if (Objects.equals(System.getenv("LAMBDA"), "true")) {
+      return new GenericContainer<>(DockerImageName.parse("smt:webmvc"))
+              .withNetwork(network)
+              .withNetworkAliases("webmvc")
+              .withLogConsumer(new Slf4jLogConsumer(logger))
+              .withExposedPorts(SERVER_PORT)
+              .waitingFor(Wait.forHttp("/actuator/health").withReadTimeout(Duration.ofMinutes(5)).forPort(SERVER_PORT))
+              .withEnv("SERVER_PORT", String.format("%d", SERVER_PORT))
+              .withEnv("SW_APM_CONFIG_FILE", "/app/apm-config.json")
+              .withEnv("SW_APM_DEBUG_LEVEL", "trace")
+              .withEnv("OTEL_EXPORTER_OTLP_ENDPOINT", System.getenv("OTEL_EXPORTER_OTLP_ENDPOINT"))
+              .withEnv("OTEL_EXPORTER_OTLP_HEADERS",
+                      String.format("authorization=Bearer %s", System.getenv("SW_APM_SERVICE_KEY")))
+              .withEnv("OTEL_SERVICE_NAME", "java-apm-smoke-test-webmvc-lambda")
+              .withStartupTimeout(Duration.ofMinutes(5))
+              .withCopyFileToContainer(
+                      MountableFile.forHostPath(agentPath),
+                      "/app/" + agentPath.getFileName())
+              .withCommand(buildCommandline(agentPath));
+    }
+
     return new GenericContainer<>(DockerImageName.parse("smt:webmvc"))
             .withNetwork(network)
             .withNetworkAliases("webmvc")

solarwinds-otel-sdk/build.gradle

@@ -36,8 +36,6 @@ dependencies {
   compileOnly("io.opentelemetry:opentelemetry-context:${versions.opentelemetry}")
   compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-bootstrap:${versions.opentelemetryJavaagentAlpha}")
 
-  compileOnly("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api:${versions.opentelemetryJavaagent}")
-
   testImplementation 'org.mockito:mockito-core:5.3.0'
   testImplementation 'org.mockito:mockito-junit-jupiter:5.3.0'
 

solarwinds-otel-sdk/src/main/java/com/solarwinds/api/ext/Transaction.java

@@ -20,7 +20,6 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.context.Context;
-import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
 import java.util.regex.Pattern;
 
 /**
@@ -48,23 +47,7 @@ static boolean setName(String name) {
       return false;
     }
 
-    CustomTransactionNameDict.set(spanContext.getTraceId(), transformTransactionName(name));
-    LocalRootSpan.fromContext(Context.current())
-        .setAttribute("sw.transaction", transformTransactionName(name));
-
+    CustomTransactionNameDict.set(spanContext.getTraceId(), name);
     return true;
   }
-
-  private static String transformTransactionName(String transactionName) {
-
-    if (transactionName.length() > 255) {
-      transactionName = transactionName.substring(0, 252) + "...";
-    } else if (transactionName.isEmpty()) {
-      transactionName = " "; // ensure that it at least has 1 character
-    }
-
-    transactionName = REPLACE_PATTERN.matcher(transactionName).replaceAll("_");
-    transactionName = transactionName.toLowerCase();
-    return transactionName;
-  }
 }