From 0359ac0801cf09e3158aa2307cd9ff850cee610e Mon Sep 17 00:00:00 2001 From: Kevin Glowacz Date: Thu, 31 Oct 2024 20:26:09 +0000 Subject: [PATCH 1/4] Add a WithCoreTracerName function to allow azcore consumers to indicate the azcore tracer should be named after the azcore module instead of the consumer's module --- sdk/azcore/core.go | 13 ++++++++++ sdk/azcore/core_test.go | 53 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/sdk/azcore/core.go b/sdk/azcore/core.go index 9d1c2f0c0537..b84c7ff8e57b 100644 --- a/sdk/azcore/core.go +++ b/sdk/azcore/core.go @@ -171,3 +171,16 @@ func (c *Client) WithClientName(clientName string) *Client { } return &Client{pl: c.pl, tr: tr, tp: c.tp, modVer: c.modVer, namespace: c.namespace} } + +// WithCoreName returns a shallow copy of the Client with its tracing client name changed to use azcore's module name and version. +// This is intended for library consumers that want to have their own named tracer for client activities that are +// Seperate from the Core HTTP tracing. +// Note that the values for module name and version will be preserved from the source Client for use in the +// HTTP UserAgent Header set by TelemetryPolicy +func (c *Client) WithCoreTracerName() *Client { + tr := c.tp.NewTracer(shared.Module, shared.Version) + if tr.Enabled() && c.namespace != "" { + tr.SetAttributes(tracing.Attribute{Key: shared.TracingNamespaceAttrName, Value: c.namespace}) + } + return &Client{pl: c.pl, tr: tr, tp: c.tp, modVer: c.modVer, namespace: c.namespace} +} diff --git a/sdk/azcore/core_test.go b/sdk/azcore/core_test.go index cef05c170c6f..afe69a94dd79 100644 --- a/sdk/azcore/core_test.go +++ b/sdk/azcore/core_test.go @@ -220,6 +220,59 @@ func TestClientWithClientName(t *testing.T) { require.EqualValues(t, "az.namespace:Widget.Factory", attrString) } +func TestClientWithCoreName(t *testing.T) { + srv, close := mock.NewServer() + defer close() + + var clientName string + var modVersion string + var attrString string + client, err := NewClient("module", "v1.0.0", runtime.PipelineOptions{ + Tracing: runtime.TracingOptions{ + Namespace: "Widget.Factory", + }, + }, &policy.ClientOptions{ + TracingProvider: tracing.NewProvider(func(name, version string) tracing.Tracer { + clientName = name + modVersion = version + 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 == shared.TracingNamespaceAttrName { + v, ok := attr.Value.(string) + require.True(t, ok) + attrString = attr.Key + ":" + v + } + } + return ctx, tracing.Span{} + }, nil) + }, nil), + Transport: srv, + }) + newClient := client.WithCoreTracerName() + require.NoError(t, err) + require.NotNil(t, newClient) + require.NotZero(t, newClient.Pipeline()) + require.NotZero(t, newClient.Tracer()) + require.EqualValues(t, client.Pipeline(), newClient.Pipeline()) + + // Tracer's name and version are set to azcore's module and version + require.EqualValues(t, shared.Module, clientName) + require.EqualValues(t, shared.Version, modVersion) + + 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.SetResponse() + _, err = newClient.Pipeline().Do(req) + require.NoError(t, err) + require.EqualValues(t, "az.namespace:Widget.Factory", attrString) + + // HTTP User agent is still set to calling module's name and version + ua := req.Raw().Header.Get(shared.HeaderUserAgent) + require.Contains(t, ua, "azsdk-go-module/v1.0.0") +} + func TestNewKeyCredential(t *testing.T) { require.NotNil(t, NewKeyCredential("foo")) } From 1aca7c6eb9094d9d727e383bfcc52fa442a9fc4b Mon Sep 17 00:00:00 2001 From: Kevin Glowacz Date: Thu, 7 Nov 2024 23:38:33 +0000 Subject: [PATCH 2/4] Update NewTracer to take a schemaURL --- sdk/azcore/arm/client.go | 3 ++- sdk/azcore/core.go | 7 ++++--- sdk/azcore/core_test.go | 6 +++--- sdk/azcore/go.mod | 1 + sdk/azcore/go.sum | 3 +++ sdk/azcore/tracing/tracing.go | 9 +++++---- sdk/azcore/tracing/tracing_test.go | 6 +++--- 7 files changed, 21 insertions(+), 14 deletions(-) diff --git a/sdk/azcore/arm/client.go b/sdk/azcore/arm/client.go index c373cc43fd50..913855bd25d4 100644 --- a/sdk/azcore/arm/client.go +++ b/sdk/azcore/arm/client.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared" "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + semconv "go.opentelemetry.io/otel/semconv/v1.18.0" ) // ClientOptions contains configuration settings for a client's pipeline. @@ -52,7 +53,7 @@ func NewClient(moduleName, moduleVersion string, cred azcore.TokenCredential, op return nil, err } - tr := options.TracingProvider.NewTracer(moduleName, moduleVersion) + tr := options.TracingProvider.NewTracer(moduleName, moduleVersion, semconv.SchemaURL) return &Client{ep: ep, pl: pl, tr: tr}, nil } diff --git a/sdk/azcore/core.go b/sdk/azcore/core.go index b84c7ff8e57b..f6841fc80f83 100644 --- a/sdk/azcore/core.go +++ b/sdk/azcore/core.go @@ -15,6 +15,7 @@ import ( "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" + semconv "go.opentelemetry.io/otel/semconv/v1.18.0" ) // AccessToken represents an Azure service bearer access token with expiry information. @@ -137,7 +138,7 @@ func NewClient(moduleName, moduleVersion string, plOpts runtime.PipelineOptions, pl := runtime.NewPipeline(moduleName, moduleVersion, plOpts, options) - tr := options.TracingProvider.NewTracer(moduleName, moduleVersion) + tr := options.TracingProvider.NewTracer(moduleName, moduleVersion, semconv.SchemaURL) if tr.Enabled() && plOpts.Tracing.Namespace != "" { tr.SetAttributes(tracing.Attribute{Key: shared.TracingNamespaceAttrName, Value: plOpts.Tracing.Namespace}) } @@ -165,7 +166,7 @@ func (c *Client) Tracer() tracing.Tracer { // Note that the values for module name and version will be preserved from the source Client. // - clientName - the fully qualified name of the client ("package.Client"); this is used by the tracing provider when creating spans func (c *Client) WithClientName(clientName string) *Client { - tr := c.tp.NewTracer(clientName, c.modVer) + tr := c.tp.NewTracer(clientName, c.modVer, semconv.SchemaURL) if tr.Enabled() && c.namespace != "" { tr.SetAttributes(tracing.Attribute{Key: shared.TracingNamespaceAttrName, Value: c.namespace}) } @@ -178,7 +179,7 @@ func (c *Client) WithClientName(clientName string) *Client { // Note that the values for module name and version will be preserved from the source Client for use in the // HTTP UserAgent Header set by TelemetryPolicy func (c *Client) WithCoreTracerName() *Client { - tr := c.tp.NewTracer(shared.Module, shared.Version) + tr := c.tp.NewTracer(shared.Module, shared.Version, semconv.SchemaURL) if tr.Enabled() && c.namespace != "" { tr.SetAttributes(tracing.Attribute{Key: shared.TracingNamespaceAttrName, Value: c.namespace}) } diff --git a/sdk/azcore/core_test.go b/sdk/azcore/core_test.go index afe69a94dd79..73768cbeea57 100644 --- a/sdk/azcore/core_test.go +++ b/sdk/azcore/core_test.go @@ -138,7 +138,7 @@ func TestNewClientTracingEnabled(t *testing.T) { Namespace: "Widget.Factory", }, }, &policy.ClientOptions{ - TracingProvider: tracing.NewProvider(func(name, version string) tracing.Tracer { + TracingProvider: tracing.NewProvider(func(name, version, schemaURL 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 { @@ -179,7 +179,7 @@ func TestClientWithClientName(t *testing.T) { Namespace: "Widget.Factory", }, }, &policy.ClientOptions{ - TracingProvider: tracing.NewProvider(func(name, version string) tracing.Tracer { + TracingProvider: tracing.NewProvider(func(name, version, schemaURL string) tracing.Tracer { clientName = name modVersion = version return tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { @@ -232,7 +232,7 @@ func TestClientWithCoreName(t *testing.T) { Namespace: "Widget.Factory", }, }, &policy.ClientOptions{ - TracingProvider: tracing.NewProvider(func(name, version string) tracing.Tracer { + TracingProvider: tracing.NewProvider(func(name, version, schemaURL string) tracing.Tracer { clientName = name modVersion = version return tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { diff --git a/sdk/azcore/go.mod b/sdk/azcore/go.mod index 43d6a2b819c8..4bb69b611f9d 100644 --- a/sdk/azcore/go.mod +++ b/sdk/azcore/go.mod @@ -5,6 +5,7 @@ go 1.18 require ( github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 github.com/stretchr/testify v1.9.0 + go.opentelemetry.io/otel v1.14.0 golang.org/x/net v0.29.0 ) diff --git a/sdk/azcore/go.sum b/sdk/azcore/go.sum index 72af30bf8a1d..a8a7dc3cdf4b 100644 --- a/sdk/azcore/go.sum +++ b/sdk/azcore/go.sum @@ -3,6 +3,7 @@ github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkY github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= @@ -11,6 +12,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +go.opentelemetry.io/otel v1.14.0 h1:/79Huy8wbf5DnIPhemGB+zEPVwnN6fuQybr/SRXa6hM= +go.opentelemetry.io/otel v1.14.0/go.mod h1:o4buv+dJzx8rohcUeRmWUZhqupFvzWis188WlggnNeU= golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo= golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0= golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= diff --git a/sdk/azcore/tracing/tracing.go b/sdk/azcore/tracing/tracing.go index 1ade7c560ff1..1e14606cd6cf 100644 --- a/sdk/azcore/tracing/tracing.go +++ b/sdk/azcore/tracing/tracing.go @@ -19,7 +19,7 @@ type ProviderOptions struct { // NewProvider creates a new Provider with the specified values. // - newTracerFn is the underlying implementation for creating Tracer instances // - options contains optional values; pass nil to accept the default value -func NewProvider(newTracerFn func(name, version string) Tracer, options *ProviderOptions) Provider { +func NewProvider(newTracerFn func(name, version, schemaURL string) Tracer, options *ProviderOptions) Provider { return Provider{ newTracerFn: newTracerFn, } @@ -28,15 +28,16 @@ func NewProvider(newTracerFn func(name, version string) Tracer, options *Provide // Provider is the factory that creates Tracer instances. // It defaults to a no-op provider. type Provider struct { - newTracerFn func(name, version string) Tracer + newTracerFn func(name, version, schemaURL string) Tracer } // NewTracer creates a new Tracer for the specified module name and version. // - module - the fully qualified name of the module // - version - the version of the module -func (p Provider) NewTracer(module, version string) (tracer Tracer) { +// - schemaURL - the url for the schema used for the tracing span names +func (p Provider) NewTracer(module, version, schemaURL string) (tracer Tracer) { if p.newTracerFn != nil { - tracer = p.newTracerFn(module, version) + tracer = p.newTracerFn(module, version, schemaURL) } return } diff --git a/sdk/azcore/tracing/tracing_test.go b/sdk/azcore/tracing/tracing_test.go index 46163c1d2589..37fe5bbeae51 100644 --- a/sdk/azcore/tracing/tracing_test.go +++ b/sdk/azcore/tracing/tracing_test.go @@ -15,7 +15,7 @@ import ( func TestProviderZeroValues(t *testing.T) { pr := Provider{} - tr := pr.NewTracer("name", "version") + tr := pr.NewTracer("name", "version", "") require.Zero(t, tr) require.False(t, tr.Enabled()) tr.SetAttributes() @@ -37,7 +37,7 @@ func TestProvider(t *testing.T) { var setStatusCalled bool var spanFromContextCalled bool - pr := NewProvider(func(name, version string) Tracer { + pr := NewProvider(func(name, version, schemaURL string) Tracer { return NewTracer(func(context.Context, string, *SpanOptions) (context.Context, Span) { return nil, NewSpan(SpanImpl{ AddEvent: func(string, ...Attribute) { addEventCalled = true }, @@ -52,7 +52,7 @@ func TestProvider(t *testing.T) { }, }) }, nil) - tr := pr.NewTracer("name", "version") + tr := pr.NewTracer("name", "version", "") require.NotZero(t, tr) require.True(t, tr.Enabled()) sp := tr.SpanFromContext(context.Background()) From 0dd20733455bd258ca9cd79f71f753119a4efbd3 Mon Sep 17 00:00:00 2001 From: Kevin Glowacz Date: Thu, 7 Nov 2024 23:40:58 +0000 Subject: [PATCH 3/4] Use the semconv key consts where available --- sdk/azcore/runtime/policy_http_trace.go | 18 ++++++------------ sdk/azcore/runtime/policy_http_trace_test.go | 13 +++++++------ 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/sdk/azcore/runtime/policy_http_trace.go b/sdk/azcore/runtime/policy_http_trace.go index f375195c4b51..e29cc86bcb23 100644 --- a/sdk/azcore/runtime/policy_http_trace.go +++ b/sdk/azcore/runtime/policy_http_trace.go @@ -18,18 +18,12 @@ import ( "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/tracing" + semconv "go.opentelemetry.io/otel/semconv/v1.18.0" ) const ( - attrHTTPMethod = "http.method" - attrHTTPURL = "http.url" - attrHTTPUserAgent = "http.user_agent" - attrHTTPStatusCode = "http.status_code" - attrAZClientReqID = "az.client_request_id" attrAZServiceReqID = "az.service_request_id" - - attrNetPeerName = "net.peer.name" ) // newHTTPTracePolicy creates a new instance of the httpTracePolicy. @@ -48,13 +42,13 @@ func (h *httpTracePolicy) Do(req *policy.Request) (resp *http.Response, err erro rawTracer := req.Raw().Context().Value(shared.CtxWithTracingTracer{}) if tracer, ok := rawTracer.(tracing.Tracer); ok && tracer.Enabled() { attributes := []tracing.Attribute{ - {Key: attrHTTPMethod, Value: req.Raw().Method}, - {Key: attrHTTPURL, Value: getSanitizedURL(*req.Raw().URL, h.allowedQP)}, - {Key: attrNetPeerName, Value: req.Raw().URL.Host}, + {Key: string(semconv.HTTPMethodKey), Value: req.Raw().Method}, + {Key: string(semconv.HTTPURLKey), Value: getSanitizedURL(*req.Raw().URL, h.allowedQP)}, + {Key: string(semconv.NetPeerNameKey), Value: req.Raw().URL.Host}, } if ua := req.Raw().Header.Get(shared.HeaderUserAgent); ua != "" { - attributes = append(attributes, tracing.Attribute{Key: attrHTTPUserAgent, Value: ua}) + attributes = append(attributes, tracing.Attribute{Key: string(semconv.HTTPUserAgentKey), Value: ua}) } if reqID := req.Raw().Header.Get(shared.HeaderXMSClientRequestID); reqID != "" { attributes = append(attributes, tracing.Attribute{Key: attrAZClientReqID, Value: reqID}) @@ -68,7 +62,7 @@ func (h *httpTracePolicy) Do(req *policy.Request) (resp *http.Response, err erro defer func() { if resp != nil { - span.SetAttributes(tracing.Attribute{Key: attrHTTPStatusCode, Value: resp.StatusCode}) + span.SetAttributes(tracing.Attribute{Key: string(semconv.HTTPStatusCodeKey), Value: resp.StatusCode}) if resp.StatusCode > 399 { span.SetStatus(tracing.SpanStatusError, resp.Status) } diff --git a/sdk/azcore/runtime/policy_http_trace_test.go b/sdk/azcore/runtime/policy_http_trace_test.go index 43581ec54d29..f8dd1174a8d0 100644 --- a/sdk/azcore/runtime/policy_http_trace_test.go +++ b/sdk/azcore/runtime/policy_http_trace_test.go @@ -21,6 +21,7 @@ import ( "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" + semconv "go.opentelemetry.io/otel/semconv/v1.18.0" ) func TestHTTPTracePolicy(t *testing.T) { @@ -75,12 +76,12 @@ func TestHTTPTracePolicy(t *testing.T) { require.EqualValues(t, "HTTP GET", fullSpanName) require.EqualValues(t, tracing.SpanKindClient, spanKind) require.Len(t, spanAttrs, 7) - require.Contains(t, spanAttrs, tracing.Attribute{Key: attrHTTPMethod, Value: http.MethodGet}) - require.Contains(t, spanAttrs, tracing.Attribute{Key: attrHTTPURL, Value: srv.URL() + "?foo=REDACTED&visibleqp=bar"}) - require.Contains(t, spanAttrs, tracing.Attribute{Key: attrNetPeerName, Value: srv.URL()[7:]}) // strip off the http:// - require.Contains(t, spanAttrs, tracing.Attribute{Key: attrHTTPUserAgent, Value: "my-user-agent"}) + require.Contains(t, spanAttrs, tracing.Attribute{Key: string(semconv.HTTPMethodKey), Value: http.MethodGet}) + require.Contains(t, spanAttrs, tracing.Attribute{Key: string(semconv.HTTPURLKey), Value: srv.URL() + "?foo=REDACTED&visibleqp=bar"}) + require.Contains(t, spanAttrs, tracing.Attribute{Key: string(semconv.NetPeerNameKey), Value: srv.URL()[7:]}) // strip off the http:// + require.Contains(t, spanAttrs, tracing.Attribute{Key: string(semconv.HTTPUserAgentKey), Value: "my-user-agent"}) require.Contains(t, spanAttrs, tracing.Attribute{Key: attrAZClientReqID, Value: "my-client-request"}) - require.Contains(t, spanAttrs, tracing.Attribute{Key: attrHTTPStatusCode, Value: http.StatusOK}) + require.Contains(t, spanAttrs, tracing.Attribute{Key: string(semconv.HTTPStatusCodeKey), Value: http.StatusOK}) require.Contains(t, spanAttrs, tracing.Attribute{Key: attrAZServiceReqID, Value: "request-id"}) // HTTP bad request @@ -91,7 +92,7 @@ func TestHTTPTracePolicy(t *testing.T) { require.NoError(t, err) require.EqualValues(t, tracing.SpanStatusError, spanStatus) require.EqualValues(t, "400 Bad Request", spanStatusStr) - require.Contains(t, spanAttrs, tracing.Attribute{Key: attrHTTPStatusCode, Value: http.StatusBadRequest}) + require.Contains(t, spanAttrs, tracing.Attribute{Key: string(semconv.HTTPStatusCodeKey), Value: http.StatusBadRequest}) // HTTP error req, err = exported.NewRequest(context.WithValue(context.Background(), shared.CtxWithTracingTracer{}, tr), http.MethodGet, srv.URL()) From 09654f8dcfc6cbc5c1244d20cd244999192f3b4c Mon Sep 17 00:00:00 2001 From: Kevin Glowacz Date: Fri, 8 Nov 2024 00:17:08 +0000 Subject: [PATCH 4/4] Changelog --- sdk/azcore/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 6cc4042033a8..17fc6fdaac39 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/sdk/azcore/CHANGELOG.md @@ -4,8 +4,12 @@ ### Features Added +* `Client` has a new method called `WithCoreTracerName` that can be used by consumers that wish to have the azcore http tracer's module name and version set to azcore's values instead of the consumer modules values + ### Breaking Changes +* `tracing.NewProvider`'s `newTracerFn` and `Provider.NewTracer` now take a schemaURL for their 3rd argument. + ### Bugs Fixed ### Other Changes