Revert "Add supporting features to enable distributed tracing (Azure#20301)"

This reverts commit 8d1e8af.

Author: chlowell

sdk/azcore/CHANGELOG.md

@@ -3,14 +3,6 @@
 ## 1.5.0-beta.2 (Unreleased)
 
 ### Features Added
-* Added supporting features to enable distributed tracing.
-  * Added func `runtime.StartSpan()` for use by SDKs to start spans.
-  * Added method `WithContext()` to `runtime.Request` to support shallow cloning with a new context.
-  * Added field `TracingNamespace` to `runtime.PipelineOptions`.
-  * Added field `Tracer` to `runtime.NewPollerOptions` and `runtime.NewPollerFromResumeTokenOptions` types.
-  * Added field `SpanFromContext` to `tracing.TracerOptions`.
-  * Added methods `Enabled()`, `SetAttributes()`, and `SpanFromContext()` to `tracing.Tracer`.
-  * Added supporting pipeline policies to include HTTP spans when creating clients.
 
 ### Breaking Changes
 

sdk/azcore/arm/runtime/pipeline.go

@@ -13,7 +13,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported"
 	azpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	azruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
 )
@@ -35,7 +34,7 @@ func NewPipeline(module, version string, cred azcore.TokenCredential, plOpts azr
 	})
 	perRetry := make([]azpolicy.Policy, len(plOpts.PerRetry), len(plOpts.PerRetry)+1)
 	copy(perRetry, plOpts.PerRetry)
-	plOpts.PerRetry = append(perRetry, authPolicy, exported.PolicyFunc(httpTraceNamespacePolicy))
+	plOpts.PerRetry = append(perRetry, authPolicy)
 	if !options.DisableRPRegistration {
 		regRPOpts := armpolicy.RegistrationOptions{ClientOptions: options.ClientOptions}
 		regPolicy, err := NewRPRegistrationPolicy(cred, &regRPOpts)

sdk/azcore/arm/runtime/policy_trace_namespace.go

@@ -99,9 +99,6 @@ func NewClient(clientName, moduleVersion string, plOpts runtime.PipelineOptions,
 	pl := runtime.NewPipeline(pkg, moduleVersion, plOpts, options)
 
 	tr := options.TracingProvider.NewTracer(clientName, moduleVersion)
-	if tr.Enabled() && plOpts.TracingNamespace != "" {
-		tr.SetAttributes(tracing.Attribute{Key: "az.namespace", Value: plOpts.TracingNamespace})
-	}
 	return &Client{pl: pl, tr: tr}, nil
 }
 

sdk/azcore/arm/runtime/policy_trace_namespace_test.go

@@ -7,17 +7,11 @@
 package azcore
 
 import (
-	"context"
-	"net/http"
 	"reflect"
 	"testing"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported"
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing"
-	"github.com/Azure/azure-sdk-for-go/sdk/internal/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -137,38 +131,3 @@ func TestNewClientError(t *testing.T) {
 	require.Error(t, err)
 	require.Nil(t, client)
 }
-
-func TestNewClientTracingEnabled(t *testing.T) {
-	srv, close := mock.NewServer()
-	defer close()
-
-	var attrString string
-	client, err := NewClient("package.Client", "v1.0.0", runtime.PipelineOptions{TracingNamespace: "Widget.Factory"}, &policy.ClientOptions{
-		TracingProvider: tracing.NewProvider(func(name, version string) tracing.Tracer {
-			return tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) {
-				require.NotNil(t, options)
-				for _, attr := range options.Attributes {
-					if attr.Key == "az.namespace" {
-						v, ok := attr.Value.(string)
-						require.True(t, ok)
-						attrString = attr.Key + ":" + v
-					}
-				}
-				return ctx, tracing.Span{}
-			}, nil)
-		}, nil),
-		Transport: srv,
-	})
-	require.NoError(t, err)
-	require.NotNil(t, client)
-	require.NotZero(t, client.Pipeline())
-	require.NotZero(t, client.Tracer())
-
-	const requestEndpoint = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/fakeResourceGroupo/providers/Microsoft.Storage/storageAccounts/fakeAccountName"
-	req, err := exported.NewRequest(context.WithValue(context.Background(), shared.CtxWithTracingTracer{}, client.Tracer()), http.MethodGet, srv.URL()+requestEndpoint)
-	require.NoError(t, err)
-	srv.AppendResponse()
-	_, err = client.Pipeline().Do(req)
-	require.NoError(t, err)
-	require.EqualValues(t, "az.namespace:Widget.Factory", attrString)
-}

