opentelemetry-java-instrumentation icon indicating copy to clipboard operation
opentelemetry-java-instrumentation copied to clipboard

Reduce allocations by using same UnsafeAttributes in Instrumenter.onEnd as onStart.

Open johnbley opened this issue 1 year ago • 0 comments

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.onEnd implementations and found no issues with passing in the same object as onStart. 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 original onStart attributes on their own, and his code was adjusted.
  • In the analysis, even Instrumenters without OperationListeners (i.e., not an http or rpc client or server library) still reduced allocations, since the UnsafeAttribute is more expensive to allocate than the context storage for itself.
  • ./gradlew :benchmark-overhead-jmh:jmh shows that gc.alloc.rate.norm drops 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.

johnbley avatar Apr 10 '24 14:04 johnbley