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

Servlet 5 API, reorganize servlet modules #2609

Merged
merged 11 commits into from
Mar 23, 2021
17 changes: 0 additions & 17 deletions instrumentation-core/README.md

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ dependencies {
library("org.grails:grails-plugin-url-mappings:$grailsVersion")

testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')
testInstrumentation project(':instrumentation:tomcat-7.0:javaagent')
testInstrumentation project(':instrumentation:spring:spring-webmvc-3.1:javaagent')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ dependencies {
implementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-common:javaagent')

testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

testImplementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-testing')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ dependencies {
implementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-common:javaagent')

testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

testImplementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-testing')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ dependencies {
implementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-common:javaagent')

testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

testImplementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-testing')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ muzzle {
dependencies {
library group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.0.0.v20110901'
implementation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

// Don't want to conflict with jetty from the test server.
testImplementation(project(':testing-common')) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.servlet.AppServerBridge;
import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
import io.opentelemetry.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
import javax.servlet.http.HttpServletRequest;

public class JettyHttpServerTracer extends Servlet3HttpServerTracer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
* has its own {@code JettyHandlerInstrumentation} and {@code JettyHandlerAdvice}. But this is the
* only difference between two instrumentations, thus {@link
* io.opentelemetry.javaagent.instrumentation.jetty.JettyHttpServerTracer} is a very thin subclass
* of {@link io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3HttpServerTracer}.
* of {@link io.opentelemetry.instrumentation.servlet.v3_0.Servlet3HttpServerTracer}.
*/
package io.opentelemetry.javaagent.instrumentation.jetty;
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ dependencies {

testImplementation project(':instrumentation:jsf:jsf-testing-common')
testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

mojarra12TestImplementation group: 'javax.faces', name: 'jsf-impl', version: '1.2-20'
mojarra12TestImplementation group: 'javax.faces', name: 'jsf-api', version: '1.2'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ dependencies {

testImplementation project(':instrumentation:jsf:jsf-testing-common')
testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

myfaces12TestImplementation group: 'org.apache.myfaces.core', name: 'myfaces-impl', version: '1.2.12'
myfaces12TestImplementation group: 'com.sun.facelets', name: 'jsf-facelets', version: '1.1.14'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ dependencies {
compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.1.0'

testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

// using tomcat 7.0.37 because there seems to be some issues with Tomcat's jar scanning in versions < 7.0.37
// https://stackoverflow.com/questions/23484098/org-apache-tomcat-util-bcel-classfile-classformatexception-invalid-byte-tag-in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package io.opentelemetry.javaagent.instrumentation.liberty;

import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
import io.opentelemetry.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
import javax.servlet.http.HttpServletRequest;

public class LibertyHttpServerTracer extends Servlet3HttpServerTracer {
Expand Down
25 changes: 17 additions & 8 deletions instrumentation/servlet/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,24 @@

## A word about version

We support Servlet API starting from version 2.2.
We support Servlet API starting from version 2.2.
But various instrumentations apply to different versions of the API.
They are divided into 3 sub-modules:

`servlet-common` contains instrumentations applicable to all API versions that we support.

`servlet-2.2` contains instrumentations applicable to Servlet API 2.2, but not to 3+.

`servlet-3.0` contains instrumentations that require Servlet API 3.0 or newer.
They are divided into the following sub-modules:
- `servlet-common` contains shared code for both `javax.servlet` and `jakarta.servlet` packages
- `library` contains the abstract tracer applicable to all servlet versions given an
implementation of `ServletAccessor` to access request and response objects of the specific
version
- `javaagent` contains shared type instrumentations which can be used by version specific modules
by specifying the base package and advice class to use with them. Contains some helper classes
used by advices to reduce code duplication. It does not define any instrumentation modules and
is used only as a dependency for other `javaagent` modules.
- Version-specific modules where `library` contains the version-specific tracer and request/response
accessor, and `javaagent` contains the instrumentation modules and advices.
- `servlet-javax-common` contains instrumentations/abstract tracer common for Servlet API versions `[2.2, 5)`
- `servlet-2.2` contains instrumentations/tracer for Servlet API versions `[2.2, 3)`
- `servlet-3.0` contains instrumentations/tracer for Servlet API versions `[3.0, 5)`
- `servlet-5.0` contains instrumentations/tracer for Servlet API versions `[5,)`

## Implementation details

Expand Down Expand Up @@ -49,7 +58,7 @@ This is the main target for `Servlet3Instrumentation` and `Servlet2Instrumentati

`public void javax.servlet.http.HttpServlet#service(ServletRequest, ServletResponse)`.

These instrumentations are located in two separate submodules `servlet-3.0` and `servlet-2.2`,
These instrumentations are located in separate submodules `servlet-3.0`, `servlet-2.2` and `servlet-5.0`,
because they and corresponding tests depend on different versions of the servlet specification.

At last, request processing may reach the specific framework that your application uses.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ apply from: "$rootDir/gradle/instrumentation.gradle"

dependencies {
testInstrumentation project(':instrumentation:servlet:servlet-3.0:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')
testInstrumentation project(':instrumentation:grizzly-2.0:javaagent')

testLibrary group: 'org.glassfish.main.extras', name: 'glassfish-embedded-all', version: '4.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ muzzle {

dependencies {
compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2'
api(project(':instrumentation-core:servlet-2.2'))
api(project(':instrumentation:servlet:servlet-2.2:library'))
implementation(project(':instrumentation:servlet:servlet-common:javaagent'))

testInstrumentation project(':instrumentation:servlet:servlet-common:javaagent')
testInstrumentation project(':instrumentation:servlet:servlet-javax-common:javaagent')

testImplementation(project(':testing-common')) {
exclude group: 'org.eclipse.jetty', module: 'jetty-server'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@

package io.opentelemetry.javaagent.instrumentation.servlet.v2_2;

import static io.opentelemetry.javaagent.instrumentation.servlet.v2_2.Servlet2HttpServerTracer.tracer;
import static io.opentelemetry.instrumentation.servlet.v2_2.Servlet2HttpServerTracer.tracer;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.servlet.AppServerBridge;
import io.opentelemetry.instrumentation.servlet.v2_2.ResponseWithStatus;
import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static net.bytebuddy.matcher.ElementMatchers.not;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation;
import io.opentelemetry.javaagent.tooling.InstrumentationModule;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.util.Arrays;
Expand All @@ -32,7 +33,10 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
@Override
public List<TypeInstrumentation> typeInstrumentations() {
return Arrays.asList(
new HttpServletResponseInstrumentation(), new ServletAndFilterInstrumentation());
new HttpServletResponseInstrumentation(),
new ServletAndFilterInstrumentation(
"javax.servlet",
Servlet2InstrumentationModule.class.getPackage().getName() + ".Servlet2Advice"));
trask marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
Loading