opentelemetry-java-instrumentation
opentelemetry-java-instrumentation copied to clipboard
Reduce allocations by using same UnsafeAttributes in Instrumenter.onEnd as onStart.
This idea takes the UnsafeAttributes allocated in onStart, stores it in the context (with the start time), and then reuses it in onEnd.
- I audited all 72
AttributeExtractor.onEndimplementations and found no issues with passing in the same object asonStart. There were 34 that did nothing with the parameter, 28 that simply did puts, 1 funny one that did a remove (HttpMethodAttributeExtractor), 7 that were just tests, and 2 that existed just to proxy type transforms. - I audited the
OperationListeners too and they all got a lot simpler from this (in addition to further reducing allocations). Many of them were storing the starting attributes and then doing a lot of copying to make a new combined start+end attributes. Only one (HttpServerExperimentalMetrics) specifically needed to keep the originalonStartattributes on their own, and his code was adjusted. - In the analysis, even
Instrumenters withoutOperationListeners (i.e., not an http or rpc client or server library) still reduced allocations, since theUnsafeAttributeis more expensive to allocate than the context storage for itself. ./gradlew :benchmark-overhead-jmh:jmhshows thatgc.alloc.rate.normdrops from 41119 B/op to 40453 B/op (666 thrash GC bytes per op saving).
The only funny edge case to me is if, somehow, the context storage doesn't work. In that scenario, the starting attributes are "lost" and the operation listeners can't be run. I added logging and checks for this (similar to the logging that existed in this "can't happen" case for the operation listeners doing the same work previously) but it's not as clean as I would like, I think in part because there isn't an "error API" for the context storage insertion (this concept doesn't really make sense to me) which could allow for better fallback code.