From cf8927bebb4afe758ab94731560cf8606eb9589b Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 16 Aug 2020 16:45:40 -0400 Subject: [PATCH] Add comments explaining copy-on-write behavior of baggage Per issue #526. Signed-off-by: Yuri Shkuro --- span.go | 9 ++++++++- span_context.go | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/span.go b/span.go index 42c9112c..efc37067 100644 --- a/span.go +++ b/span.go @@ -303,7 +303,14 @@ func (s *Span) fixLogsIfDropped() { s.numDroppedLogs = 0 } -// SetBaggageItem implements SetBaggageItem() of opentracing.SpanContext +// SetBaggageItem implements SetBaggageItem() of opentracing.SpanContext. +// The call is proxied via tracer.baggageSetter to allow policies to be applied +// before allowing to set/replace baggage keys. +// The setter eventually stores a new SpanContext with extended baggage: +// +// span.context = span.context.WithBaggageItem(key, value) +// +// See SpanContext.WithBaggageItem() for explanation why it's done this way. func (s *Span) SetBaggageItem(key, value string) opentracing.Span { s.Lock() defer s.Unlock() diff --git a/span_context.go b/span_context.go index ae9d94a9..03aa99a0 100644 --- a/span_context.go +++ b/span_context.go @@ -304,6 +304,14 @@ func (c *SpanContext) CopyFrom(ctx *SpanContext) { } // WithBaggageItem creates a new context with an extra baggage item. +// +// The SpanContext is designed to be immutable and passed by value. As such, +// it cannot contain any locks, and should only hold immutable data, including baggage. +// Another reason for why baggage is immutable is when the span context is passed +// as a parent when starting a new span. The new span's baggage cannot affect the parent +// span's baggage, so the child span either needs to take a copy of the parent baggage +// (which is expensive and unnecessary since baggage rarely changes in the life span of +// a trace), or it needs to do a copy-on-write, which is the approach taken here. func (c SpanContext) WithBaggageItem(key, value string) SpanContext { var newBaggage map[string]string if c.baggage == nil {