Add a Context that flows through span processors to SpanWriters

[esnible]

This PR replaces #3605 and is an alternative to #3660

This PR adds a Context to ProcessSpans(). This allows key/value pairs to flow through the processor down to the SpanWriter.

I hope to build on this when I introduce tenancy.

I have chosen to implement sending the tenant through the processors this way because

• We already use a context on the read path, e.g. /proto-gen/storage_v1/storage.pb.go which has GetTrace(ctx context.Context, in *GetTraceRequest, opts ...grpc.CallOption) (SpanReaderPlugin_GetTraceClient, error); it is symmetric to add a context to the write path.
• Contexts are traditionally used when data needs to flow through to an inner API; that is what we are doing.
• This is very flexible; only processors and writers that care need to look for the keys they honor
• Currently only the saveSpan() processor uses the context, and it only uses it to call the spanWriter. However, in the future, other processors might take advantage of the context settings.

cc @pavolloffay

[codecov]

Codecov Report

Merging #3659 (ce1a4c7) into main (754880c) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3659      +/-   ##
==========================================
+ Coverage   96.50%   96.52%   +0.02%     
==========================================
  Files         265      265              
  Lines       15581    15587       +6     
==========================================
+ Hits        15036    15045       +9     
+ Misses        455      453       -2     
+ Partials       90       89       -1     

Impacted Files	Coverage Δ
cmd/collector/app/model_consumer.go	100.00% <100.00%> (ø)
cmd/collector/app/options.go	100.00% <100.00%> (ø)
cmd/collector/app/root_span_handler.go	100.00% <100.00%> (ø)
cmd/collector/app/span_processor.go	100.00% <100.00%> (ø)
cmd/query/app/static_handler.go	97.60% <0.00%> (+1.79%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 754880c...ce1a4c7. Read the comment docs.

[yurishkuro]

have a concern about this one. First, it is not idiomatic and explicitly goes against recommendation to avoid storing context in structs. Second, the reason we have this struct is because we have a queue sitting between the RPC endpoint accepting spans and the processing pipeline. Which means the context coming from RPC has a different life-cycle than the processing life-cycle. For example, by the time the item is processed from the queue the RPC context may have been cancelled already, which won't affect passing the tenancy, but might affect sending DB requests if the same context is piped all the way through the writers.

I think it would be better to capture the tenancy explicitly at this boundary and let the queue consumer create its own context (and re-save the tenancy there if necessary)

[yurishkuro]

I think you want context.Background() in the tests - TODO() implies you want to change it in the future

[yurishkuro]

this is what I meant in the previous comment - I think this should always be context.Background() (with some enrichment) due to different life-cycles.