proposal: split inject/extract baggage prefix in zipkin/propagation.go

[zacscoding]

Proposal - what do you suggest to solve the problem or improve the existing situation?

How about split inject and extract baggage prefix instead of common prefix?

For example, suppose we receive x-project-id and x-project-token from another server

and we just want to propagate x-project-id only.

If i set baggage prefix to empty string, then propagate all, "x-" prefix also.

How can i propagate only x-project-id via baggage items without any additional code?

[yurishkuro]

Please try to use more descriptive issue titles in the future (describe problem, not solution), your title seems barely related to the content of the question.

The baggage api has an iterator and the ability to unset the values, so you can implement any kind of custom filtering in your own code, it does not need to be a feature of the jaeger client.

[zacscoding]

@yurishkuro thx for answer, i'll do that next time :)
How can i unset the values from span or span context? i just found iterator for read and setter

[yurishkuro]

set to bank values

[zacscoding]

I though there is a way to delete baggage
How about adding a baggage filter in zipkin/propagation.go?
Then it is helpful propagate my custom header only not all such as x-request-id, x-account-id and etc. :D

[yurishkuro]

Setting to blank value should delete the entry

[zacscoding]

thx for ur kindness :) but in my case, it is not working.
Here are my full code, Is there any misuse in my code?
(in this case, i just want to propagate x-request-id only and delete x-custom

ctx := context.Background()
propagator := zipkin.NewZipkinB3HTTPHeaderPropagator(zipkin.BaggagePrefix(""))
tracer, closer := jaeger.NewTracer("test",
    jaeger.NewConstSampler(false),
    jaeger.NewNullReporter(),
    jaeger.TracerOptions.Injector(opentracing.HTTPHeaders, propagator),
    jaeger.TracerOptions.Extractor(opentracing.HTTPHeaders, propagator),
)
opentracing.SetGlobalTracer(tracer)
defer closer.Close()

// 2) extract span from request
reqHeader := make(http.Header)
reqHeader.Set("x-b3-traceid", "520074fd2207a4e3")
reqHeader.Set("x-b3-spanid", "2983658a41996bad")
reqHeader.Set("x-b3-parentspanid", "7cf2b60cc11b4db3")
reqHeader.Set("x-request-id", "66e45b9f-85d6-4791-913b-71a10cc16a72") // want to propagate only
reqHeader.Set("x-custom", "custom-value")                             // want to delete baggage

wireContext, err := tracer.Extract(opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(reqHeader))
assert.NoError(t, err)
span := opentracing.StartSpan("/root", opentracing.ChildOf(wireContext))
span.Context().ForeachBaggageItem(func(k, v string) bool {
    switch strings.ToLower(k) {
    case "x-request-id":
    default:
        span.SetBaggageItem(k, "")
    }
    return true
})
defer span.Finish()
ctx = opentracing.ContextWithSpan(ctx, span)

// 3) start child span
currentSpan := opentracing.SpanFromContext(ctx)
childSpan := opentracing.StartSpan("/child", opentracing.ChildOf(currentSpan.Context()))
childHeader := make(http.Header)
tracer.Inject(childSpan.Context(), opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(childHeader))
for k, values := range childHeader {
    fmt.Println("Key:", k, ", Values:", strings.Join(values, ","))
}
// Output
//Key: X-B3-Traceid , Values: 520074fd2207a4e3
//Key: X-B3-Parentspanid , Values: 799790be7bbe8988
//Key: X-B3-Spanid , Values: f1994a95284d02b
//Key: X-B3-Sampled , Values: 0
//Key: X-Request-Id , Values: 66e45b9f-85d6-4791-913b-71a10cc16a72
//Key: X-Custom , Values:

[yurishkuro]

I see that there's no special code to remove keys with empty/nil values as we do in some other Jaeger SDKs. Feel free to add a PR.

jaeger-client-go/span_context.go

Lines 325 to 338 in fe3fa55

	func (c SpanContext) WithBaggageItem(key, value string) SpanContext {
	var newBaggage map[string]string
	if c.baggage == nil {
	newBaggage = map[string]string{key: value}
	} else {
	newBaggage = make(map[string]string, len(c.baggage)+1)
	for k, v := range c.baggage {
	newBaggage[k] = v
	}
	newBaggage[key] = value
	}
	// Use positional parameters so the compiler will help catch new fields.
	return SpanContext{c.traceID, c.spanID, c.parentID, newBaggage, "", c.samplingState, c.remote}
	}

On a separate note, this looks like a patently bad idea:

propagator := zipkin.NewZipkinB3HTTPHeaderPropagator(zipkin.BaggagePrefix(""))

If you don't use a prefix for baggage, every possible HTTP header will be treated as baggage and propagated.

[zacscoding]

Added a new pr #562 :)
I know it is a bad idea to configure "" baggage prefix.

So how about adding a BaggageFilter in zipkin.Propagator?
I think it is more flexible if provide a filter like below

type BaggageFilter interface {
	// InjectFilter returns a parsed key to propagate this baggage item and true if inject these key,value.
	InjectFilter(k, v string) (string, bool)
	// ExtractFilter returns a parsed key to set in baggage and true if extract these key,value.
	ExtractFilter(k, v string) (string, bool)
}

type PrefixBaggageFilter struct {
	prefix string
}

func (p *PrefixBaggageFilter) InjectFilter(k, _ string) (string, bool) {
	return p.prefix + k, true
}

func (p *PrefixBaggageFilter) ExtractFilter(k, _ string) (string, bool) {
	if !strings.HasPrefix(k, p.prefix) {
		return k, false
	}
	return k[len(p.prefix):], true
}

type Propagator struct {
	baggageFilter BaggageFilter
}