sdk/azcore/core.go

@@ -170,14 +170,6 @@ func (req *Request) Clone(ctx context.Context) *Request {
 	return &r2
 }
 
-// WithContext returns a shallow copy of the request with its context changed to ctx.
-func (req *Request) WithContext(ctx context.Context) *Request {
-	r2 := new(Request)
-	*r2 = *req
-	r2.req = r2.req.WithContext(ctx)
-	return r2
-}
-
 // not exported but dependent on Request
 
 // PolicyFunc is a type that implements the Policy interface.

sdk/azcore/core_test.go

@@ -194,20 +194,3 @@ func TestNewRequestFail(t *testing.T) {
 		t.Fatal("unexpected request")
 	}
 }
-
-func TestRequestWithContext(t *testing.T) {
-	type ctxKey1 struct{}
-	type ctxKey2 struct{}
-
-	req1, err := NewRequest(context.WithValue(context.Background(), ctxKey1{}, 1), http.MethodPost, testURL)
-	require.NoError(t, err)
-	require.NotNil(t, req1.Raw().Context().Value(ctxKey1{}))
-
-	req2 := req1.WithContext(context.WithValue(context.Background(), ctxKey2{}, 1))
-	require.Nil(t, req2.Raw().Context().Value(ctxKey1{}))
-	require.NotNil(t, req2.Raw().Context().Value(ctxKey2{}))
-
-	// shallow copy, so changing req2 affects req1
-	req2.Raw().Header.Add("added-req2", "value")
-	require.EqualValues(t, "value", req1.Raw().Header.Get("added-req2"))
-}

sdk/azcore/internal/exported/request.go

@@ -23,7 +23,6 @@ const (
 	HeaderUserAgent              = "User-Agent"
 	HeaderWWWAuthenticate        = "WWW-Authenticate"
 	HeaderXMSClientRequestID     = "x-ms-client-request-id"
-	HeaderXMSRequestID           = "x-ms-request-id"
 )
 
 const BearerTokenPrefix = "Bearer "

sdk/azcore/internal/exported/request_test.go

@@ -28,9 +28,6 @@ type CtxWithRetryOptionsKey struct{}
 // CtxIncludeResponseKey is used as a context key for retrieving the raw response.
 type CtxIncludeResponseKey struct{}
 
