Skip to content

Commit

Permalink
Replace and deprecate baggagetrace (#5824)
Browse files Browse the repository at this point in the history
Resolve #5808 

- Move current version of
`go.opentelemetry.io/contrib/processors/baggage/baggagetrace` to
`go.opentelemetry.io/contrib/processors/baggagecopy`
- Rename `baggagecopy.New` to `baggagecopy.NewSpanProcessor` to allow
for future processors
- Restore `go.opentelemetry.io/contrib/processors/baggage/baggagetrace`
to its last released version (v0.0.1)
  - This excludes dependency upgrades
- Deprecate
`go.opentelemetry.io/contrib/processors/baggage/baggagetrace`
  • Loading branch information
MrAlias authored Jul 1, 2024
1 parent 5b6a960 commit 6e8528b
Show file tree
Hide file tree
Showing 12 changed files with 360 additions and 194 deletions.
15 changes: 8 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add log support for the autoexport package. (#5733)
- Add support for disabling the old runtime metrics using the `OTEL_GO_X_DEPRECATED_RUNTIME_METRICS=false` environment variable. (#5747)
- Add support for signal-specific protocols environment variables (`OTEL_EXPORTER_OTLP_TRACES_PROTOCOL`, `OTEL_EXPORTER_OTLP_LOGS_PROTOCOL`, `OTEL_EXPORTER_OTLP_METRICS_PROTOCOL`) in `go.opentelemetry.io/contrib/exporters/autoexport`. (#5816)
- The `go.opentelemetry.io/contrib/processors/baggagecopy` module.
This module is a replacment of `go.opentelemetry.io/contrib/processors/baggage/baggagetrace`. (#5824)

### Fixed

Expand All @@ -24,7 +26,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Use `c.FullPath()` method to set `http.route` attribute in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#5734)
- The double setup in `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace/example` that caused duplicate traces. (#5564)
- Out-of-bounds panic in case of invalid span ID in `go.opentelemetry.io/contrib/propagators/b3`. (#5754)
- Do not panic if a zero-value `SpanProcessor` is used from `go.opentelemetry.io/contrib/processors/baggage/baggagetrace`. (#5811)

### Deprecated

Expand All @@ -36,6 +37,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
If you would like to become a Code Owner of this module and prevent it from being removed, see [#5552]. (#5646)
- The `go.opentelemetry.io/contrib/samplers/aws/xray` package is deprecated.
If you would like to become a Code Owner of this module and prevent it from being removed, see [#5554]. (#5647)
- The `go.opentelemetry.io/contrib/processors/baggage/baggagetrace` package is deprecated.
Use the added `go.opentelemetry.io/contrib/processors/baggagecopy` package instead. (#5824)
- Use `baggagecopy.NewSpanProcessor` as a replacement for `baggagetrace.New`.
- `NewSpanProcessor` accepts a `Fitler` function type that selects which baggage members are added to a span.
- `NewSpanProcessor` returns a `*baggagecopy.SpanProcessor` instead of a `trace.SpanProcessor` interface.
The returned type still implements the interface.

[#5550]: https://github.com/open-telemetry/opentelemetry-go-contrib/issues/5550
[#5551]: https://github.com/open-telemetry/opentelemetry-go-contrib/issues/5551
Expand All @@ -47,10 +54,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Improve performance of `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` with the usage of `WithAttributeSet()` instead of `WithAttribute()`. (#5664)
- Improve performance of `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` with the usage of `WithAttributeSet()` instead of `WithAttribute()`. (#5664)
- Update `go.opentelemetry.io/contrib/config` to latest released configuration schema which introduces breaking changes where `Attributes` is now a `map[string]interface{}`. (#5758)
- Rename `BaggageKeyPredicate` to `Filter` in `go.opentelemetry.io/contrib/processors/baggage/baggagetrace`. (#5809)
- Return `*SpanProcessor` from `"go.opentelemetry.io/contrib/processors/baggage/baggagetrace".New` instead of the `trace.SpanProcessor` interface. (#5810)
- The `Filter` in `go.opentelemetry.io/contrib/processors/baggage/baggagetrace` now accepts a `baggage.Member` as a parameter instead of a string. (#5813)
- Rename `AllowAllBaggageKeys` to `AllowAllMembers` in `go.opentelemetry.io/contrib/processors/baggage/baggagetrace`. (#5813)

## [1.27.0/0.52.0/0.21.0/0.7.0/0.2.0] - 2024-05-21

Expand Down Expand Up @@ -84,8 +87,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the returned `Handler`. (#5588)
- Upgrade all dependencies of `go.opentelemetry.io/otel/semconv/v1.24.0` to `go.opentelemetry.io/otel/semconv/v1.25.0`. (#5605)

- Update the span processor in `go.opentelemetry.io/contrib/processors/baggage/baggagetrace` to require a baggage key predicate. (#5619)

### Removed

- The `WithInstrumentationScope` option function in `go.opentelemetry.io/contrib/bridges/otelslog` is removed.
Expand Down
24 changes: 2 additions & 22 deletions processors/baggage/baggagetrace/doc.go
Original file line number Diff line number Diff line change
@@ -1,27 +1,7 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

// Package baggagetrace is an OpenTelemetry [Span Processor] that reads key/values
// stored in [Baggage] in the starting span's parent context and adds them as
// attributes to the span.
// Package baggagetrace implements a baggage span processor.
//
// Keys and values added to Baggage will appear on all subsequent child spans for
// a trace within this service and will be propagated to external services via
// propagation headers.
// If the external services also have a Baggage span processor, the keys and
// values will appear in those child spans as well.
//
// Do not put sensitive information in Baggage.
//
// # Usage
//
// Add the span processor when configuring the tracer provider.
//
// The convenience function [AllowAllBaggageKeys] is provided to
// allow all baggage keys to be copied to the span. Alternatively, you can
// provide a custom baggage key predicate to select which baggage keys you want
// to copy.
//
// [Span Processor]: https://opentelemetry.io/docs/specs/otel/trace/sdk/#span-processor
// [Baggage]: https://opentelemetry.io/docs/specs/otel/api/baggage
// Deprecated: Use go.opentelemetry.io/contrib/processors/baggagecopy instead.
package baggagetrace // import "go.opentelemetry.io/contrib/processors/baggage/baggagetrace"
4 changes: 4 additions & 0 deletions processors/baggage/baggagetrace/go.mod
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Deprecated: Use go.opentelemetry.io/contrib/processors/baggagecopy instead.
module go.opentelemetry.io/contrib/processors/baggage/baggagetrace

go 1.21

require (
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/contrib/processors/baggagecopy v0.0.0-00010101000000-000000000000
go.opentelemetry.io/otel v1.27.0
go.opentelemetry.io/otel/sdk v1.27.0
)
Expand All @@ -18,3 +20,5 @@ require (
golang.org/x/sys v0.21.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace go.opentelemetry.io/contrib/processors/baggagecopy => ../../baggagecopy
39 changes: 10 additions & 29 deletions processors/baggage/baggagetrace/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,48 +7,29 @@ import (
"context"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/baggage"
otelbaggage "go.opentelemetry.io/otel/baggage"
"go.opentelemetry.io/otel/sdk/trace"
)

// Filter returns true if the baggage member should be added to a span.
type Filter func(member baggage.Member) bool

// AllowAllMembers allows all baggage members to be added to a span.
var AllowAllMembers Filter = func(baggage.Member) bool { return true }
"go.opentelemetry.io/contrib/processors/baggagecopy"
)

// SpanProcessor is a [trace.SpanProcessor] implementation that adds baggage
// members onto a span as attributes.
type SpanProcessor struct {
filter Filter
}
// SpanProcessor is a processing pipeline for spans in the trace signal.
type SpanProcessor struct{}

var _ trace.SpanProcessor = (*SpanProcessor)(nil)

// New returns a new [SpanProcessor].
// New returns a new SpanProcessor.
//
// The Baggage span processor duplicates onto a span the attributes found
// in Baggage in the parent context at the moment the span is started.
// The passed filter determines which baggage members are added to the span.
//
// If filter is nil, all baggage members will be added.
func New(filter Filter) *SpanProcessor {
return &SpanProcessor{
filter: filter,
}
func New() trace.SpanProcessor {
return baggagecopy.NewSpanProcessor(baggagecopy.AllowAllMembers)
}

// OnStart is called when a span is started and adds span attributes for baggage contents.
func (processor SpanProcessor) OnStart(ctx context.Context, span trace.ReadWriteSpan) {
filter := processor.filter
if filter == nil {
filter = AllowAllMembers
}

for _, member := range baggage.FromContext(ctx).Members() {
if filter(member) {
span.SetAttributes(attribute.String(member.Key(), member.Value()))
}
for _, entry := range otelbaggage.FromContext(ctx).Members() {
span.SetAttributes(attribute.String(entry.Key(), entry.Value()))
}
}

Expand Down
144 changes: 13 additions & 131 deletions processors/baggage/baggagetrace/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@ package baggagetrace

import (
"context"
"regexp"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/baggage"
otelbaggage "go.opentelemetry.io/otel/baggage"
"go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/sdk/trace/tracetest"
)

var _ trace.SpanExporter = &testExporter{}
Expand All @@ -36,72 +33,19 @@ func NewTestExporter() *testExporter {
return &testExporter{}
}

func TestSpanProcessorAppendsAllBaggageAttributes(t *testing.T) {
b, _ := baggage.New()
b = addEntryToBaggage(t, b, "baggage.test", "baggage value")
ctx := baggage.ContextWithBaggage(context.Background(), b)

// create trace provider with baggage processor and test exporter
exporter := NewTestExporter()
tp := trace.NewTracerProvider(
trace.WithSpanProcessor(New(AllowAllMembers)),
trace.WithSpanProcessor(trace.NewSimpleSpanProcessor(exporter)),
)

// create tracer and start/end span
tracer := tp.Tracer("test")
_, span := tracer.Start(ctx, "test")
span.End()

require.Len(t, exporter.spans, 1)
require.Len(t, exporter.spans[0].Attributes(), 1)

want := []attribute.KeyValue{attribute.String("baggage.test", "baggage value")}
require.Equal(t, want, exporter.spans[0].Attributes())
}

func TestSpanProcessorAppendsBaggageAttributesWithHaPrefixPredicate(t *testing.T) {
b, _ := baggage.New()
b = addEntryToBaggage(t, b, "baggage.test", "baggage value")
ctx := baggage.ContextWithBaggage(context.Background(), b)

baggageKeyPredicate := func(m baggage.Member) bool {
return strings.HasPrefix(m.Key(), "baggage.")
}

// create trace provider with baggage processor and test exporter
exporter := NewTestExporter()
tp := trace.NewTracerProvider(
trace.WithSpanProcessor(New(baggageKeyPredicate)),
trace.WithSpanProcessor(trace.NewSimpleSpanProcessor(exporter)),
)

// create tracer and start/end span
tracer := tp.Tracer("test")
_, span := tracer.Start(ctx, "test")
span.End()

require.Len(t, exporter.spans, 1)
require.Len(t, exporter.spans[0].Attributes(), 1)

want := []attribute.KeyValue{attribute.String("baggage.test", "baggage value")}
require.Equal(t, want, exporter.spans[0].Attributes())
}

func TestSpanProcessorAppendsBaggageAttributesWithRegexPredicate(t *testing.T) {
b, _ := baggage.New()
b = addEntryToBaggage(t, b, "baggage.test", "baggage value")
ctx := baggage.ContextWithBaggage(context.Background(), b)

expr := regexp.MustCompile(`^baggage\..*`)
baggageKeyPredicate := func(m baggage.Member) bool {
return expr.MatchString(m.Key())
}
func TestSpanProcessorAppendsBaggageAttributes(t *testing.T) {
suitcase, err := otelbaggage.New()
require.NoError(t, err)
packingCube, err := otelbaggage.NewMemberRaw("baggage.test", "baggage value")
require.NoError(t, err)
suitcase, err = suitcase.SetMember(packingCube)
require.NoError(t, err)
ctx := otelbaggage.ContextWithBaggage(context.Background(), suitcase)

// create trace provider with baggage processor and test exporter
exporter := NewTestExporter()
tp := trace.NewTracerProvider(
trace.WithSpanProcessor(New(baggageKeyPredicate)),
trace.WithSpanProcessor(New()),
trace.WithSpanProcessor(trace.NewSimpleSpanProcessor(exporter)),
)

Expand All @@ -110,71 +54,9 @@ func TestSpanProcessorAppendsBaggageAttributesWithRegexPredicate(t *testing.T) {
_, span := tracer.Start(ctx, "test")
span.End()

require.Len(t, exporter.spans, 1)
require.Len(t, exporter.spans[0].Attributes(), 1)
assert.Len(t, exporter.spans, 1)
assert.Len(t, exporter.spans[0].Attributes(), 1)

want := []attribute.KeyValue{attribute.String("baggage.test", "baggage value")}
require.Equal(t, want, exporter.spans[0].Attributes())
assert.Equal(t, want, exporter.spans[0].Attributes())
}

func TestOnlyAddsBaggageEntriesThatMatchPredicate(t *testing.T) {
b, _ := baggage.New()
b = addEntryToBaggage(t, b, "baggage.test", "baggage value")
b = addEntryToBaggage(t, b, "foo", "bar")
ctx := baggage.ContextWithBaggage(context.Background(), b)

baggageKeyPredicate := func(m baggage.Member) bool {
return m.Key() == "baggage.test"
}

// create trace provider with baggage processor and test exporter
exporter := NewTestExporter()
tp := trace.NewTracerProvider(
trace.WithSpanProcessor(New(baggageKeyPredicate)),
trace.WithSpanProcessor(trace.NewSimpleSpanProcessor(exporter)),
)

// create tracer and start/end span
tracer := tp.Tracer("test")
_, span := tracer.Start(ctx, "test")
span.End()

require.Len(t, exporter.spans, 1)
require.Len(t, exporter.spans[0].Attributes(), 1)

want := attribute.String("baggage.test", "baggage value")
require.Equal(t, want, exporter.spans[0].Attributes()[0])
}

func addEntryToBaggage(t *testing.T, b baggage.Baggage, key, value string) baggage.Baggage {
member, err := baggage.NewMemberRaw(key, value)
require.NoError(t, err)
b, err = b.SetMember(member)
require.NoError(t, err)
return b
}

func TestZeroSpanProcessorNoPanic(t *testing.T) {
sp := new(SpanProcessor)

m, err := baggage.NewMember("key", "val")
require.NoError(t, err)
b, err := baggage.New(m)
require.NoError(t, err)

ctx := baggage.ContextWithBaggage(context.Background(), b)
roS := (tracetest.SpanStub{}).Snapshot()
rwS := rwSpan{}
assert.NotPanics(t, func() {
sp.OnStart(ctx, rwS)
sp.OnEnd(roS)
_ = sp.ForceFlush(ctx)
_ = sp.Shutdown(ctx)
})
}

type rwSpan struct {
trace.ReadWriteSpan
}

func (s rwSpan) SetAttributes(kv ...attribute.KeyValue) {}
27 changes: 27 additions & 0 deletions processors/baggagecopy/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

// Package baggagecopy is an OpenTelemetry [Span Processor] that reads key/values
// stored in [Baggage] in the starting span's parent context and adds them as
// attributes to the span.
//
// Keys and values added to Baggage will appear on all subsequent child spans for
// a trace within this service and will be propagated to external services via
// propagation headers.
// If the external services also have a Baggage span processor, the keys and
// values will appear in those child spans as well.
//
// Do not put sensitive information in Baggage.
//
// # Usage
//
// Add the span processor when configuring the tracer provider.
//
// The convenience function [AllowAllBaggageKeys] is provided to
// allow all baggage keys to be copied to the span. Alternatively, you can
// provide a custom baggage key predicate to select which baggage keys you want
// to copy.
//
// [Span Processor]: https://opentelemetry.io/docs/specs/otel/trace/sdk/#span-processor
// [Baggage]: https://opentelemetry.io/docs/specs/otel/api/baggage
package baggagecopy // import "go.opentelemetry.io/contrib/processors/baggagecopy"
Loading

0 comments on commit 6e8528b

Please sign in to comment.