diff --git a/CHANGELOG.md b/CHANGELOG.md index ba983f0cf34..3663ba58bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added -- Added `Marshler` config option to `otlphttp` to enable otlp over json or protobufs. (#1586) +- Added `Marshaler` config option to `otlphttp` to enable otlp over json or protobufs. (#1586) - A `ForceFlush` method to the `"go.opentelemetry.io/otel/sdk/trace".TracerProvider` to flush all registered `SpanProcessor`s. (#1608) ### Changed @@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Removed +- Removed `WithRecord()` from `trace.SpanOption` when creating a span. (#1660) - Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs. These are now returned as a SpanProcessor interface from their respective constructors. (#1638) - Removed setting status to `Error` while recording an error as a span event in `RecordError`. (#1663) @@ -97,7 +98,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The sequential timing check of timestamps in the stdout exporter are now setup explicitly to be sequential (#1571). (#1572) - Windows build of Jaeger tests now compiles with OS specific functions (#1576). (#1577) - The sequential timing check of timestamps of go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue are now setup explicitly to be sequential (#1578). (#1579) -- Validate tracestate header keys with vedors according to the W3C TraceContext specification (#1475). (#1581) +- Validate tracestate header keys with vendors according to the W3C TraceContext specification (#1475). (#1581) - The OTLP exporter includes related labels for translations of a GaugeArray (#1563). (#1570) - Jaeger Exporter: Ensure mapping between OTEL and Jaeger span data complies with the specification. (#1626) diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 4036b863522..36df70fbd75 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -411,7 +411,6 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio trace.WithAttributes(attributes...), trace.WithTimestamp(sso.StartTime), trace.WithLinks(links...), - trace.WithRecord(), trace.WithSpanKind(kind), ) if checkCtx != checkCtx2 { diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index 7d6459c6228..54d0b5bfe03 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -85,7 +85,6 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanO mockTracer: t, officialTracer: t, spanContext: spanContext, - recording: config.Record, Attributes: baggage.NewMap(baggage.MapUpdate{ MultiKV: config.Attributes, }), diff --git a/oteltest/harness.go b/oteltest/harness.go index f6809e0273c..0dcaf46f305 100644 --- a/oteltest/harness.go +++ b/oteltest/harness.go @@ -152,17 +152,6 @@ func (h *Harness) TestTracer(subjectFactory func() trace.Tracer) { e.Expect(sc1.SpanID()).NotToEqual(sc2.SpanID()) }) - t.Run("records the span if specified", func(t *testing.T) { - t.Parallel() - - e := matchers.NewExpecter(t) - subject := subjectFactory() - - _, span := subject.Start(context.Background(), "span", trace.WithRecord()) - - e.Expect(span.IsRecording()).ToBeTrue() - }) - t.Run("propagates a parent's trace ID through the context", func(t *testing.T) { t.Parallel() diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 99b236727f4..4d31579a690 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -558,10 +558,15 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac links: o.Links, kind: o.SpanKind, } - sampled := makeSamplingDecision(data) - span.spanContext = span.spanContext.WithTraceState(sampled.Tracestate) + samplingResult := makeSamplingDecision(data) + if isSampled(samplingResult) { + span.spanContext = span.spanContext.WithTraceFlags(span.spanContext.TraceFlags() | trace.FlagsSampled) + } else { + span.spanContext = span.spanContext.WithTraceFlags(span.spanContext.TraceFlags() &^ trace.FlagsSampled) + } + span.spanContext = span.spanContext.WithTraceState(samplingResult.Tracestate) - if !span.spanContext.IsSampled() && !o.Record { + if !isRecording(samplingResult) { return span } @@ -577,7 +582,7 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac span.resource = cfg.Resource span.instrumentationLibrary = tr.instrumentationLibrary - span.SetAttributes(sampled.Attributes...) + span.SetAttributes(samplingResult.Attributes...) span.parent = parent @@ -602,7 +607,7 @@ type samplingData struct { func makeSamplingDecision(data samplingData) SamplingResult { sampler := data.cfg.DefaultSampler - sampled := sampler.ShouldSample(SamplingParameters{ + return sampler.ShouldSample(SamplingParameters{ ParentContext: data.parent, TraceID: data.span.spanContext.TraceID(), Name: data.name, @@ -611,10 +616,12 @@ func makeSamplingDecision(data samplingData) SamplingResult { Attributes: data.attributes, Links: data.links, }) - if sampled.Decision == RecordAndSample { - data.span.spanContext = data.span.spanContext.WithTraceFlags(data.span.spanContext.TraceFlags() | trace.FlagsSampled) - } else { - data.span.spanContext = data.span.spanContext.WithTraceFlags(data.span.spanContext.TraceFlags() &^ trace.FlagsSampled) - } - return sampled +} + +func isRecording(s SamplingResult) bool { + return s.Decision == RecordOnly || s.Decision == RecordAndSample +} + +func isSampled(s SamplingResult) bool { + return s.Decision == RecordAndSample } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 2b8d2f184dd..4c4406d44c6 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -864,7 +864,6 @@ func startSpan(tp *TracerProvider, trName string, args ...trace.SpanOption) trac func startNamedSpan(tp *TracerProvider, trName, name string, args ...trace.SpanOption) trace.Span { ctx := context.Background() ctx = trace.ContextWithRemoteSpanContext(ctx, remoteSpanContext()) - args = append(args, trace.WithRecord()) _, span := tp.Tracer(trName).Start( ctx, name, @@ -878,7 +877,6 @@ func startNamedSpan(tp *TracerProvider, trName, name string, args ...trace.SpanO // along with the span so this parent can be used to create child // spans. func startLocalSpan(tp *TracerProvider, ctx context.Context, trName, name string, args ...trace.SpanOption) (context.Context, trace.Span) { - args = append(args, trace.WithRecord()) ctx, span := tp.Tracer(trName).Start( ctx, name, @@ -1326,7 +1324,7 @@ func TestWithInstrumentationVersion(t *testing.T) { _, span := tp.Tracer( "WithInstrumentationVersion", trace.WithInstrumentationVersion("v0.1.0"), - ).Start(ctx, "span0", trace.WithRecord()) + ).Start(ctx, "span0") got, err := endSpan(te, span) if err != nil { t.Error(err.Error()) @@ -1357,7 +1355,6 @@ func TestSpanCapturesPanic(t *testing.T) { _, span := tp.Tracer("CatchPanic").Start( context.Background(), "span", - trace.WithRecord(), ) f := func() { diff --git a/trace/config.go b/trace/config.go index 33335e7ac34..ea30ee35f15 100644 --- a/trace/config.go +++ b/trace/config.go @@ -54,8 +54,6 @@ type SpanConfig struct { Timestamp time.Time // Links are the associations a Span has with other Spans. Links []Link - // Record is the recording state of a Span. - Record bool // NewRoot identifies a Span as the root Span for a new trace. This is // commonly used when an existing trace crosses trust boundaries and the // remote parent span context should be ignored for security. @@ -165,18 +163,6 @@ func WithLinks(links ...Link) SpanOption { return linksSpanOption(links) } -type recordSpanOption bool - -func (o recordSpanOption) ApplySpan(c *SpanConfig) { c.Record = bool(o) } -func (recordSpanOption) private() {} - -// WithRecord specifies that the span should be recorded. It is important to -// note that implementations may override this option, i.e. if the span is a -// child of an un-sampled trace. -func WithRecord() SpanOption { - return recordSpanOption(true) -} - type newRootSpanOption bool func (o newRootSpanOption) ApplySpan(c *SpanConfig) { c.NewRoot = bool(o) } diff --git a/trace/config_test.go b/trace/config_test.go index 82b6b35c37c..9375191c40a 100644 --- a/trace/config_test.go +++ b/trace/config_test.go @@ -115,24 +115,6 @@ func TestNewSpanConfig(t *testing.T) { Links: []Link{link1, link1, link2}, }, }, - { - []SpanOption{ - WithRecord(), - }, - &SpanConfig{ - Record: true, - }, - }, - { - []SpanOption{ - // Multiple calls should not change Record state. - WithRecord(), - WithRecord(), - }, - &SpanConfig{ - Record: true, - }, - }, { []SpanOption{ WithNewRoot(), @@ -175,7 +157,6 @@ func TestNewSpanConfig(t *testing.T) { WithAttributes(k1v1), WithTimestamp(timestamp0), WithLinks(link1, link2), - WithRecord(), WithNewRoot(), WithSpanKind(SpanKindConsumer), }, @@ -183,7 +164,6 @@ func TestNewSpanConfig(t *testing.T) { Attributes: []attribute.KeyValue{k1v1}, Timestamp: timestamp0, Links: []Link{link1, link2}, - Record: true, NewRoot: true, SpanKind: SpanKindConsumer, },