Tracing: change baggage key to be lower kebab-case for Jaeger compatability

[conorevans]

Signed-off-by: Conor Evans coevans@tcd.ie

• I added CHANGELOG entry for this change.
• Change is not relevant to the end user.

Closes #3951

Changes

Make baggage key lowercase for Jaeger support. Per Jaeger documentation:

Limitation: since HTTP headers don’t preserve the case, Jaeger recommends baggage keys to be lowercase-kebab-case, e.g. my-baggage-key-1.

Verification

Since this is a one-line const change, no tests have been added. I also did not feel terribly inclined to spin up a local Thanos+Jaeger set-up, though I can do so for completeness if desired.

[bwplotka]

I wonder.. can we maybe support both? It will not introduce breaking change for user too. Just let's lower case the input before checking?

[conorevans]

I considered that, but I wasn't sure I saw the benefit - now I think further, backwards compatibility is preferable, for sure.

I'm not 100% sure I'm envisioning it right, so I'll just check. In pkg/tracing/http.go

// If client specified ForceTracingBaggageKey header, ensure span includes it to force tracing.
span.SetBaggageItem(ForceTracingBaggageKey, r.Header.Get(ForceTracingBaggageKey))

here we'd set to span.SetBaggageItem(strings.ToLower(ForceTracingBaggageKey),...

Then set the JaegerDebugHeader in pkg/tracing/jaeger/jaeger.go to ToLower as well.

The only other place I can see that is a forceRecorder in pkg/tracing/stackdriver/tracer.go which itself is used for a basic tracer wrapping a gcloud tracer, so I don't see it necessary there?

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.