Skip to content

Commit

Permalink
Servlet 5 API, reorganize servlet modules (#2609)
Browse files Browse the repository at this point in the history
* Servlet 5.0 API support, refactored other servlet modules

* Include correct servlet instrumentation module for grails tests

* Reapply spotless after rebase

* Fix checkstyle

* Spotless

* Addressed PR suggestions

* Fixed files in wrong package.

* Fixed mixed up instrumentation names.

* Fixed incomplete instrumentation name.

* Addressed PR suggestions

* Addressed PR suggestions
  • Loading branch information
agoallikmaa authored Mar 23, 2021
1 parent 56c52bc commit 69c2644
Show file tree
Hide file tree
Showing 107 changed files with 2,277 additions and 738 deletions.
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
2 changes: 1 addition & 1 deletion instrumentation/jsp-2.3/javaagent/jsp-2.3-javaagent.gradle
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"));
}

@Override
Expand Down
Loading

0 comments on commit 69c2644

Please sign in to comment.