Instrument javax.servlet.Filter the same way as javax.servlet.FilterChain (#1857)

[tobiasstadler]

What does this PR do?

Fixes #1857

Checklist

• This is an enhancement of existing features, or a new feature in existing plugins
  • I have updated CHANGELOG.asciidoc
  • I have added tests that prove my fix is effective or that my feature works
  • Added an API method or config option? Document in which version this will be introduced
  • I have made corresponding changes to the documentation

[ghost]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: Branch indexing

• Start Time: 2021-07-01T10:39:17.133+0000

• Duration: 54 min 8 sec

• Commit: 5e67104

Test stats 🧪

Test	Results
Failed	0
Passed	2286
Skipped	19
Total	2305

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	2286
Skipped	19
Total	2305

[tobiasstadler]

Can somebody please do a review?

[tobiasstadler]

@SylvainJuge I merged the master branch (which disabled the auto-merge), would you mind merging it?

[eyalkoren]

@SylvainJuge this may considerably increase the number of times ServletApiAdvice can be invoked. Do you think we should add a quick abort when not required? For example - if there is an active transaction when ServletApiAdvice#onEnterServletService() starts - abort early; also make sure ServletApiAdvice#onExitServletService() aborts early when not required (e.g. when onEnterServletService aborted).

[tobiasstadler]

@eyalkoren Something like

ElasticApmTracer tracer = GlobalTracer.getTracerImpl();
if(tracer == null || tracer.currentTransaction() != null) {
    return null;
}

[SylvainJuge]

I did run a few wall-clock benchmarks with & without this change and did not found significant impact.

I did test with Tomcat though, which means that the ApplicationFilterChain is a parent of any call to Filter.doFilter configured within the application, hence making the Advice exit early as the transaction scope is defined by ApplicationFilterChain which extends FilterChain (but that might be an implementation detail only for Tomcat`.

Alternatively, for the specific case of Spark, the usage of Servlet API is quite non-standard, and maybe adding a proper support for Jetty handler to handle the transaction would also work (albeit it's definitely a bit more work).

[tobiasstadler]

@SylvainJuge It is the same for Undertow (But ApplicationFilterChain is called FilterChainImpl)

[tobiasstadler]

So how should we proceed?

[tobiasstadler]

Thank You!