diff --git a/eng/pipelines/templates/jobs/archetype-sdk-client.yml b/eng/pipelines/templates/jobs/archetype-sdk-client.yml index 7a22316db7a4..31b2ef371c70 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-client.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-client.yml @@ -71,7 +71,7 @@ parameters: - name: IncludeRelease type: boolean default: false - - name: ExcludeGoNMinus3 + - name: ExcludeGoNMinus2 type: boolean default: false @@ -85,12 +85,12 @@ stages: - template: /eng/pipelines/templates/variables/globals.yml strategy: matrix: - ${{ if eq(parameters.ExcludeGoNMinus3, false) }}: + ${{ if eq(parameters.ExcludeGoNMinus2, false) }}: Linux_Go118: pool.name: azsdk-pool-mms-ubuntu-2004-general image.name: MMSUbuntu20.04 go.version: '1.18.10' - ${{ if eq(parameters.ExcludeGoNMinus3, false) }}: + ${{ if eq(parameters.ExcludeGoNMinus2, false) }}: Windows_Go118: pool.name: azsdk-pool-mms-win-2022-general image.name: MMS2022 diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 648465b9f193..81b132e38589 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -5,6 +5,11 @@ ### Features Added ### Breaking Changes +> These changes affect only code written against beta version v1.7.0-beta.1 +* Method `SpanFromContext()` on type `tracing.Tracer` had the `bool` return value removed. + * This includes the field `SpanFromContext` in supporting type `tracing.TracerOptions`. +* Method `AddError()` has been removed from type `tracing.Span`. +* Method `Span.End()` now requires an argument of type `*tracing.SpanEndOptions`. ### Bugs Fixed diff --git a/sdk/azcore/arm/runtime/policy_trace_namespace.go b/sdk/azcore/arm/runtime/policy_trace_namespace.go index 76aefe8550dd..b46a0107f967 100644 --- a/sdk/azcore/arm/runtime/policy_trace_namespace.go +++ b/sdk/azcore/arm/runtime/policy_trace_namespace.go @@ -22,9 +22,8 @@ func httpTraceNamespacePolicy(req *policy.Request) (resp *http.Response, err err rt, err := resource.ParseResourceType(req.Raw().URL.Path) if err == nil { // add the namespace attribute to the current span - if span, ok := tracer.SpanFromContext(req.Raw().Context()); ok { - span.SetAttributes(tracing.Attribute{Key: "az.namespace", Value: rt.Namespace}) - } + span := tracer.SpanFromContext(req.Raw().Context()) + span.SetAttributes(tracing.Attribute{Key: "az.namespace", Value: rt.Namespace}) } } return req.Next() diff --git a/sdk/azcore/arm/runtime/policy_trace_namespace_test.go b/sdk/azcore/arm/runtime/policy_trace_namespace_test.go index 4ac7484823f8..3fcc99a0c291 100644 --- a/sdk/azcore/arm/runtime/policy_trace_namespace_test.go +++ b/sdk/azcore/arm/runtime/policy_trace_namespace_test.go @@ -53,7 +53,7 @@ func TestHTTPTraceNamespacePolicy(t *testing.T) { tr = tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { return ctx, tracing.Span{} }, &tracing.TracerOptions{ - SpanFromContext: func(ctx context.Context) (tracing.Span, bool) { + SpanFromContext: func(ctx context.Context) tracing.Span { spanImpl := tracing.SpanImpl{ SetAttributes: func(a ...tracing.Attribute) { require.Len(t, a, 1) @@ -62,7 +62,7 @@ func TestHTTPTraceNamespacePolicy(t *testing.T) { attrString = a[0].Key + ":" + v }, } - return tracing.NewSpan(spanImpl), true + return tracing.NewSpan(spanImpl) }, }) req, err = exported.NewRequest(context.WithValue(context.Background(), shared.CtxWithTracingTracer{}, tr), http.MethodGet, srv.URL()) @@ -76,7 +76,7 @@ func TestHTTPTraceNamespacePolicy(t *testing.T) { tr = tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { return ctx, tracing.Span{} }, &tracing.TracerOptions{ - SpanFromContext: func(ctx context.Context) (tracing.Span, bool) { + SpanFromContext: func(ctx context.Context) tracing.Span { spanImpl := tracing.SpanImpl{ SetAttributes: func(a ...tracing.Attribute) { require.Len(t, a, 1) @@ -85,7 +85,7 @@ func TestHTTPTraceNamespacePolicy(t *testing.T) { attrString = a[0].Key + ":" + v }, } - return tracing.NewSpan(spanImpl), true + return tracing.NewSpan(spanImpl) }, }) req, err = exported.NewRequest(context.WithValue(context.Background(), shared.CtxWithTracingTracer{}, tr), http.MethodGet, srv.URL()+requestEndpoint) diff --git a/sdk/azcore/runtime/policy_http_trace.go b/sdk/azcore/runtime/policy_http_trace.go index 55eb7fea07cc..498434d911cc 100644 --- a/sdk/azcore/runtime/policy_http_trace.go +++ b/sdk/azcore/runtime/policy_http_trace.go @@ -78,7 +78,7 @@ func (h *httpTracePolicy) Do(req *policy.Request) (resp *http.Response, err erro // so instead of attempting to sanitize the output, we simply output the error type. span.SetStatus(tracing.SpanStatusError, fmt.Sprintf("%T", err)) } - span.End() + span.End(nil) }() req = req.WithContext(ctx) @@ -112,6 +112,6 @@ func StartSpan(ctx context.Context, name string, tracer tracing.Tracer, options errType := strings.Replace(fmt.Sprintf("%T", err), "*exported.", "*azcore.", 1) span.SetStatus(tracing.SpanStatusError, fmt.Sprintf("%s:\n%s", errType, err.Error())) } - span.End() + span.End(nil) } } diff --git a/sdk/azcore/tracing/tracing.go b/sdk/azcore/tracing/tracing.go index f5157005f555..62df070d0e12 100644 --- a/sdk/azcore/tracing/tracing.go +++ b/sdk/azcore/tracing/tracing.go @@ -46,7 +46,7 @@ func (p Provider) NewTracer(name, version string) (tracer Tracer) { // TracerOptions contains the optional values when creating a Tracer. type TracerOptions struct { // SpanFromContext contains the implementation for the Tracer.SpanFromContext method. - SpanFromContext func(context.Context) (Span, bool) + SpanFromContext func(context.Context) Span } // NewTracer creates a Tracer with the specified values. @@ -66,7 +66,7 @@ func NewTracer(newSpanFn func(ctx context.Context, spanName string, options *Spa type Tracer struct { attrs []Attribute newSpanFn func(ctx context.Context, spanName string, options *SpanOptions) (context.Context, Span) - spanFromContextFn func(ctx context.Context) (Span, bool) + spanFromContextFn func(ctx context.Context) Span } // Start creates a new span and a context.Context that contains it. @@ -99,11 +99,11 @@ func (t Tracer) Enabled() bool { // SpanFromContext returns the Span associated with the current context. // If the provided context has no Span, false is returned. -func (t Tracer) SpanFromContext(ctx context.Context) (Span, bool) { +func (t Tracer) SpanFromContext(ctx context.Context) Span { if t.spanFromContextFn != nil { return t.spanFromContextFn(ctx) } - return Span{}, false + return Span{} } // SpanOptions contains optional settings for creating a span. @@ -130,9 +130,6 @@ type SpanImpl struct { // AddEvent contains the implementation for the Span.AddEvent method. AddEvent func(string, ...Attribute) - // AddError contains the implementation for the Span.AddError method. - AddError func(err error) - // SetStatus contains the implementation for the Span.SetStatus method. SetStatus func(SpanStatus, string) } @@ -152,7 +149,7 @@ type Span struct { // End terminates the span and MUST be called before the span leaves scope. // Any further updates to the span will be ignored after End is called. -func (s Span) End() { +func (s Span) End(opts *SpanEndOptions) { if s.impl.End != nil { s.impl.End() } @@ -173,13 +170,6 @@ func (s Span) AddEvent(name string, attrs ...Attribute) { } } -// AddError adds the specified error event to the span. -func (s Span) AddError(err error) { - if s.impl.AddError != nil { - s.impl.AddError(err) - } -} - // SetStatus sets the status on the span along with a description. func (s Span) SetStatus(code SpanStatus, desc string) { if s.impl.SetStatus != nil { @@ -187,6 +177,11 @@ func (s Span) SetStatus(code SpanStatus, desc string) { } } +// SpanEndOptions contains the optional values for the Span.End() method. +type SpanEndOptions struct { + // for future expansion +} + ///////////////////////////////////////////////////////////////////////////////////////////////////////////// // Attribute is a key-value pair. diff --git a/sdk/azcore/tracing/tracing_test.go b/sdk/azcore/tracing/tracing_test.go index 5ca8b3f267de..6318c257703d 100644 --- a/sdk/azcore/tracing/tracing_test.go +++ b/sdk/azcore/tracing/tracing_test.go @@ -22,18 +22,15 @@ func TestProviderZeroValues(t *testing.T) { ctx, sp := tr.Start(context.Background(), "spanName", nil) require.Equal(t, context.Background(), ctx) require.Zero(t, sp) - sp.AddError(nil) sp.AddEvent("event") - sp.End() + sp.End(nil) sp.SetAttributes(Attribute{}) sp.SetStatus(SpanStatusError, "boom") - sp, ok := tr.SpanFromContext(ctx) - require.False(t, ok) - require.Zero(t, sp) + spCtx := tr.SpanFromContext(ctx) + require.Zero(t, spCtx) } func TestProvider(t *testing.T) { - var addErrorCalled bool var addEventCalled bool var endCalled bool var setAttributesCalled bool @@ -43,24 +40,22 @@ func TestProvider(t *testing.T) { pr := NewProvider(func(name, version string) Tracer { return NewTracer(func(context.Context, string, *SpanOptions) (context.Context, Span) { return nil, NewSpan(SpanImpl{ - AddError: func(error) { addErrorCalled = true }, AddEvent: func(string, ...Attribute) { addEventCalled = true }, End: func() { endCalled = true }, SetAttributes: func(...Attribute) { setAttributesCalled = true }, SetStatus: func(SpanStatus, string) { setStatusCalled = true }, }) }, &TracerOptions{ - SpanFromContext: func(context.Context) (Span, bool) { + SpanFromContext: func(context.Context) Span { spanFromContextCalled = true - return Span{}, true + return Span{} }, }) }, nil) tr := pr.NewTracer("name", "version") require.NotZero(t, tr) require.True(t, tr.Enabled()) - sp, ok := tr.SpanFromContext(context.Background()) - require.True(t, ok) + sp := tr.SpanFromContext(context.Background()) require.Zero(t, sp) tr.SetAttributes(Attribute{Key: "some", Value: "attribute"}) require.Len(t, tr.attrs, 1) @@ -71,12 +66,10 @@ func TestProvider(t *testing.T) { require.NotEqual(t, context.Background(), ctx) require.NotZero(t, sp) - sp.AddError(nil) sp.AddEvent("event") - sp.End() + sp.End(nil) sp.SetAttributes() sp.SetStatus(SpanStatusError, "desc") - require.True(t, addErrorCalled) require.True(t, addEventCalled) require.True(t, endCalled) require.True(t, setAttributesCalled) diff --git a/sdk/tracing/azotel/ci.yml b/sdk/tracing/azotel/ci.yml index ee1b1445c15c..6312f0eec5c0 100644 --- a/sdk/tracing/azotel/ci.yml +++ b/sdk/tracing/azotel/ci.yml @@ -25,4 +25,4 @@ stages: - template: /eng/pipelines/templates/jobs/archetype-sdk-client.yml parameters: ServiceDirectory: tracing/azotel - ExcludeGoNMinus3: true + ExcludeGoNMinus2: true diff --git a/sdk/tracing/azotel/go.mod b/sdk/tracing/azotel/go.mod index 230115c7835c..4dc478a8e644 100644 --- a/sdk/tracing/azotel/go.mod +++ b/sdk/tracing/azotel/go.mod @@ -31,3 +31,5 @@ require ( golang.org/x/text v0.8.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace github.com/Azure/azure-sdk-for-go/sdk/azcore => ../../azcore diff --git a/sdk/tracing/azotel/otel.go b/sdk/tracing/azotel/otel.go index bbf62417fc96..9038d1743153 100644 --- a/sdk/tracing/azotel/otel.go +++ b/sdk/tracing/azotel/otel.go @@ -40,8 +40,8 @@ func NewTracingProvider(tracerProvider *otelsdk.TracerProvider, opts *TracingPro ctx, span := tracer.Start(ctx, spanName, trace.WithSpanKind(convertSpanKind(kind)), trace.WithAttributes(attrs...)) return ctx, convertSpan(span) }, &tracing.TracerOptions{ - SpanFromContext: func(ctx context.Context) (tracing.Span, bool) { - return convertSpan(trace.SpanFromContext(ctx)), true + SpanFromContext: func(ctx context.Context) tracing.Span { + return convertSpan(trace.SpanFromContext(ctx)) }, }) @@ -59,9 +59,6 @@ func convertSpan(traceSpan trace.Span) tracing.Span { AddEvent: func(name string, attrs ...tracing.Attribute) { traceSpan.AddEvent(name, trace.WithAttributes(convertAttributes(attrs)...)) }, - AddError: func(err error) { - traceSpan.RecordError(err) - }, SetStatus: func(code tracing.SpanStatus, desc string) { traceSpan.SetStatus(convertStatus(code), desc) }, diff --git a/sdk/tracing/azotel/otel_test.go b/sdk/tracing/azotel/otel_test.go index 7eda078316ac..8a6fa6960788 100644 --- a/sdk/tracing/azotel/otel_test.go +++ b/sdk/tracing/azotel/otel_test.go @@ -8,7 +8,6 @@ package azotel import ( "context" - "io" "net/http" "testing" @@ -37,8 +36,7 @@ func TestNewTracingProvider(t *testing.T) { require.NoError(t, err) // returns a no-op span as there is no span yet - emptySpan, ok := client.Tracer().SpanFromContext(context.Background()) - assert.True(t, ok) + emptySpan := client.Tracer().SpanFromContext(context.Background()) emptySpan.AddEvent("noop_event") ctx, endSpan := azruntime.StartSpan(context.Background(), "test_span", client.Tracer(), nil) @@ -46,8 +44,7 @@ func TestNewTracingProvider(t *testing.T) { req, err := azruntime.NewRequest(ctx, http.MethodGet, "https://www.microsoft.com/") require.NoError(t, err) - startedSpan, ok := client.Tracer().SpanFromContext(req.Raw().Context()) - assert.True(t, ok) + startedSpan := client.Tracer().SpanFromContext(req.Raw().Context()) startedSpan.AddEvent("post_event") _, err = client.Pipeline().Do(req) @@ -65,9 +62,6 @@ func TestConvertSpan(t *testing.T) { ts := testSpan{t: t} span := convertSpan(&ts) - span.AddError(io.EOF) - assert.Equal(t, io.EOF, ts.recordedError) - const eventName = "event" eventAttr := tracing.Attribute{ Key: "key", @@ -77,7 +71,7 @@ func TestConvertSpan(t *testing.T) { assert.EqualValues(t, eventName, ts.eventName) require.Len(t, ts.eventOptions, 1) - span.End() + span.End(nil) assert.True(t, ts.endCalled) attr := tracing.Attribute{ @@ -206,14 +200,13 @@ func (c *testExporter) Shutdown(ctx context.Context) error { } type testSpan struct { - t *testing.T - attributes []attribute.KeyValue - eventName string - eventOptions []trace.EventOption - endCalled bool - recordedError error - statusCode codes.Code - statusDesc string + t *testing.T + attributes []attribute.KeyValue + eventName string + eventOptions []trace.EventOption + endCalled bool + statusCode codes.Code + statusDesc string } func (ts *testSpan) End(options ...trace.SpanEndOption) { @@ -231,7 +224,7 @@ func (ts *testSpan) IsRecording() bool { } func (ts *testSpan) RecordError(err error, options ...trace.EventOption) { - ts.recordedError = err + ts.t.Fatal("RecordError not required") } func (ts *testSpan) SpanContext() trace.SpanContext {