Needed Advice for fixing a TODO

[AnmolxSingh]

file cmd/query/app/querysvc/internal/adjuster/zipkin_span_id_uniquify.go has a func (d *spanIDDeduper) groupSpansByID() which groups spans with the same ID returning a map

that func has todo which needs to be fixed
Todo has a requirement that if we find more than 2 spans with same span id we could return an error but returning an error would be okay (breaking existing pipelines) or we should log a warning instead?

[dosubot[bot]]

The established pattern in Jaeger adjusters is to never return errors or break processing pipelines for data anomalies like duplicate span IDs. Instead, any issues are logged by appending descriptive warnings to the affected span's Warnings field. This approach is used consistently across the Zipkin Span ID Uniquifier, clock skew, and span reference adjusters, and is verified by both code and tests. For your TODO, logging a warning (e.g., "more than 2 spans found with the same span ID") is the recommended and compatible approach, rather than returning an error that could disrupt existing pipelines. See source and test example.

To reply, just mention @dosu.

---

^{How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other}    

[yurishkuro]

I do not see any negative side effects to the behavior of this adjuster if more than two spans with the same ID are found. The TODO may can be deleted.

[AnmolxSingh]

just for clarification.
In which case we can have multiple spans with the same ID?

[yurishkuro]

Zipkin SDKs used to reuse span ID between matching client and server spans. There is another theoretical use case of SDKs emitting partial spans and Jaeger query merging them together into one (#729), but this has never been properly implemented in Jaeger (even though it's quite useful in some scenarios, especially for long running spans).