Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azcore separate tracer #23715

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions sdk/azcore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion sdk/azcore/arm/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down
18 changes: 16 additions & 2 deletions sdk/azcore/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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})
}
Expand Down Expand Up @@ -165,7 +166,20 @@ 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})
}
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, semconv.SchemaURL)
if tr.Enabled() && c.namespace != "" {
tr.SetAttributes(tracing.Attribute{Key: shared.TracingNamespaceAttrName, Value: c.namespace})
}
Expand Down
57 changes: 55 additions & 2 deletions sdk/azcore/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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, schemaURL 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"))
}
Expand Down
1 change: 1 addition & 0 deletions sdk/azcore/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
3 changes: 3 additions & 0 deletions sdk/azcore/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand All @@ -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=
Expand Down
18 changes: 6 additions & 12 deletions sdk/azcore/runtime/policy_http_trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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})
Expand All @@ -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)
}
Expand Down
13 changes: 7 additions & 6 deletions sdk/azcore/runtime/policy_http_trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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())
Expand Down
9 changes: 5 additions & 4 deletions sdk/azcore/tracing/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions sdk/azcore/tracing/tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 },
Expand All @@ -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())
Expand Down
Loading