[File based config] Snapshot profiling span processor implementation - #2542
Conversation
…-snapshot-span-processor
| private final TraceRegistry traceRegistry; | ||
|
|
||
| public SnapshotProfilingSpanProcessorComponentProvider() { | ||
| this(TraceRegistryHolder.getTraceRegistry()); | ||
| } | ||
|
|
||
| SnapshotProfilingSpanProcessorComponentProvider(TraceRegistry traceRegistry) { | ||
| this.traceRegistry = traceRegistry; | ||
| } |
There was a problem hiding this comment.
I don't think that you actually need to hold this global static in a field on this class -- just call TraceRegistryHolder.getTraceRegistry() in the create() method below right?
There was a problem hiding this comment.
The clue of TraceRegistryHolder class and static instance of TraceRegistry is to share this instance between SnapshotProfilingConfigurationCustomizerProvider and SnapshotProfilingSpanProcessorComponentProvider, which is impossible via YAML and ComponentProvider SPI.
This is not obvious and I should make a comment about it in code review initially.
breedx-splk
left a comment
breedx-splk
left a comment
There was a problem hiding this comment.
Had one comment to make things a bit more tidy, but looks fine overall.
|
|
||
| package com.splunk.opentelemetry.profiler.snapshot; | ||
|
|
||
| class TraceRegistryHolder { |
There was a problem hiding this comment.
Rest of the code uses ConfigurableSupplier in similar situations. I have a feeling that the existing code might be overcomplicating things we may need to revisit it at one point and see whether it could be improved.
3 participants
Snapshot profiling span processor can be used in YAML with the following snippet: