feat: context propagation

[lukas-reining]

This PR

Adds context propagation as described in #81.

To me it looks like this should be implementable in all the current languages.

The following things I am not sure on:

• Is this concrete enough?
• Should 3.2 and 3.3 be switched around as 3.3 is assumed in 3.2?
• Should we nest the requirements more e.g. do we want to have a specific section for the propagator?

Related Issues

Fixes #81

[toddbaert]

Hey @lukas-reining - sorry for a slow response from my part on this. I will be reviewing this first thing on Monday.

[beeme1mr]

I wouldn't consider myself a polyglot developer, so I have some concerns about how this could be implemented in languages like Rust and Go. Perhaps we should change RFC-2119 keyword to "SHOULD" or "MAY" to avoid requiring a feature that can't reasonably be implemented in some technologies.

@sheepduke what are your thoughts from Rust perspective?

[austindrenski]

@beeme1mr wrote https://github.com/open-feature/spec/pull/227#issuecomment-1900692701:

[...] I have some concerns about how this could be implemented in languages like [...] Go. [...]

I'm relatively new to Go, but to me this sounds quite comparable to how OTel trace and baggage propagation works, i.e., via the context package.

For example, if I want to start a child trace in Go, I can write:

const scopeName = "github.com/some-repo/some-module"

func someFunc(ctx context.Context) error {
    // create a new child span from the incoming ctx, and overwrite/shadow it with a new ctx
    ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer(scopeName).Start(ctx, "some_func")
    defer span.End()

    // do stuff covered by the child span
    // ...
    // ...

    // now call into someOtherFunc, passing the overwritten/shadowed ctx
    return someOtherFunc(ctx)
}

func someOtherFunc(ctx context.Context) error {
    ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer(scopeName).Start(ctx, "some_other_function")
    defer span.End()

    // do stuff covered by the child (grandchild?) span
    // ...
    // ...

    return nil
}

With the resulting span looking something like:

github.com/some-repo/some-module:some_func       |-----------------------------------------|
github.com/some-repo/some-module:some_other_func               |---------------------------|

or if the caller to someFunc passed a ctx that already contained a span:

some_outer_caller                                |-----------------------------------------|
github.com/some-repo/some-module:some_func                |---------------|
github.com/some-repo/some-module:some_other_func               |----------|

Related

• trace/context.go

  // Copyright The OpenTelemetry Authors
  //
  // Licensed under the Apache License, Version 2.0 (the "License");
  // you may not use this file except in compliance with the License.
  // You may obtain a copy of the License at
  //
  //     http://www.apache.org/licenses/LICENSE-2.0
  //
  // Unless required by applicable law or agreed to in writing, software
  // distributed under the License is distributed on an "AS IS" BASIS,
  // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  // See the License for the specific language governing permissions and
  // limitations under the License.
  
  package trace // import "go.opentelemetry.io/otel/trace"
  
  import "context"
  
  type traceContextKeyType int
  
  const currentSpanKey traceContextKeyType = iota
  
  // ContextWithSpan returns a copy of parent with span set as the current Span.
  func ContextWithSpan(parent context.Context, span Span) context.Context {
    return context.WithValue(parent, currentSpanKey, span)
  }
  
  // ContextWithSpanContext returns a copy of parent with sc as the current
  // Span. The Span implementation that wraps sc is non-recording and performs
  // no operations other than to return sc as the SpanContext from the
  // SpanContext method.
  func ContextWithSpanContext(parent context.Context, sc SpanContext) context.Context {
    return ContextWithSpan(parent, nonRecordingSpan{sc: sc})
  }
  
  // ContextWithRemoteSpanContext returns a copy of parent with rsc set explicly
  // as a remote SpanContext and as the current Span. The Span implementation
  // that wraps rsc is non-recording and performs no operations other than to
  // return rsc as the SpanContext from the SpanContext method.
  func ContextWithRemoteSpanContext(parent context.Context, rsc SpanContext) context.Context {
    return ContextWithSpanContext(parent, rsc.WithRemote(true))
  }
  
  // SpanFromContext returns the current Span from ctx.
  //
  // If no Span is currently set in ctx an implementation of a Span that
  // performs no operations is returned.
  func SpanFromContext(ctx context.Context) Span {
    if ctx == nil {
        return noopSpan{}
    }
    if span, ok := ctx.Value(currentSpanKey).(Span); ok {
        return span
    }
    return noopSpan{}
  }
  
  // SpanContextFromContext returns the current Span's SpanContext.
  func SpanContextFromContext(ctx context.Context) SpanContext {
    return SpanFromContext(ctx).SpanContext()
  }

[lukas-reining]

I'm relatively new to Go, but to me this sounds quite comparable to how OTel trace and baggage propagation works, i.e., via the context package.

Yes exactly, this is how it was meant.

[lukas-reining]

Perhaps we should change RFC-2119 keyword to "SHOULD" or "MAY" to avoid requiring a feature that can't reasonably be implemented in some technologies.

I would be fine with this, but my feeling is that there should be a way in all languages. But to be sure we could also just go for SHOULD to prevent the case of a language not capable of implementing it. @beeme1mr

[lukas-reining]

Perhaps we should change RFC-2119 keyword to "SHOULD" or "MAY" to avoid requiring a feature that can't reasonably be implemented in some technologies.

I would be fine with this, but my feeling is that there should be a way in all languages. But to be sure we could also just go for SHOULD to prevent the case of a language not capable of implementing it. @beeme1mr

@beeme1mr If we go for SHOULD, how would we express that if transaction propagation is implemented, then all methods have to be implemented. A new condition feels too much? Or maybe that the right thing here?

[beeme1mr]

I would just add a short blurb in the non-normative section.

[toddbaert]

Perhaps we should change RFC-2119 keyword to "SHOULD" or "MAY" to avoid requiring a feature that can't reasonably be implemented in some technologies. I would be fine with this, but my feeling is that there should be a way in all languages. But to be sure we could also just go for SHOULD to prevent the case of a language not capable of implementing it. @beeme1mr

@beeme1mr If we go for SHOULD, how would we express that if transaction propagation is implemented, then all methods have to be implemented. A new condition feels too much? Or maybe that the right thing here?

I think you could do a single condition and put everything under that if you want, but I also think you could get away without one.

You could use a should/may and then just say "the transaction propagation implementation must..." and I think people would probably interpret that as not applicable.

[lukas-reining]

Perhaps we should change RFC-2119 keyword to "SHOULD" or "MAY" to avoid requiring a feature that can't reasonably be implemented in some technologies. I would be fine with this, but my feeling is that there should be a way in all languages. But to be sure we could also just go for SHOULD to prevent the case of a language not capable of implementing it. @beeme1mr

@beeme1mr If we go for SHOULD, how would we express that if transaction propagation is implemented, then all methods have to be implemented. A new condition feels too much? Or maybe that the right thing here?

I think you could do a single condition and put everything under that if you want, but I also think you could get away without one.

You could use a should/may and then just say "the transaction propagation implementation must..." and I think people would probably interpret that as not applicable.

I chose to add a condition now @toddbaert @beeme1mr. It feels the cleanest to me this way. I also tried to say "the transaction propagation implementation must..." but it felt hard for 3.3.1.2.1 to say this and that the method must be on the API.

The only concern I have is the deep nesting, something like 3.3.1.2.1 feels pretty heavy but it feels better than having conditions or assumptions in the spec sentences. What do you think?

[toddbaert]

The only concern I have is the deep nesting, something like 3.3.1.2.1 feels pretty heavy but it feels better than having conditions or assumptions in the spec sentences. What do you think?

I'm OK with the deep nesting. I hope we never have to go deeper than 5, but I think it's fine.

[lukas-reining]

As said in the community meeting we have enough approvals now and it seems that we have a consensus. We will merge this next Wednesday (08.02) if there is no blocking feedback or any objections until then.