Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New agent instrumentation for Apache Sling #9469

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions instrumentation/sling/javaagent/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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
}
Original file line number Diff line number Diff line change
@@ -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<TypeDescription> 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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could see this as potentially surprising if it shows up as the "name", but I think it's fine. Hopefully more informative than just the class name when the name is missing from the config.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could've sworn that I found this pattern in the repository, but I can't find it anymore.

}
if (name == null || name.isEmpty()) {
name = servlet.getClass().getName();
}

request.setAttribute(REQUEST_ATTR_RESOLVED_SERVLET_NAME, name);
}
}
}
Original file line number Diff line number Diff line change
@@ -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<TypeInstrumentation> typeInstrumentations() {
return Arrays.asList(
new ServletResolverInstrumentation(), new SlingSafeMethodsServletInstrumentation());
}
}
Original file line number Diff line number Diff line change
@@ -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<TypeDescription> typeMatcher() {
return implementsInterface(named("javax.servlet.Servlet"));
}

@Override
public ElementMatcher<ClassLoader> 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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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<SlingHttpServletRequest> SPAN_NAME_EXTRACTOR =
s -> (String) s.getAttribute(REQUEST_ATTR_RESOLVED_SERVLET_NAME);
private static final Instrumenter<SlingHttpServletRequest, Void> INSTRUMENTER =
Instrumenter.<SlingHttpServletRequest, Void>builder(
GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, SPAN_NAME_EXTRACTOR)
.buildInstrumenter();

public static Instrumenter<SlingHttpServletRequest, Void> helper() {
return INSTRUMENTER;
}

private SlingSingletons() {}
}
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,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")
Expand Down
Loading