Library instrumentation for Java Servlet 3.0 Filters.

[tylerbenson]

Includes handling for attaching all the async stuff, but can't support everything the java agent does.

I could use help figuring out the right way to shadow all the necessary dependencies. I'm a bit rusty with our shadow usage. Depending on how aggressive we want to be, the dependency could be reversed so that :servlet-3.0:javaagent depends on this new module, but that's more aggressive. Perhaps shadow could prune unused classes (with minimize()) instead.

Once we get the modules figured out, I can work on adding some unit tests.

If we like this approach, it could probably be copied and modified pretty easily to support servlet 5, but I'm not familiar with that instrumentation.

[laurit]

I could use help figuring out the right way to shadow all the necessary dependencies.

You shouldn't shadow anything. If you need to share code you can extract common code between the javaagent and library instrumentation into separate module or make javaagent module depend on the library module if that make sense. In your place I would focus on building a functioning instrumentation along with tests and explore options for sharing code later. It is quite possible that the agent code does things that don't make sense for the library instrumentation which could limit the amount of code that can be shared between them.

[tylerbenson]

It seems like an oversight that this isn't already here.

[tylerbenson]

This change isn't necessary, but it seems odd that these are duplicated. I combined them, but if it's intentional I can revert this change.

[tylerbenson]

I tried to keep the instrumentation as similar as possible to the javaagent instrumentation.

[otelbot-java-instrumentation]

🔧 The result from spotlessApply was committed to the PR branch.

[jaydeluca]

i'm wondering about the value / confusion tradeoff with having "copied" in the package name. I understand the motivation, but wondering if it will be obvious to someone stumbling across it. I don't have any better ideas at this time though

[tylerbenson]

This is why I originally suggested using shadow/shade to generate the jar and avoid copying all this over. I personally like the distinction between what is copied over vs specific to this library. Open to better ideas.

[jaydeluca]

I think we could just add a field to this class instead of using a virtual field? or maybe this falls under "I tried to keep the instrumentation as similar as possible to the javaagent instrumentation."

just thinking we could add

  private MappingResolver.Factory mappingResolverFactory;

  @Override
  public void init(FilterConfig filterConfig) {
    this.mappingResolverFactory = new Servlet3FilterMappingResolverFactory(filterConfig);
  }

and then pass it to the advice scope:

    Servlet3RequestAdviceScope adviceScope =
        new Servlet3RequestAdviceScope(
            CallDepth.forClass(OpenTelemetryServletFilter.class),
            httpRequest,
            httpResponse,
            this,
            mappingResolverFactory);

and in Servlet3RequestAdviceScope something like:

    MappingResolver mappingResolver =
        mappingResolverFactory != null ? mappingResolverFactory.get() : null;

[tylerbenson]

yeah, there a lot of things about this that I would do very differently if I were writing it from scratch. Trying to keep it as similar as possible to make it easier to maintain long term.