-// CtxWithTracingTracer is used as a context key for adding/retrieving tracing.Tracer.
-type CtxWithTracingTracer struct{}
-
 // Delay waits for the duration to elapse or the context to be cancelled.
 func Delay(ctx context.Context, delay time.Duration) error {
 	select {

sdk/azcore/internal/shared/constants.go

@@ -29,8 +29,7 @@ type Request = exported.Request
 // ClientOptions contains optional settings for a client's pipeline.
 // All zero-value fields will be initialized with default values.
 type ClientOptions struct {
-	// APIVersion overrides the default version requested of the service.
-	// Set with caution as this package version has not been tested with arbitrary service versions.
+	// APIVersion overrides the default version requested of the service. Set with caution as this package version has not been tested with arbitrary service versions.
 	APIVersion string
 
 	// Cloud specifies a cloud for the client. The default is Azure Public Cloud.

sdk/azcore/internal/shared/shared.go

@@ -10,10 +10,6 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
-	"reflect"
-
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing"
 )
 
 // PagingHandler contains the required data for constructing a Pager.
@@ -24,16 +20,12 @@ type PagingHandler[T any] struct {
 
 	// Fetcher fetches the first and subsequent pages.
 	Fetcher func(context.Context, *T) (T, error)
-
-	// Tracer contains the Tracer from the client that's creating the Pager.
-	Tracer tracing.Tracer
 }
 
 // Pager provides operations for iterating over paged responses.
 type Pager[T any] struct {
 	current   *T
 	handler   PagingHandler[T]
-	tracer    tracing.Tracer
 	firstPage bool
 }
 
@@ -42,7 +34,6 @@ type Pager[T any] struct {
 func NewPager[T any](handler PagingHandler[T]) *Pager[T] {
 	return &Pager[T]{
 		handler:   handler,
-		tracer:    handler.Tracer,
 		firstPage: true,
 	}
 }
@@ -67,14 +58,10 @@ func (p *Pager[T]) NextPage(ctx context.Context) (T, error) {
 		} else if !p.handler.More(*p.current) {
 			return *new(T), errors.New("no more pages")
 		}
-		ctx, endSpan := StartSpan(ctx, fmt.Sprintf("%s.NextPage", shortenTypeName(reflect.TypeOf(*p).Name())), p.tracer, nil)
-		defer endSpan(err)
 		resp, err = p.handler.Fetcher(ctx, p.current)
 	} else {
 		// non-LRO case, first page
 		p.firstPage = false
-		ctx, endSpan := StartSpan(ctx, fmt.Sprintf("%s.NextPage", shortenTypeName(reflect.TypeOf(*p).Name())), p.tracer, nil)
-		defer endSpan(err)
 		resp, err = p.handler.Fetcher(ctx, nil)
 	}
 	if err != nil {

sdk/azcore/policy/policy.go

@@ -13,29 +13,9 @@ import (
 
 // PipelineOptions contains Pipeline options for SDK developers
 type PipelineOptions struct {
-	// AllowedHeaders is the slice of headers to log with their values intact.
-	// All headers not in the slice will have their values REDACTED.
-	// Applies to request and response headers.
-	AllowedHeaders []string
-
-	// AllowedQueryParameters is the slice of query parameters to log with their values intact.
-	// All query parameters not in the slice will have their values REDACTED.
-	AllowedQueryParameters []string
-
-	// APIVersion overrides the default version requested of the service.
-	// Set with caution as this package version has not been tested with arbitrary service versions.
-	APIVersion APIVersionOptions
-
-	// PerCall contains custom policies to inject into the pipeline.
-	// Each policy is executed once per request.
-	PerCall []policy.Policy
-
-	// PerRetry contains custom policies to inject into the pipeline.
-	// Each policy is executed once per request, and for each retry of that request.
-	PerRetry []policy.Policy
-
-	// TracingNamespace contains the value to use for the az.namespace span attribute.
-	TracingNamespace string
+	AllowedHeaders, AllowedQueryParameters []string
+	APIVersion                             APIVersionOptions
+	PerCall, PerRetry                      []policy.Policy
 }
 
 // Pipeline represents a primitive for sending HTTP requests and receiving responses.
@@ -76,10 +56,8 @@ func NewPipeline(module, version string, plOpts PipelineOptions, options *policy
 	policies = append(policies, NewRetryPolicy(&cp.Retry))
 	policies = append(policies, plOpts.PerRetry...)
 	policies = append(policies, cp.PerRetryPolicies...)
-	policies = append(policies, exported.PolicyFunc(httpHeaderPolicy))
-	policies = append(policies, newHTTPTracePolicy(cp.Logging.AllowedQueryParams))
 	policies = append(policies, NewLogPolicy(&cp.Logging))
-	policies = append(policies, exported.PolicyFunc(bodyDownloadPolicy))
+	policies = append(policies, exported.PolicyFunc(httpHeaderPolicy), exported.PolicyFunc(bodyDownloadPolicy))
 	transport := cp.Transport
 	if transport == nil {
 		transport = defaultHTTPClient