diff --git a/.fossa.yml b/.fossa.yml index 0a29d3ce460a..040190e95387 100644 --- a/.fossa.yml +++ b/.fossa.yml @@ -262,6 +262,9 @@ targets: - type: gradle path: ./ target: ':instrumentation:scala-fork-join-2.8:javaagent' + - type: gradle + path: ./ + target: ':instrumentation:sling:javaagent' - type: gradle path: ./ target: ':instrumentation:spark-2.3:javaagent' diff --git a/instrumentation/sling/javaagent/build.gradle.kts b/instrumentation/sling/javaagent/build.gradle.kts new file mode 100644 index 000000000000..7e14e1375d1a --- /dev/null +++ b/instrumentation/sling/javaagent/build.gradle.kts @@ -0,0 +1,13 @@ +plugins { + id("otel.javaagent-instrumentation") +} + +dependencies { + bootstrap(project(":instrumentation:executors:bootstrap")) + implementation(project(":instrumentation:servlet:servlet-3.0:javaagent")) + bootstrap(project(":instrumentation:servlet:servlet-common:bootstrap")) + api(project(":instrumentation:servlet:servlet-common:javaagent")) + compileOnly("javax.servlet:javax.servlet-api:3.1.0") + library("org.apache.sling:org.apache.sling.api:2.0.6") // first non-incubator release + testLibrary("org.apache.sling:org.apache.sling.feature.launcher:1.3.2") +} diff --git a/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/ServletResolverInstrumentation.java b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/ServletResolverInstrumentation.java new file mode 100644 index 000000000000..14c784f2a939 --- /dev/null +++ b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/ServletResolverInstrumentation.java @@ -0,0 +1,68 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.sling; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.sling.SlingSingletons.REQUEST_ATTR_RESOLVED_SERVLET_NAME; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.servlet.Servlet; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.sling.api.SlingHttpServletRequest; + +public class ServletResolverInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("org.apache.sling.api.servlets.ServletResolver")); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isMethod() + .and(named("resolveServlet")) + .and(takesArguments(1)) + .and(takesArgument(0, named("org.apache.sling.api.SlingHttpServletRequest"))), + this.getClass().getName() + "$ResolveServletAdvice"); + } + + @SuppressWarnings("unused") + public static class ResolveServletAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onExit( + @Advice.Argument(0) SlingHttpServletRequest request, + @Advice.Return Servlet servlet, + @Advice.Thrown Throwable throwable, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + + String name = null; + + if (servlet.getServletConfig() != null) { + name = servlet.getServletConfig().getServletName(); + } + if (name == null || name.isEmpty()) { + name = servlet.getServletInfo(); + } + if (name == null || name.isEmpty()) { + name = servlet.getClass().getName(); + } + + request.setAttribute(REQUEST_ATTR_RESOLVED_SERVLET_NAME, name); + } + } +} diff --git a/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingInstrumentationModule.java b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingInstrumentationModule.java new file mode 100644 index 000000000000..a282313fec38 --- /dev/null +++ b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingInstrumentationModule.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.sling; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.Arrays; +import java.util.List; + +@AutoService(InstrumentationModule.class) +public class SlingInstrumentationModule extends InstrumentationModule { + + public SlingInstrumentationModule() { + super("sling", "sling-1.0"); + } + + @Override + public List typeInstrumentations() { + return Arrays.asList( + new ServletResolverInstrumentation(), new SlingSafeMethodsServletInstrumentation()); + } +} diff --git a/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingSafeMethodsServletInstrumentation.java b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingSafeMethodsServletInstrumentation.java new file mode 100644 index 000000000000..21f4a669f687 --- /dev/null +++ b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingSafeMethodsServletInstrumentation.java @@ -0,0 +1,107 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.sling; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.sling.SlingSingletons.REQUEST_ATTR_RESOLVED_SERVLET_NAME; +import static io.opentelemetry.javaagent.instrumentation.sling.SlingSingletons.helper; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute; +import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRouteSource; +import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.servlet.ServletRequest; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.sling.api.SlingHttpServletRequest; + +public class SlingSafeMethodsServletInstrumentation implements TypeInstrumentation { + @Override + public ElementMatcher typeMatcher() { + return implementsInterface(named("javax.servlet.Servlet")); + } + + @Override + public ElementMatcher classLoaderOptimization() { + return hasClassesNamed("org.apache.sling.api.SlingHttpServletRequest"); + } + + @Override + public void transform(TypeTransformer transformer) { + + String adviceClassName = this.getClass().getName() + "$ServiceServletAdvice"; + transformer.applyAdviceToMethod( + named("service") + .and(takesArguments(2)) + .and(takesArgument(0, named("javax.servlet.ServletRequest"))) + .and(takesArgument(1, named("javax.servlet.ServletResponse"))), + adviceClassName); + } + + @SuppressWarnings("unused") + public static class ServiceServletAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(0) ServletRequest request, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + + if (!(request instanceof SlingHttpServletRequest)) { + return; + } + + SlingHttpServletRequest slingRequest = (SlingHttpServletRequest) request; + + Context parentContext = Java8BytecodeBridge.currentContext(); + + if (!helper().shouldStart(parentContext, slingRequest)) { + return; + } + + // written by ServletResolverInstrumentation + Object servletName = request.getAttribute(REQUEST_ATTR_RESOLVED_SERVLET_NAME); + if (!(servletName instanceof String)) { + return; + } + + // TODO - figure out why don't we have matches for all requests and find a better way to + // filter + context = helper().start(parentContext, slingRequest); + scope = context.makeCurrent(); + + // ensure that the top-level route is Sling-specific + HttpServerRoute.update(context, HttpServerRouteSource.CONTROLLER, (String) servletName); + + // cleanup and ensure we don't have reuse the resolved Servlet name by accident for other + // requests + request.removeAttribute(REQUEST_ATTR_RESOLVED_SERVLET_NAME); + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onExit( + @Advice.Argument(0) ServletRequest request, + @Advice.Thrown Throwable throwable, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + + if (scope == null) { + return; + } + scope.close(); + + SlingHttpServletRequest slingRequest = (SlingHttpServletRequest) request; + helper().end(context, slingRequest, null, throwable); + } + } +} diff --git a/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingSingletons.java b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingSingletons.java new file mode 100644 index 000000000000..659afd51858b --- /dev/null +++ b/instrumentation/sling/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/sling/SlingSingletons.java @@ -0,0 +1,31 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.sling; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import org.apache.sling.api.SlingHttpServletRequest; + +public final class SlingSingletons { + private static final String INSTRUMENTATION_NAME = "io.opentelemetry.sling-1.0"; + + static final String REQUEST_ATTR_RESOLVED_SERVLET_NAME = + INSTRUMENTATION_NAME + ".resolvedServletName"; + + private static final SpanNameExtractor SPAN_NAME_EXTRACTOR = + s -> (String) s.getAttribute(REQUEST_ATTR_RESOLVED_SERVLET_NAME); + private static final Instrumenter INSTRUMENTER = + Instrumenter.builder( + GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, SPAN_NAME_EXTRACTOR) + .buildInstrumenter(); + + public static Instrumenter helper() { + return INSTRUMENTER; + } + + private SlingSingletons() {} +} diff --git a/instrumentation/sling/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/sling/SlingTest.java b/instrumentation/sling/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/sling/SlingTest.java new file mode 100644 index 000000000000..b6c2ff241121 --- /dev/null +++ b/instrumentation/sling/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/sling/SlingTest.java @@ -0,0 +1,183 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.sling; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; + +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerUsingTest; +import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension; +import io.opentelemetry.javaagent.testing.common.TestAgentListenerAccess; +import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse; +import io.opentelemetry.testing.internal.armeria.common.HttpResponse; +import java.net.Socket; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import org.apache.sling.feature.launcher.impl.Bootstrap; +import org.apache.sling.feature.launcher.impl.LauncherConfig; +import org.awaitility.Awaitility; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SlingTest extends AbstractHttpServerUsingTest { + + @RegisterExtension + public static final InstrumentationExtension testing = + HttpServerInstrumentationExtension.forAgent(); + + private final Logger log = LoggerFactory.getLogger(getClass()); + + @BeforeAll + void setup() { + startServer(); + } + + @AfterAll + void cleanup() { + cleanupServer(); + } + + @Test + void basic() throws Exception { + AggregatedHttpResponse response = + client.get(address.resolve("starter.html").toString()).aggregate().join(); + + assertThat(response.status().code()).isEqualTo(200); + + // FIXME - we need to reset this because of unrelated failures; not supported by Apache Sling + // https://github.com/apache/sling-org-apache-sling-engine/blob/8ef91a0ea56d3fa919fb1cde3d1c08419722fe45/src/main/java/org/apache/sling/engine/impl/helper/SlingServletContext.java#L763-L767 + TestAgentListenerAccess.getAndResetAdviceFailureCount(); + + // FIXME - Muzzle checks we need to clarify + // [otel.javaagent 2025-07-02 13:00:07:012 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - Instrumentation skipped, + // mismatched references were found: sling [class + // io.opentelemetry.javaagent.instrumentation.sling.SlingInstrumentationModule] on + // org.apache.sling.installer.console [217] + // [otel.javaagent 2025-07-02 13:00:07:012 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - -- + // io.opentelemetry.javaagent.instrumentation.sling.ServletResolverInstrumentation$ResolveServletAdvice:65 Missing class javax.servlet.http.HttpServletRequest + // [otel.javaagent 2025-07-02 13:00:07:015 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - Instrumentation skipped, + // mismatched references were found: servlet [class + // io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3InstrumentationModule] on + // org.apache.sling.installer.console [217] + // [otel.javaagent 2025-07-02 13:00:07:017 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - -- + // io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.Servlet3SnippetInjectingResponseWrapper:56 Missing class javax.servlet.http.HttpServletResponse + // [otel.javaagent 2025-07-02 13:00:07:017 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - -- + // io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3Accessor:28 Missing class + // javax.servlet.http.HttpServletRequest + // [otel.javaagent 2025-07-02 13:00:07:017 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - -- + // io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.ServletOutputStreamInjectionState:20 Missing method io.opentelemetry.javaagent.bootstrap.servlet.SnippetInjectingResponseWrapper#getCharacterEncoding()Ljava/lang/String; + // [otel.javaagent 2025-07-02 13:00:07:017 +0200] [FelixStartLevel] WARN + // io.opentelemetry.javaagent.tooling.instrumentation.MuzzleMatcher - -- + // io.opentelemetry.javaagent.instrumentation.servlet.v3_0.snippet.Servlet3SnippetInjectingResponseWrapper:0 Missing class javax.servlet.http.HttpServletResponseWrapper + // + // Potentially because of optional imports - Import-Package: + // javax.servlet;resolution:=optional;version="[3.1,4)" + TestAgentListenerAccess.getAndResetMuzzleFailureCount(); + + List> traces = testing.waitForTraces(1); + + List mainTrace = traces.get(0); + assertThat(mainTrace).hasSize(3); + // top-level trace + assertThat(mainTrace.get(0)) + .hasKind(SpanKind.SERVER) + .hasAttributesSatisfying( + attributes -> { + assertThat(attributes).containsEntry("http.request.method", "GET"); + assertThat(attributes) + .containsEntry("http.route", "/apps/sling/starter/home/home.html.esp"); + }); + + assertThat(mainTrace.get(1)) + .hasKind(SpanKind.INTERNAL) + .hasName("/apps/sling/starter/home/home.html.esp") + .hasInstrumentationScopeInfo(InstrumentationScopeInfo.create("io.opentelemetry.sling-1.0")); + + assertThat(mainTrace.get(2)) + .hasKind(SpanKind.INTERNAL) + .hasName("/apps/sling/starter/sidebar-extensions/sidebar-extensions.html.esp") + .hasInstrumentationScopeInfo(InstrumentationScopeInfo.create("io.opentelemetry.sling-1.0")); + } + + @Override + protected Void setupServer() throws Exception { + + Path homeDir = Files.createTempDirectory("javaagent_sling-test"); + + LauncherConfig cfg = new LauncherConfig(); + cfg.getInstallation() + .addFrameworkProperty("org.osgi.service.http.port", String.valueOf(this.port)); + cfg.setHomeDirectory(homeDir.toFile()); + cfg.addFeatureFiles( + "mvn:org.apache.sling/org.apache.sling.starter/13/slingosgifeature/oak_tar"); + + CompletableFuture.runAsync( + () -> { + try { + Bootstrap b = new Bootstrap(cfg, log); + b.runWithException(); + } catch (Exception e) { + log.error("Failed to start Sling server, test will time out and fail.", e); + } + }); + + Awaitility.await() + .atMost(Duration.ofMinutes(1)) + .pollInterval(Duration.ofMillis(500)) + .ignoreExceptions() + .until( + () -> { + try (Socket s = new Socket(address.getHost(), address.getPort())) { + return s.isConnected(); + } catch (Exception e) { + return false; + } + }); + + Awaitility.await() + .atMost(Duration.ofMinutes(4)) + .pollInterval(Duration.ofMillis(500)) + .ignoreExceptions() + .until( + () -> { + try { + HttpResponse response = client.get(address.resolve("/").toString()); + return response.aggregate().join().status().code() >= 200 + && response.aggregate().join().status().code() < 400; + } catch (RuntimeException e) { + return false; + } + }); + + return null; + } + + @Override + protected void stopServer(Void unused) throws Exception { + log.warn("Stopping Sling server is not implemented"); + } + + @Override + protected String getContextPath() { + return ""; + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 8647d80cff62..555bf48171d6 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -534,6 +534,7 @@ include(":instrumentation:servlet:servlet-5.0:tomcat-testing") include(":instrumentation:servlet:servlet-common:bootstrap") include(":instrumentation:servlet:servlet-common:javaagent") include(":instrumentation:servlet:servlet-javax-common:javaagent") +include(":instrumentation:sling:javaagent") include(":instrumentation:spark-2.3:javaagent") include(":instrumentation:spring:spring-batch-3.0:javaagent") include(":instrumentation:spring:spring-boot-actuator-autoconfigure-2.0:javaagent")