Clean orphaned traces #2330
Clean orphaned traces #2330
Conversation
| void register(Span span) { | ||
| traces.add(new WeakKey(span, referenceQueue)); | ||
| } | ||
|
|
||
| void unregister(SpanContext spanContext) { | ||
| traces.remove(new LookupKey(spanContext)); | ||
| } |
There was a problem hiding this comment.
I think I grok the reasons why, but the asymmetry between register(span) and unregister(context) gives me pause. 🤷🏻
|
|
||
| SnapshotProfilingSpanProcessor(TraceRegistry registry, Supplier<StackTraceSampler> sampler) { | ||
| this.registry = registry; | ||
| this.sampler = sampler; | ||
| this.orphanedTraceCleaner = new OrphanedTraceCleaner(registry, sampler); |
There was a problem hiding this comment.
nit: inject it or make a factory method, because this violates DI principles.
| @@ -24,15 +24,27 @@ | |||
| class TraceRegistry { | |||
| private final Set<String> traceIds = Collections.newSetFromMap(new ConcurrentHashMap<>()); | |||
There was a problem hiding this comment.
Does this make the Set also concurrent/threadsafe?
There was a problem hiding this comment.
The main implementation here (the OrphanedTraceCleaner) seems very similar to the implementation in #2320, with the main difference being that the Registry is responsible for it in #2320 but here we have a separate cleaner that the SnapshotProfilingSpanProcessor delegates to.
To me, this looks like a simpler approach overall, with less code and a slightly clearer separation of responsibilities. I'm not an expert here, so I'm also curious about what @tduncan thinks. Thanks!
An alternative for #2320