Avoid setting request attributes to reduce allocations

[felixbarny]

Adding a request attribute creates a new HashMap$Node which allocates 32b

• Only set transaction request attribute when startAsync is called
• Set excluded as ThreadLocal as opposed to request attribute

[eyalkoren]

I think for the sake of accurate state management, this should be done only when transaction != null.

[felixbarny]

The transaction is null for excluded requests which is the exact case where we do want to set excluded back to the initial value. We could set it to false only if it's true or if transaction is null but I think it's simpler and not necessarily slower to set it always to false, just to be sure we don't miss anything.

[eyalkoren]

In the case of nested service calls, you set the excluded to true in the outmost invocation, so you should reset it in the exit of the same method. I understand why transaction != null is not good though as you say.
So what I suggest is: if you DO care about the state being set and reset accurately in the case of nested calls, you can add a local boolean variable that says whether this specific service method was the one changing the state or not. However, this is not super important I assume, so I am fine with whatever you decide.

[felixbarny]

In the case of nested service calls, you set the excluded to true in the outmost invocation, so you should reset it in the exit of the same method.

I think it's just fine to set it to false in all nested filters and servlets. If it has already been reset to false, resetting it again does not change the state.

[codecov-io]

Codecov Report

Merging #316 into master will decrease coverage by 0.29%.
The diff coverage is 40%.

@@             Coverage Diff             @@
##             master     #316     +/-   ##
===========================================
- Coverage     72.44%   72.14%   -0.3%     
+ Complexity     1195     1190      -5     
===========================================
  Files           131      131             
  Lines          4478     4484      +6     
  Branches        456      459      +3     
===========================================
- Hits           3244     3235      -9     
- Misses         1025     1038     +13     
- Partials        209      211      +2

Impacted Files	Coverage Δ	Complexity Δ
...lastic/apm/servlet/FilterChainInstrumentation.java	100% <ø> (ø)	7 <0> (ø)	⬇️
...a/co/elastic/apm/servlet/AsyncInstrumentation.java	65.21% <0%> (-13.73%)	7 <0> (ø)
.../elastic/apm/servlet/ServletTransactionHelper.java	73.01% <100%> (ø)	38 <8> (ø)	⬇️
.../java/co/elastic/apm/servlet/ServletApiAdvice.java	9.67% <40%> (+3.01%)	2 <0> (ø)	⬇️
...ava/co/elastic/apm/bci/bytebuddy/MatcherTimer.java	62.5% <0%> (-31.25%)	4% <0%> (-3%)
...elastic/apm/bci/MatcherTimerLifecycleListener.java	75% <0%> (-18.75%)	4% <0%> (-1%)
.../main/java/co/elastic/apm/bci/ElasticApmAgent.java	69.92% <0%> (-2.26%)	18% <0%> (-1%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7f5107...b95f749. Read the comment docs.