From feffdf85f0ed46190ef0091168853803d65e0a48 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Tue, 10 Dec 2024 17:19:04 -0800 Subject: [PATCH 01/15] add internal tracing wrapper and fake tracer for UT --- sdk/messaging/azservicebus/go.mod | 10 +- sdk/messaging/azservicebus/go.sum | 20 ++-- .../azservicebus/internal/constants.go | 2 + .../internal/tracing/constants.go | 49 +++++++++ .../internal/tracing/fake_tracing.go | 99 +++++++++++++++++++ .../azservicebus/internal/tracing/tracing.go | 21 ++++ 6 files changed, 186 insertions(+), 15 deletions(-) create mode 100644 sdk/messaging/azservicebus/internal/tracing/constants.go create mode 100644 sdk/messaging/azservicebus/internal/tracing/fake_tracing.go create mode 100644 sdk/messaging/azservicebus/internal/tracing/tracing.go diff --git a/sdk/messaging/azservicebus/go.mod b/sdk/messaging/azservicebus/go.mod index c0fe09a96f14..1ddb84768278 100644 --- a/sdk/messaging/azservicebus/go.mod +++ b/sdk/messaging/azservicebus/go.mod @@ -5,7 +5,7 @@ go 1.18 retract v1.1.2 // Breaks customers in situations where close is slow/infinite. require ( - github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 github.com/Azure/go-amqp v1.1.0 @@ -34,9 +34,9 @@ require ( github.com/kylelemons/godebug v1.1.0 // indirect github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/crypto v0.25.0 // indirect - golang.org/x/net v0.27.0 // indirect - golang.org/x/sys v0.22.0 // indirect - golang.org/x/text v0.16.0 // indirect + golang.org/x/crypto v0.27.0 // indirect + golang.org/x/net v0.29.0 // indirect + golang.org/x/sys v0.25.0 // indirect + golang.org/x/text v0.18.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/sdk/messaging/azservicebus/go.sum b/sdk/messaging/azservicebus/go.sum index bb1323045dae..6e3fa14a700a 100644 --- a/sdk/messaging/azservicebus/go.sum +++ b/sdk/messaging/azservicebus/go.sum @@ -1,5 +1,5 @@ -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 h1:GJHeeA2N7xrG3q30L2UXDyuWRzDM900/65j70wcM4Ww= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0/go.mod h1:l38EPgmsp71HHLq9j7De57JcKOWPyhrsW1Awm1JS6K0= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BLisIzM9dG1R50zUk9C/M= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0/go.mod h1:YL1xnZ6QejvQHWJrX/AvhFl4WW4rqHVoKspWNVwFk0M= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 h1:tfLQ34V6F7tVSwoTf/4lH5sE0o6eCJuNDTmH09nDpbc= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0/go.mod h1:9kIvujWAA58nmPmWB1m23fyWic1kYZMxD9CxaWn4Qpg= github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY= @@ -35,14 +35,14 @@ github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= -golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= +golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A= +golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= -golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys= -golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= +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/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -51,13 +51,13 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= -golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34= +golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= -golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= +golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= diff --git a/sdk/messaging/azservicebus/internal/constants.go b/sdk/messaging/azservicebus/internal/constants.go index 8d90db9d55ab..7f55b1b7ddd6 100644 --- a/sdk/messaging/azservicebus/internal/constants.go +++ b/sdk/messaging/azservicebus/internal/constants.go @@ -3,5 +3,7 @@ package internal +const ModuleName = "azservicebus" + // Version is the semantic version number const Version = "v1.7.4" diff --git a/sdk/messaging/azservicebus/internal/tracing/constants.go b/sdk/messaging/azservicebus/internal/tracing/constants.go new file mode 100644 index 000000000000..3e55044a61cf --- /dev/null +++ b/sdk/messaging/azservicebus/internal/tracing/constants.go @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +// OTel-specific messaging attributes +const ( + MessagingSystem = "messaging.system" + OperationName = "messaging.operation.name" + BatchMessageCount = "messaging.batch.message_count" + DestinationName = "messaging.destination.name" + SubscriptionName = "messaging.destination.subscription.name" + OperationType = "messaging.operation.type" + DispositionStatus = "messaging.servicebus.disposition_status" + DeliveryCount = "messaging.servicebus.message.delivery_count" + ConversationID = "messaging.message.conversation_id" + MessageID = "messaging.message.id" + EnqueuedTime = "messaging.servicebus.message.enqueued_time" + + ServerAddress = "server.address" + ServerPort = "server.port" +) + +type MessagingOperationType string + +const ( + SendOperationType MessagingOperationType = "send" + ReceiveOperationType MessagingOperationType = "receive" + SettleOperationType MessagingOperationType = "settle" +) + +type MessagingOperationName string + +const ( + SendOperationName MessagingOperationName = "send" + ScheduleOperationName MessagingOperationName = "schedule" + CancelScheduledOperationName MessagingOperationName = "cancel_scheduled" + + ReceiveOperationName MessagingOperationName = "receive" + PeekOperationName MessagingOperationName = "peek" + ReceiveDeferredOperationName MessagingOperationName = "receive_deferred" + RenewMessageLockOperationName MessagingOperationName = "renew_message_lock" + + AbandonOperationName MessagingOperationName = "abandon" + CompleteOperationName MessagingOperationName = "complete" + DeferOperationName MessagingOperationName = "defer" + DeadLetterOperationName MessagingOperationName = "deadletter" + DeleteOperationName MessagingOperationName = "delete" +) diff --git a/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go b/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go new file mode 100644 index 000000000000..6d4c1d3002a0 --- /dev/null +++ b/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +import ( + "context" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + "github.com/stretchr/testify/require" +) + +// TODO: stole this from sdk/data/aztables, but this should really be in sdk/azcore/tracing? + +const ( + SpanStatusUnset = tracing.SpanStatusUnset + SpanStatusError = tracing.SpanStatusError + SpanStatusOK = tracing.SpanStatusOK +) + +type SpanStatus = tracing.SpanStatus + +// NewSpanValidator creates a Provider that verifies a span was created that matches the specified SpanMatcher. +func NewSpanValidator(t *testing.T, matcher SpanMatcher) Provider { + return tracing.NewProvider(func(name, version string) Tracer { + tt := matchingTracer{ + matcher: matcher, + } + + t.Cleanup(func() { + require.NotNil(t, tt.match, "didn't find a span with name %s", tt.matcher.Name) + require.True(t, tt.match.ended, "span wasn't ended") + require.EqualValues(t, matcher.Status, tt.match.status, "span status values don't match") + require.ElementsMatch(t, matcher.Attributes, tt.match.attrs, "span attributes don't match") + }) + + return tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { + kind := tracing.SpanKindInternal + attrs := []Attribute{} + if options != nil { + kind = options.Kind + attrs = append(attrs, options.Attributes...) + } + return tt.Start(ctx, spanName, kind, attrs) + }, nil) + }, nil) +} + +// SpanMatcher contains the values to match when a span is created. +type SpanMatcher struct { + Name string + Status SpanStatus + Attributes []Attribute +} + +type matchingTracer struct { + matcher SpanMatcher + attrs []Attribute + match *span +} + +func (mt *matchingTracer) Start(ctx context.Context, spanName string, kind tracing.SpanKind, attrs []Attribute) (context.Context, tracing.Span) { + if spanName != mt.matcher.Name { + return ctx, tracing.Span{} + } + // span name matches our matcher, track it + mt.match = &span{ + name: spanName, + attrs: attrs, + } + return ctx, tracing.NewSpan(tracing.SpanImpl{ + End: mt.match.End, + SetStatus: mt.match.SetStatus, + SetAttributes: mt.match.SetAttributes, + }) +} + +type span struct { + name string + status SpanStatus + desc string + attrs []Attribute + ended bool +} + +func (s *span) End() { + s.ended = true +} + +func (s *span) SetAttributes(attrs ...Attribute) { + s.attrs = append(s.attrs, attrs...) +} + +func (s *span) SetStatus(code SpanStatus, desc string) { + s.status = code + s.desc = desc + s.ended = true +} diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go new file mode 100644 index 000000000000..f15f3e3bcc70 --- /dev/null +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" +) + +type Attribute = tracing.Attribute +type Tracer = tracing.Tracer +type Provider = tracing.Provider + +type StartSpanOptions = runtime.StartSpanOptions + +func StartSpan(ctx context.Context, spanName string, tracer Tracer, options *StartSpanOptions) (context.Context, func(error)) { + return runtime.StartSpan(ctx, spanName, tracer, options) +} From 2f4a2b2bc4a7b84788dc73c5f2bb813a42a60369 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Tue, 10 Dec 2024 17:20:36 -0800 Subject: [PATCH 02/15] set up tracer in SB client and traces in sender methods --- sdk/messaging/azservicebus/client.go | 30 +++++++++ sdk/messaging/azservicebus/client_test.go | 22 +++++++ sdk/messaging/azservicebus/sender.go | 65 +++++++++++++++++-- .../azservicebus/sender_unit_test.go | 44 ++++++++++++- sdk/messaging/azservicebus/tracing.go | 42 ++++++++++++ sdk/messaging/azservicebus/tracing_test.go | 4 ++ 6 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 sdk/messaging/azservicebus/tracing.go create mode 100644 sdk/messaging/azservicebus/tracing_test.go diff --git a/sdk/messaging/azservicebus/client.go b/sdk/messaging/azservicebus/client.go index ed3f389ac203..20fb84ea51da 100644 --- a/sdk/messaging/azservicebus/client.go +++ b/sdk/messaging/azservicebus/client.go @@ -8,6 +8,7 @@ import ( "crypto/tls" "errors" "net" + "strings" "sync" "sync/atomic" "time" @@ -17,6 +18,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" ) // Client provides methods to create Sender and Receiver @@ -31,6 +33,7 @@ type Client struct { linksMu *sync.Mutex links map[uint64]amqpwrap.Closeable + tracer tracing.Tracer creds clientCreds namespace internal.NamespaceForAMQPLinks retryOptions RetryOptions @@ -53,6 +56,10 @@ type ClientOptions struct { // For an example, see ExampleNewClient_usingWebsockets() function in example_client_test.go. NewWebSocketConn func(ctx context.Context, args NewWebSocketConnArgs) (net.Conn, error) + // TracingProvider configures the tracing provider. + // It defaults to a no-op tracer. + TracingProvider tracing.Provider + // RetryOptions controls how often operations are retried from this client and any // Receivers and Senders created from this client. RetryOptions RetryOptions @@ -133,6 +140,7 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { } var err error + var tracingProvider = tracing.Provider{} var nsOptions []internal.NamespaceOption if client.creds.connectionString != "" { @@ -146,6 +154,7 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { } if args.ClientOptions != nil { + tracingProvider = args.ClientOptions.TracingProvider client.retryOptions = args.ClientOptions.RetryOptions if args.ClientOptions.TLSConfig != nil { @@ -166,6 +175,10 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { nsOptions = append(nsOptions, args.NSOptions...) client.namespace, err = internal.NewNamespace(nsOptions...) + + hostName := getHostName(creds) + client.tracer = newTracer(tracingProvider, hostName) + return client, err } @@ -192,6 +205,7 @@ func (client *Client) NewReceiverForQueue(queueName string, options *ReceiverOpt func (client *Client) NewReceiverForSubscription(topicName string, subscriptionName string, options *ReceiverOptions) (*Receiver, error) { id, cleanupOnClose := client.getCleanupForCloseable() receiver, err := newReceiver(newReceiverArgs{ + tracer: client.tracer, cleanupOnClose: cleanupOnClose, ns: client.namespace, entity: entity{Topic: topicName, Subscription: subscriptionName}, @@ -216,6 +230,7 @@ type NewSenderOptions struct { func (client *Client) NewSender(queueOrTopic string, options *NewSenderOptions) (*Sender, error) { id, cleanupOnClose := client.getCleanupForCloseable() sender, err := newSender(newSenderArgs{ + tracer: client.tracer, ns: client.namespace, queueOrTopic: queueOrTopic, cleanupOnClose: cleanupOnClose, @@ -369,3 +384,18 @@ func (client *Client) getCleanupForCloseable() (uint64, func()) { client.linksMu.Unlock() } } + +// getHostName returns fullyQualifiedNamespace if it is set, otherwise it returns the host name from the connection string. +// If the connection string is not in the expected format, it returns an empty string. +func getHostName(creds clientCreds) string { + if creds.fullyQualifiedNamespace != "" { + return creds.fullyQualifiedNamespace + } + + parts := strings.Split(creds.connectionString, "/") + if len(parts) < 3 { + return "" + } + + return parts[2] +} diff --git a/sdk/messaging/azservicebus/client_test.go b/sdk/messaging/azservicebus/client_test.go index b5afd398eaf6..9ca3d2c30396 100644 --- a/sdk/messaging/azservicebus/client_test.go +++ b/sdk/messaging/azservicebus/client_test.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" @@ -471,6 +472,27 @@ func TestNewClientUnitTests(t *testing.T) { require.EqualValues(t, ns.FQDN, "mysb.windows.servicebus.net") }) + t.Run("TracerIsSetUp", func(t *testing.T) { + // when tracing provider is not set, use a no-op tracer. + client, err := NewClient("fake.something", struct{ azcore.TokenCredential }{}, nil) + require.NoError(t, err) + require.Zero(t, client.tracer) + require.False(t, client.tracer.Enabled()) + + // when tracing provider is set, the tracer is set up with the provider. + provider := tracing.NewProvider(func(name, version string) tracing.Tracer { + return tracing.NewTracer(func(context.Context, string, *tracing.SpanOptions) (context.Context, tracing.Span) { + return nil, tracing.NewSpan(tracing.SpanImpl{}) + }, nil) + }, nil) + client, err = NewClient("fake.something", struct{ azcore.TokenCredential }{}, &ClientOptions{ + TracingProvider: provider, + }) + require.NoError(t, err) + require.NotZero(t, client.tracer) + require.True(t, client.tracer.Enabled()) + }) + t.Run("RetryOptionsArePropagated", func(t *testing.T) { // retry options are passed and copied along several routes, just make sure it's properly propagated. // NOTE: session receivers are checked in a separate test because they require actual SB access. diff --git a/sdk/messaging/azservicebus/sender.go b/sdk/messaging/azservicebus/sender.go index 8b3771e3ca45..f5c0437bbad8 100644 --- a/sdk/messaging/azservicebus/sender.go +++ b/sdk/messaging/azservicebus/sender.go @@ -6,10 +6,12 @@ package azservicebus import ( "context" "errors" + "fmt" "time" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -17,6 +19,7 @@ import ( type ( // Sender is used to send messages as well as schedule them to be delivered at a later date. Sender struct { + tracer tracing.Tracer queueOrTopic string cleanupOnClose func() links internal.AMQPLinks @@ -66,7 +69,12 @@ type SendMessageOptions struct { // - [ErrMessageTooLarge] if the message is larger than the maximum allowed link size. // - An [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendMessage(ctx context.Context, message *Message, options *SendMessageOptions) error { - return s.sendMessage(ctx, message) + var err error + ctx, endSpan := s.startSpan(ctx, "SendMessage", tracing.SendOperationName, getSpanAttributesForMessage(message)...) + defer func() { endSpan(err) }() + + err = s.sendMessage(ctx, message) + return err } // SendAMQPAnnotatedMessageOptions contains optional parameters for the SendAMQPAnnotatedMessage function. @@ -81,7 +89,12 @@ type SendAMQPAnnotatedMessageOptions struct { // - [ErrMessageTooLarge] if the message is larger than the maximum allowed link size. // - An [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendAMQPAnnotatedMessage(ctx context.Context, message *AMQPAnnotatedMessage, options *SendAMQPAnnotatedMessageOptions) error { - return s.sendMessage(ctx, message) + var err error + ctx, endSpan := s.startSpan(ctx, "SendAMQPAnnotatedMessage", tracing.SendOperationName) + defer func() { endSpan(err) }() + + err = s.sendMessage(ctx, message) + return err } // SendMessageBatchOptions contains optional parameters for the SendMessageBatch function. @@ -93,7 +106,13 @@ type SendMessageBatchOptions struct { // Message batches can be created using [Sender.NewMessageBatch]. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendMessageBatch(ctx context.Context, batch *MessageBatch, options *SendMessageBatchOptions) error { - err := s.links.Retry(ctx, EventSender, "SendMessageBatch", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { + var err error + ctx, endSpan := s.startSpan(ctx, "SendMessageBatch", tracing.SendOperationName, + tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(batch.NumMessages())}, + ) + defer func() { endSpan(err) }() + + err = s.links.Retry(ctx, EventSender, "SendMessageBatch", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, batch.toAMQPMessage(), nil) }, RetryOptions(s.retryOptions)) @@ -110,7 +129,14 @@ type ScheduleMessagesOptions struct { // delivered can be cancelled using `Receiver.CancelScheduleMessage(s)` // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) ScheduleMessages(ctx context.Context, messages []*Message, scheduledEnqueueTime time.Time, options *ScheduleMessagesOptions) ([]int64, error) { - return scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) + var err error + ctx, endSpan := s.startSpan(ctx, "ScheduleMessages", tracing.ScheduleOperationName, + tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(len(messages))}, + ) + defer func() { endSpan(err) }() + + sequenceNumbers, err := scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) + return sequenceNumbers, err } // ScheduleAMQPAnnotatedMessagesOptions contains optional parameters for the ScheduleAMQPAnnotatedMessages function. @@ -123,7 +149,14 @@ type ScheduleAMQPAnnotatedMessagesOptions struct { // delivered can be cancelled using `Receiver.CancelScheduleMessage(s)` // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) ScheduleAMQPAnnotatedMessages(ctx context.Context, messages []*AMQPAnnotatedMessage, scheduledEnqueueTime time.Time, options *ScheduleAMQPAnnotatedMessagesOptions) ([]int64, error) { - return scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) + var err error + ctx, endSpan := s.startSpan(ctx, "ScheduleAMQPAnnotatedMessages", tracing.ScheduleOperationName, + tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(len(messages))}, + ) + defer func() { endSpan(err) }() + + sequenceNumbers, err := scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) + return sequenceNumbers, err } func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links internal.AMQPLinks, retryOptions RetryOptions, messages []T, scheduledEnqueueTime time.Time) ([]int64, error) { @@ -158,7 +191,13 @@ type CancelScheduledMessagesOptions struct { // CancelScheduledMessages cancels multiple messages that were scheduled. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) CancelScheduledMessages(ctx context.Context, sequenceNumbers []int64, options *CancelScheduledMessagesOptions) error { - err := s.links.Retry(ctx, EventSender, "CancelScheduledMessages", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { + var err error + ctx, endSpan := s.startSpan(ctx, "CancelScheduledMessages", tracing.CancelScheduledOperationName, + tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(len(sequenceNumbers))}, + ) + defer func() { endSpan(err) }() + + err = s.links.Retry(ctx, EventSender, "CancelScheduledMessages", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.CancelScheduledMessages(ctx, lwv.RPC, lwv.Sender.LinkName(), sequenceNumbers) }, s.retryOptions) @@ -199,7 +238,20 @@ func (sender *Sender) createSenderLink(ctx context.Context, session amqpwrap.AMQ return amqpSender, nil, nil } +func (s *Sender) startSpan(ctx context.Context, spanName string, operationName tracing.MessagingOperationName, attrs ...tracing.Attribute) (context.Context, func(error)) { + spanName = fmt.Sprintf("Sender.%s", spanName) + attributes := []tracing.Attribute{ + {Key: tracing.OperationName, Value: string(operationName)}, + {Key: tracing.OperationType, Value: string(tracing.SendOperationType)}, + {Key: tracing.DestinationName, Value: s.queueOrTopic}, + } + attributes = append(attributes, attrs...) + + return tracing.StartSpan(ctx, spanName, s.tracer, &tracing.StartSpanOptions{Attributes: attributes}) +} + type newSenderArgs struct { + tracer tracing.Tracer ns internal.NamespaceForAMQPLinks queueOrTopic string cleanupOnClose func() @@ -212,6 +264,7 @@ func newSender(args newSenderArgs) (*Sender, error) { } sender := &Sender{ + tracer: args.tracer, queueOrTopic: args.queueOrTopic, cleanupOnClose: args.cleanupOnClose, retryOptions: args.retryOptions, diff --git a/sdk/messaging/azservicebus/sender_unit_test.go b/sdk/messaging/azservicebus/sender_unit_test.go index b676ffc75150..3494c70d5b10 100644 --- a/sdk/messaging/azservicebus/sender_unit_test.go +++ b/sdk/messaging/azservicebus/sender_unit_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/mock/emulation" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/go-amqp" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -45,14 +46,45 @@ func TestSender_UserFacingError(t *testing.T) { var asSBError *Error - err = sender.SendMessage(context.Background(), &Message{}, nil) + msgID := "testID" + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Sender.SendMessage", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "send"}, + {Key: tracing.OperationType, Value: "send"}, + {Key: tracing.MessageID, Value: msgID}, + }, + }).NewTracer("module", "version") + err = sender.SendMessage(context.Background(), &Message{MessageID: &msgID}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Sender.CancelScheduledMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "cancel_scheduled"}, + {Key: tracing.OperationType, Value: "send"}, + {Key: tracing.BatchMessageCount, Value: int64(1)}, + }, + }).NewTracer("module", "version") err = sender.CancelScheduledMessages(context.Background(), []int64{1}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Sender.ScheduleMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "schedule"}, + {Key: tracing.OperationType, Value: "send"}, + {Key: tracing.BatchMessageCount, Value: int64(0)}, + }, + }).NewTracer("module", "version") seqNumbers, err := sender.ScheduleMessages(context.Background(), []*Message{}, time.Now(), nil) require.Empty(t, seqNumbers) require.ErrorAs(t, err, &asSBError) @@ -67,6 +99,16 @@ func TestSender_UserFacingError(t *testing.T) { }, nil) require.NoError(t, err) + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Sender.SendMessageBatch", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "send"}, + {Key: tracing.OperationType, Value: "send"}, + {Key: tracing.BatchMessageCount, Value: int64(1)}, + }, + }).NewTracer("module", "version") err = sender.SendMessageBatch(context.Background(), batch, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go new file mode 100644 index 000000000000..25bc7a3614c7 --- /dev/null +++ b/sdk/messaging/azservicebus/tracing.go @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azservicebus + +import ( + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" +) + +const messagingSystemName = "servicebus" + +func newTracer(provider tracing.Provider, hostName string) tracing.Tracer { + tracer := provider.NewTracer(internal.ModuleName, internal.Version) + if !tracer.Enabled() { + return tracer + } + + tracer.SetAttributes( + tracing.Attribute{Key: tracing.MessagingSystem, Value: messagingSystemName}, + ) + if hostName != "" { + tracer.SetAttributes( + tracing.Attribute{Key: tracing.ServerAddress, Value: hostName}, + ) + } + + return tracer +} + +func getSpanAttributesForMessage(message *Message) []tracing.Attribute { + attrs := []tracing.Attribute{} + if message != nil { + if message.MessageID != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.MessageID, Value: *message.MessageID}) + } + if message.CorrelationID != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.ConversationID, Value: *message.CorrelationID}) + } + } + return attrs +} diff --git a/sdk/messaging/azservicebus/tracing_test.go b/sdk/messaging/azservicebus/tracing_test.go new file mode 100644 index 000000000000..70ba170e8e51 --- /dev/null +++ b/sdk/messaging/azservicebus/tracing_test.go @@ -0,0 +1,4 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azservicebus From 7c2e4ce7687055f8f8c6187e9a1d8a87eea0d4e0 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Tue, 10 Dec 2024 17:57:31 -0800 Subject: [PATCH 03/15] add more unit tests --- sdk/messaging/azservicebus/client_test.go | 37 ++++++++++++++---- sdk/messaging/azservicebus/receiver.go | 4 ++ sdk/messaging/azservicebus/tracing_test.go | 44 ++++++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/sdk/messaging/azservicebus/client_test.go b/sdk/messaging/azservicebus/client_test.go index 9ca3d2c30396..25d3a017ada0 100644 --- a/sdk/messaging/azservicebus/client_test.go +++ b/sdk/messaging/azservicebus/client_test.go @@ -14,12 +14,12 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/sas" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/stretchr/testify/require" "nhooyr.io/websocket" ) @@ -473,24 +473,45 @@ func TestNewClientUnitTests(t *testing.T) { }) t.Run("TracerIsSetUp", func(t *testing.T) { + hostName := "fake.servicebus.windows.net" // when tracing provider is not set, use a no-op tracer. - client, err := NewClient("fake.something", struct{ azcore.TokenCredential }{}, nil) + client, err := NewClient(hostName, struct{ azcore.TokenCredential }{}, nil) require.NoError(t, err) require.Zero(t, client.tracer) require.False(t, client.tracer.Enabled()) // when tracing provider is set, the tracer is set up with the provider. - provider := tracing.NewProvider(func(name, version string) tracing.Tracer { - return tracing.NewTracer(func(context.Context, string, *tracing.SpanOptions) (context.Context, tracing.Span) { - return nil, tracing.NewSpan(tracing.SpanImpl{}) - }, nil) - }, nil) - client, err = NewClient("fake.something", struct{ azcore.TokenCredential }{}, &ClientOptions{ + provider := tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "TestSpan", + Status: tracing.SpanStatusUnset, + Attributes: []tracing.Attribute{ + {Key: tracing.MessagingSystem, Value: "servicebus"}, + {Key: tracing.ServerAddress, Value: hostName}, + }, + }) + client, err = NewClient(hostName, struct{ azcore.TokenCredential }{}, &ClientOptions{ TracingProvider: provider, }) require.NoError(t, err) require.NotZero(t, client.tracer) require.True(t, client.tracer.Enabled()) + + // ensure attributes are set up correctly. + _, endSpan := tracing.StartSpan(context.Background(), "TestSpan", client.tracer, nil) + endSpan(nil) + + // attributes should be set up when using a connection string. + fakeConnectionString := "Endpoint=sb://fake.servicebus.windows.net/;SharedAccessKeyName=TestName;SharedAccessKey=TestKey" + client, err = NewClientFromConnectionString(fakeConnectionString, &ClientOptions{ + TracingProvider: provider, + }) + require.NoError(t, err) + require.NotZero(t, client.tracer) + require.True(t, client.tracer.Enabled()) + + // ensure attributes are set up correctly. + _, endSpan = tracing.StartSpan(context.Background(), "TestSpan", client.tracer, nil) + endSpan(nil) }) t.Run("RetryOptionsArePropagated", func(t *testing.T) { diff --git a/sdk/messaging/azservicebus/receiver.go b/sdk/messaging/azservicebus/receiver.go index 86012c27cbc7..5f836b3373b4 100644 --- a/sdk/messaging/azservicebus/receiver.go +++ b/sdk/messaging/azservicebus/receiver.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -44,6 +45,7 @@ const ( // Receiver receives messages using pull based functions (ReceiveMessages). type Receiver struct { + tracer tracing.Tracer amqpLinks internal.AMQPLinks cancelReleaser *atomic.Value cleanupOnClose func() @@ -110,6 +112,7 @@ func applyReceiverOptions(receiver *Receiver, entity *entity, options *ReceiverO } type newReceiverArgs struct { + tracer tracing.Tracer ns internal.NamespaceForAMQPLinks entity entity cleanupOnClose func() @@ -128,6 +131,7 @@ func newReceiver(args newReceiverArgs, options *ReceiverOptions) (*Receiver, err } receiver := &Receiver{ + tracer: args.tracer, cancelReleaser: &atomic.Value{}, cleanupOnClose: args.cleanupOnClose, lastPeekedSequenceNumber: 0, diff --git a/sdk/messaging/azservicebus/tracing_test.go b/sdk/messaging/azservicebus/tracing_test.go index 70ba170e8e51..35095749a189 100644 --- a/sdk/messaging/azservicebus/tracing_test.go +++ b/sdk/messaging/azservicebus/tracing_test.go @@ -2,3 +2,47 @@ // Licensed under the MIT License. package azservicebus + +// write unit tests for tracing.go +import ( + "context" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" + "github.com/stretchr/testify/require" +) + +func TestNewTracer(t *testing.T) { + hostName := "fake.something" + provider := tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "TestSpan", + Status: tracing.SpanStatusUnset, + Attributes: []tracing.Attribute{ + {Key: tracing.MessagingSystem, Value: "servicebus"}, + {Key: tracing.ServerAddress, Value: hostName}, + }, + }) + tracer := newTracer(provider, hostName) + require.NotNil(t, tracer) + require.True(t, tracer.Enabled()) + + _, endSpan := tracing.StartSpan(context.Background(), "TestSpan", tracer, nil) + defer func() { endSpan(nil) }() +} + +func TestSpanAttributesForMessage(t *testing.T) { + attrs := getSpanAttributesForMessage(nil) + require.Empty(t, attrs) + + msgID := "messageID" + message := &Message{ + MessageID: &msgID, + } + attrs = getSpanAttributesForMessage(message) + require.Equal(t, 1, len(attrs)) + + correlationID := "correlationID" + message.CorrelationID = &correlationID + attrs = getSpanAttributesForMessage(message) + require.Equal(t, 2, len(attrs)) +} From 23d7e94ac29c19325e71299aa9104cf23696a919 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Wed, 11 Dec 2024 10:22:05 -0800 Subject: [PATCH 04/15] linting --- sdk/messaging/azservicebus/internal/tracing/fake_tracing.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go b/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go index 6d4c1d3002a0..762b97f2b572 100644 --- a/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go @@ -56,7 +56,6 @@ type SpanMatcher struct { type matchingTracer struct { matcher SpanMatcher - attrs []Attribute match *span } From 1be63f7dd659855342393935e09b60eeac7524b5 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Wed, 11 Dec 2024 15:40:35 -0800 Subject: [PATCH 05/15] move matcher to sdk/internal folder and add callback function for starting a span --- sdk/internal/go.mod | 10 ++-- sdk/internal/go.sum | 8 +++ sdk/internal/telemetry/matcher.go | 83 ++++++++++++++++++++++++++ sdk/internal/telemetry/matcher_test.go | 53 ++++++++++++++++ sdk/internal/telemetry/span.go | 23 +++++++ sdk/internal/telemetry/span_test.go | 36 +++++++++++ 6 files changed, 208 insertions(+), 5 deletions(-) create mode 100644 sdk/internal/telemetry/matcher.go create mode 100644 sdk/internal/telemetry/matcher_test.go create mode 100644 sdk/internal/telemetry/span.go create mode 100644 sdk/internal/telemetry/span_test.go diff --git a/sdk/internal/go.mod b/sdk/internal/go.mod index 744ba0aee7bc..5408fb314683 100644 --- a/sdk/internal/go.mod +++ b/sdk/internal/go.mod @@ -3,11 +3,11 @@ module github.com/Azure/azure-sdk-for-go/sdk/internal go 1.18 require ( - github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 github.com/stretchr/testify v1.9.0 - golang.org/x/net v0.27.0 - golang.org/x/text v0.16.0 + golang.org/x/net v0.29.0 + golang.org/x/text v0.18.0 ) require ( @@ -20,8 +20,8 @@ require ( github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rogpeppe/go-internal v1.12.0 // indirect - golang.org/x/crypto v0.25.0 // indirect - golang.org/x/sys v0.22.0 // indirect + golang.org/x/crypto v0.27.0 // indirect + golang.org/x/sys v0.25.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/sdk/internal/go.sum b/sdk/internal/go.sum index 230708319d8f..e6e67c7b21bf 100644 --- a/sdk/internal/go.sum +++ b/sdk/internal/go.sum @@ -1,5 +1,7 @@ github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 h1:GJHeeA2N7xrG3q30L2UXDyuWRzDM900/65j70wcM4Ww= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0/go.mod h1:l38EPgmsp71HHLq9j7De57JcKOWPyhrsW1Awm1JS6K0= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BLisIzM9dG1R50zUk9C/M= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0/go.mod h1:YL1xnZ6QejvQHWJrX/AvhFl4WW4rqHVoKspWNVwFk0M= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 h1:tfLQ34V6F7tVSwoTf/4lH5sE0o6eCJuNDTmH09nDpbc= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0/go.mod h1:9kIvujWAA58nmPmWB1m23fyWic1kYZMxD9CxaWn4Qpg= github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mxXfQidrMEnLlPk9UMeRtyBTnEFtxkV0kU= @@ -32,13 +34,19 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= +golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70= golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys= golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= +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/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= +golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/sdk/internal/telemetry/matcher.go b/sdk/internal/telemetry/matcher.go new file mode 100644 index 000000000000..10d6e340609d --- /dev/null +++ b/sdk/internal/telemetry/matcher.go @@ -0,0 +1,83 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package telemetry + +import ( + "context" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + "github.com/stretchr/testify/require" +) + +// NewSpanValidator creates a Provider that verifies a span was created that matches the specified SpanMatcher. +func NewSpanValidator(t *testing.T, matcher SpanMatcher) tracing.Provider { + return tracing.NewProvider(func(name, version string) tracing.Tracer { + tt := matchingTracer{ + matcher: matcher, + } + + t.Cleanup(func() { + require.NotNil(t, tt.match, "didn't find a span with name %s", tt.matcher.Name) + require.True(t, tt.match.ended, "span wasn't ended") + require.EqualValues(t, matcher.Status, tt.match.status, "span status values don't match") + require.ElementsMatch(t, matcher.Attributes, tt.match.attrs, "span attributes don't match") + }) + + return tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { + kind := tracing.SpanKindInternal + attrs := []tracing.Attribute{} + if options != nil { + kind = options.Kind + attrs = append(attrs, options.Attributes...) + } + return tt.Start(ctx, spanName, kind, attrs) + }, nil) + }, nil) +} + +// SpanMatcher contains the values to match when a span is created. +type SpanMatcher struct { + Name string + Status tracing.SpanStatus + Attributes []tracing.Attribute +} + +type matchingTracer struct { + matcher SpanMatcher + match *span +} + +func (mt *matchingTracer) Start(ctx context.Context, spanName string, kind tracing.SpanKind, attrs []tracing.Attribute) (context.Context, tracing.Span) { + if spanName != mt.matcher.Name { + return ctx, tracing.Span{} + } + // span name matches our matcher, track it + mt.match = &span{ + name: spanName, + attrs: attrs, + } + return ctx, tracing.NewSpan(tracing.SpanImpl{ + End: mt.match.End, + SetStatus: mt.match.SetStatus, + }) +} + +type span struct { + name string + status tracing.SpanStatus + desc string + attrs []tracing.Attribute + ended bool +} + +func (s *span) End() { + s.ended = true +} + +func (s *span) SetStatus(code tracing.SpanStatus, desc string) { + s.status = code + s.desc = desc + s.ended = true +} diff --git a/sdk/internal/telemetry/matcher_test.go b/sdk/internal/telemetry/matcher_test.go new file mode 100644 index 000000000000..7a4867e48e0b --- /dev/null +++ b/sdk/internal/telemetry/matcher_test.go @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package telemetry + +// unit test for matcher.go +import ( + "context" + "errors" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + "github.com/stretchr/testify/require" +) + +func TestNewSpanValidator(t *testing.T) { + // validates a span with no attributes + matcher := SpanMatcher{ + Name: "TestSpan", + Status: tracing.SpanStatusError, + } + provider := NewSpanValidator(t, matcher) + tracer := provider.NewTracer("module", "version") + require.NotNil(t, tracer) + ctx, endSpan := runtime.StartSpan(context.Background(), "TestSpan", tracer, nil) + defer endSpan(errors.New("test error")) + require.NotNil(t, ctx) + + // does not track a span with a different name + ctx, endSpan = runtime.StartSpan(context.Background(), "AnotherSpan", tracer, nil) + defer endSpan(errors.New("test error")) + require.NotNil(t, ctx) + + // validates when attributes are provided + matcher = SpanMatcher{ + Name: "TestSpan", + Status: tracing.SpanStatusUnset, + Attributes: []tracing.Attribute{ + {Key: "testKey", Value: "testValue"}, + }, + } + provider = NewSpanValidator(t, matcher) + tracer = provider.NewTracer("module", "version") + require.NotNil(t, tracer) + _, endSpan = runtime.StartSpan(context.Background(), "TestSpan", tracer, &runtime.StartSpanOptions{ + Attributes: []tracing.Attribute{ + {Key: "testKey", Value: "testValue"}, + }, + }) + defer endSpan(nil) + require.NotNil(t, ctx) +} diff --git a/sdk/internal/telemetry/span.go b/sdk/internal/telemetry/span.go new file mode 100644 index 000000000000..8ba7a764027a --- /dev/null +++ b/sdk/internal/telemetry/span.go @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package telemetry + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" +) + +type SpanCallbackFn = func(context.Context) error + +// WithSpan creates a span and executes the provided function with the span's context. +// The span is ended with the error returned from the function. +func WithSpan(ctx context.Context, spanName string, tracer tracing.Tracer, options *runtime.StartSpanOptions, fn SpanCallbackFn) error { + var err error + ctx, endSpan := runtime.StartSpan(ctx, spanName, tracer, options) + defer func() { endSpan(err) }() + err = fn(ctx) + return err +} diff --git a/sdk/internal/telemetry/span_test.go b/sdk/internal/telemetry/span_test.go new file mode 100644 index 000000000000..4082f377f9d8 --- /dev/null +++ b/sdk/internal/telemetry/span_test.go @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package telemetry + +// unit test for span.go +import ( + "context" + "errors" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + "github.com/stretchr/testify/require" +) + +func TestWithSpan(t *testing.T) { + tracer := NewSpanValidator(t, SpanMatcher{ + Name: "TestSpan", + Status: tracing.SpanStatusError, + }).NewTracer("module", "version") + require.NotNil(t, tracer) + err := WithSpan(context.Background(), "TestSpan", tracer, nil, func(ctx context.Context) error { + return errors.New("test error") + }) + require.Error(t, err) + + tracer = NewSpanValidator(t, SpanMatcher{ + Name: "TestSpan", + Status: tracing.SpanStatusUnset, + }).NewTracer("module", "version") + require.NotNil(t, tracer) + err = WithSpan(context.Background(), "TestSpan", tracer, nil, func(ctx context.Context) error { + return nil + }) + require.Nil(t, err) +} From b026074f5df986f90817616c372392d117d63fbc Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Wed, 11 Dec 2024 22:22:14 -0800 Subject: [PATCH 06/15] address comments and moved startspan snippet to retrier layer --- sdk/internal/telemetry/matcher.go | 83 ------------------- sdk/internal/telemetry/matcher_test.go | 53 ------------ sdk/internal/telemetry/span.go | 23 ----- sdk/internal/telemetry/span_test.go | 36 -------- sdk/messaging/azservicebus/client.go | 20 ++--- sdk/messaging/azservicebus/client_test.go | 4 +- sdk/messaging/azservicebus/go.mod | 3 + sdk/messaging/azservicebus/go.sum | 4 +- .../azservicebus/internal/amqpLinks.go | 20 ++++- .../azservicebus/internal/amqpLinks_test.go | 4 +- .../azservicebus/internal/amqp_test_utils.go | 12 ++- .../internal/amqplinks_unit_test.go | 8 +- .../azservicebus/internal/constants.go | 2 +- .../azservicebus/internal/namespace.go | 6 +- .../internal/tracing/constants.go | 13 ++- .../tracing/{fake_tracing.go => matcher.go} | 2 - .../azservicebus/internal/tracing/tracing.go | 31 ++++++- .../azservicebus/internal/utils/retrier.go | 5 +- .../internal/utils/retrier_test.go | 53 ++++++------ .../azservicebus/liveTestHelpers_test.go | 6 +- sdk/messaging/azservicebus/messageSettler.go | 2 +- .../azservicebus/messageSettler_test.go | 10 +-- sdk/messaging/azservicebus/receiver.go | 8 +- sdk/messaging/azservicebus/sender.go | 73 ++++------------ .../azservicebus/sender_unit_test.go | 29 +++++-- .../azservicebus/session_receiver.go | 6 +- .../azservicebus/session_receiver_test.go | 4 +- sdk/messaging/azservicebus/tracing.go | 40 +++++++-- sdk/messaging/azservicebus/tracing_test.go | 19 +---- 29 files changed, 209 insertions(+), 370 deletions(-) delete mode 100644 sdk/internal/telemetry/matcher.go delete mode 100644 sdk/internal/telemetry/matcher_test.go delete mode 100644 sdk/internal/telemetry/span.go delete mode 100644 sdk/internal/telemetry/span_test.go rename sdk/messaging/azservicebus/internal/tracing/{fake_tracing.go => matcher.go} (96%) diff --git a/sdk/internal/telemetry/matcher.go b/sdk/internal/telemetry/matcher.go deleted file mode 100644 index 10d6e340609d..000000000000 --- a/sdk/internal/telemetry/matcher.go +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package telemetry - -import ( - "context" - "testing" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" - "github.com/stretchr/testify/require" -) - -// NewSpanValidator creates a Provider that verifies a span was created that matches the specified SpanMatcher. -func NewSpanValidator(t *testing.T, matcher SpanMatcher) tracing.Provider { - return tracing.NewProvider(func(name, version string) tracing.Tracer { - tt := matchingTracer{ - matcher: matcher, - } - - t.Cleanup(func() { - require.NotNil(t, tt.match, "didn't find a span with name %s", tt.matcher.Name) - require.True(t, tt.match.ended, "span wasn't ended") - require.EqualValues(t, matcher.Status, tt.match.status, "span status values don't match") - require.ElementsMatch(t, matcher.Attributes, tt.match.attrs, "span attributes don't match") - }) - - return tracing.NewTracer(func(ctx context.Context, spanName string, options *tracing.SpanOptions) (context.Context, tracing.Span) { - kind := tracing.SpanKindInternal - attrs := []tracing.Attribute{} - if options != nil { - kind = options.Kind - attrs = append(attrs, options.Attributes...) - } - return tt.Start(ctx, spanName, kind, attrs) - }, nil) - }, nil) -} - -// SpanMatcher contains the values to match when a span is created. -type SpanMatcher struct { - Name string - Status tracing.SpanStatus - Attributes []tracing.Attribute -} - -type matchingTracer struct { - matcher SpanMatcher - match *span -} - -func (mt *matchingTracer) Start(ctx context.Context, spanName string, kind tracing.SpanKind, attrs []tracing.Attribute) (context.Context, tracing.Span) { - if spanName != mt.matcher.Name { - return ctx, tracing.Span{} - } - // span name matches our matcher, track it - mt.match = &span{ - name: spanName, - attrs: attrs, - } - return ctx, tracing.NewSpan(tracing.SpanImpl{ - End: mt.match.End, - SetStatus: mt.match.SetStatus, - }) -} - -type span struct { - name string - status tracing.SpanStatus - desc string - attrs []tracing.Attribute - ended bool -} - -func (s *span) End() { - s.ended = true -} - -func (s *span) SetStatus(code tracing.SpanStatus, desc string) { - s.status = code - s.desc = desc - s.ended = true -} diff --git a/sdk/internal/telemetry/matcher_test.go b/sdk/internal/telemetry/matcher_test.go deleted file mode 100644 index 7a4867e48e0b..000000000000 --- a/sdk/internal/telemetry/matcher_test.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package telemetry - -// unit test for matcher.go -import ( - "context" - "errors" - "testing" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" - "github.com/stretchr/testify/require" -) - -func TestNewSpanValidator(t *testing.T) { - // validates a span with no attributes - matcher := SpanMatcher{ - Name: "TestSpan", - Status: tracing.SpanStatusError, - } - provider := NewSpanValidator(t, matcher) - tracer := provider.NewTracer("module", "version") - require.NotNil(t, tracer) - ctx, endSpan := runtime.StartSpan(context.Background(), "TestSpan", tracer, nil) - defer endSpan(errors.New("test error")) - require.NotNil(t, ctx) - - // does not track a span with a different name - ctx, endSpan = runtime.StartSpan(context.Background(), "AnotherSpan", tracer, nil) - defer endSpan(errors.New("test error")) - require.NotNil(t, ctx) - - // validates when attributes are provided - matcher = SpanMatcher{ - Name: "TestSpan", - Status: tracing.SpanStatusUnset, - Attributes: []tracing.Attribute{ - {Key: "testKey", Value: "testValue"}, - }, - } - provider = NewSpanValidator(t, matcher) - tracer = provider.NewTracer("module", "version") - require.NotNil(t, tracer) - _, endSpan = runtime.StartSpan(context.Background(), "TestSpan", tracer, &runtime.StartSpanOptions{ - Attributes: []tracing.Attribute{ - {Key: "testKey", Value: "testValue"}, - }, - }) - defer endSpan(nil) - require.NotNil(t, ctx) -} diff --git a/sdk/internal/telemetry/span.go b/sdk/internal/telemetry/span.go deleted file mode 100644 index 8ba7a764027a..000000000000 --- a/sdk/internal/telemetry/span.go +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package telemetry - -import ( - "context" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" -) - -type SpanCallbackFn = func(context.Context) error - -// WithSpan creates a span and executes the provided function with the span's context. -// The span is ended with the error returned from the function. -func WithSpan(ctx context.Context, spanName string, tracer tracing.Tracer, options *runtime.StartSpanOptions, fn SpanCallbackFn) error { - var err error - ctx, endSpan := runtime.StartSpan(ctx, spanName, tracer, options) - defer func() { endSpan(err) }() - err = fn(ctx) - return err -} diff --git a/sdk/internal/telemetry/span_test.go b/sdk/internal/telemetry/span_test.go deleted file mode 100644 index 4082f377f9d8..000000000000 --- a/sdk/internal/telemetry/span_test.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package telemetry - -// unit test for span.go -import ( - "context" - "errors" - "testing" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" - "github.com/stretchr/testify/require" -) - -func TestWithSpan(t *testing.T) { - tracer := NewSpanValidator(t, SpanMatcher{ - Name: "TestSpan", - Status: tracing.SpanStatusError, - }).NewTracer("module", "version") - require.NotNil(t, tracer) - err := WithSpan(context.Background(), "TestSpan", tracer, nil, func(ctx context.Context) error { - return errors.New("test error") - }) - require.Error(t, err) - - tracer = NewSpanValidator(t, SpanMatcher{ - Name: "TestSpan", - Status: tracing.SpanStatusUnset, - }).NewTracer("module", "version") - require.NotNil(t, tracer) - err = WithSpan(context.Background(), "TestSpan", tracer, nil, func(ctx context.Context) error { - return nil - }) - require.Nil(t, err) -} diff --git a/sdk/messaging/azservicebus/client.go b/sdk/messaging/azservicebus/client.go index 20fb84ea51da..e7c64ebfb481 100644 --- a/sdk/messaging/azservicebus/client.go +++ b/sdk/messaging/azservicebus/client.go @@ -8,7 +8,6 @@ import ( "crypto/tls" "errors" "net" - "strings" "sync" "sync/atomic" "time" @@ -17,6 +16,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/internal/log" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/conn" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" ) @@ -175,9 +175,7 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { nsOptions = append(nsOptions, args.NSOptions...) client.namespace, err = internal.NewNamespace(nsOptions...) - - hostName := getHostName(creds) - client.tracer = newTracer(tracingProvider, hostName) + client.tracer = newTracer(tracingProvider, getHostName(creds)) return client, err } @@ -186,6 +184,7 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { func (client *Client) NewReceiverForQueue(queueName string, options *ReceiverOptions) (*Receiver, error) { id, cleanupOnClose := client.getCleanupForCloseable() receiver, err := newReceiver(newReceiverArgs{ + tracer: client.tracer, cleanupOnClose: cleanupOnClose, ns: client.namespace, entity: entity{Queue: queueName}, @@ -385,17 +384,16 @@ func (client *Client) getCleanupForCloseable() (uint64, func()) { } } -// getHostName returns fullyQualifiedNamespace if it is set, otherwise it returns the host name from the connection string. -// If the connection string is not in the expected format, it returns an empty string. +// getHostName returns fullyQualifiedNamespace from clientCreds if it is set. +// Otherwise, it parses the connection string and returns the FullyQualifiedNamespace from it. +// If both are empty, it returns an empty string. func getHostName(creds clientCreds) string { if creds.fullyQualifiedNamespace != "" { return creds.fullyQualifiedNamespace } - - parts := strings.Split(creds.connectionString, "/") - if len(parts) < 3 { + csp, err := conn.ParseConnectionString(creds.connectionString) + if err != nil { return "" } - - return parts[2] + return csp.FullyQualifiedNamespace } diff --git a/sdk/messaging/azservicebus/client_test.go b/sdk/messaging/azservicebus/client_test.go index 25d3a017ada0..8973ebac34e7 100644 --- a/sdk/messaging/azservicebus/client_test.go +++ b/sdk/messaging/azservicebus/client_test.go @@ -497,7 +497,7 @@ func TestNewClientUnitTests(t *testing.T) { require.True(t, client.tracer.Enabled()) // ensure attributes are set up correctly. - _, endSpan := tracing.StartSpan(context.Background(), "TestSpan", client.tracer, nil) + _, endSpan := tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanOptions("TestSpan")) endSpan(nil) // attributes should be set up when using a connection string. @@ -510,7 +510,7 @@ func TestNewClientUnitTests(t *testing.T) { require.True(t, client.tracer.Enabled()) // ensure attributes are set up correctly. - _, endSpan = tracing.StartSpan(context.Background(), "TestSpan", client.tracer, nil) + _, endSpan = tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanOptions("TestSpan")) endSpan(nil) }) diff --git a/sdk/messaging/azservicebus/go.mod b/sdk/messaging/azservicebus/go.mod index 1ddb84768278..72e4a3f7309c 100644 --- a/sdk/messaging/azservicebus/go.mod +++ b/sdk/messaging/azservicebus/go.mod @@ -40,3 +40,6 @@ require ( golang.org/x/text v0.18.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +// TODO: remove this +replace github.com/Azure/azure-sdk-for-go/sdk/internal => github.com/karenychen/azure-sdk-for-go/sdk/internal v0.0.0-20241211234035-1be63f7dd659 diff --git a/sdk/messaging/azservicebus/go.sum b/sdk/messaging/azservicebus/go.sum index 6e3fa14a700a..5461add4c7f5 100644 --- a/sdk/messaging/azservicebus/go.sum +++ b/sdk/messaging/azservicebus/go.sum @@ -2,8 +2,6 @@ github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BL github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0/go.mod h1:YL1xnZ6QejvQHWJrX/AvhFl4WW4rqHVoKspWNVwFk0M= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 h1:tfLQ34V6F7tVSwoTf/4lH5sE0o6eCJuNDTmH09nDpbc= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0/go.mod h1:9kIvujWAA58nmPmWB1m23fyWic1kYZMxD9CxaWn4Qpg= -github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY= -github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY= github.com/Azure/go-amqp v1.1.0 h1:XUhx5f4lZFVf6LQc5kBUFECW0iJW9VLxKCYrBeGwl0U= github.com/Azure/go-amqp v1.1.0/go.mod h1:vZAogwdrkbyK3Mla8m/CxSc/aKdnTZ4IbPxl51Y5WZE= github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mxXfQidrMEnLlPk9UMeRtyBTnEFtxkV0kU= @@ -21,6 +19,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= +github.com/karenychen/azure-sdk-for-go/sdk/internal v0.0.0-20241211234035-1be63f7dd659 h1:E84nd0UapmfpTQkN8aN2yCGgVtCd/WU1aP4MNeA95PQ= +github.com/karenychen/azure-sdk-for-go/sdk/internal v0.0.0-20241211234035-1be63f7dd659/go.mod h1:XA5QmAjGSl2hLWSxyUSl0pWSxmXATjchXnEl53alEUQ= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= diff --git a/sdk/messaging/azservicebus/internal/amqpLinks.go b/sdk/messaging/azservicebus/internal/amqpLinks.go index a7d0590fc669..f381befba623 100644 --- a/sdk/messaging/azservicebus/internal/amqpLinks.go +++ b/sdk/messaging/azservicebus/internal/amqpLinks.go @@ -15,6 +15,7 @@ import ( azlog "github.com/Azure/azure-sdk-for-go/sdk/internal/log" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" ) @@ -42,7 +43,7 @@ type AMQPLinks interface { Get(ctx context.Context) (*LinksWithID, error) // Retry will run your callback, recovering links when necessary. - Retry(ctx context.Context, name log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions) error + Retry(ctx context.Context, name log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, so *tracing.SpanOptions) error // RecoverIfNeeded will check if an error requires recovery, and will recover // the link or, possibly, the connection. @@ -66,6 +67,9 @@ type AMQPLinks interface { // Prefix is the current logging prefix, usable for logging and continuity. Prefix() string + + // SetTracer sets the tracer for the AMQPLinks instance. + SetTracer(tracing.Tracer) } // AMQPLinksImpl manages the set of AMQP links (and detritus) typically needed to work @@ -85,6 +89,8 @@ type AMQPLinksImpl struct { // PR: https://github.com/Azure/azure-sdk-for-go/pull/16847 id LinkID + tracer tracing.Tracer + entityPath string managementPath string audience string @@ -122,6 +128,7 @@ type AMQPLinksImpl struct { type CreateLinkFunc func(ctx context.Context, session amqpwrap.AMQPSession) (amqpwrap.AMQPSenderCloser, amqpwrap.AMQPReceiverCloser, error) type NewAMQPLinksArgs struct { + Tracer tracing.Tracer NS NamespaceForAMQPLinks EntityPath string CreateLinkFunc CreateLinkFunc @@ -132,6 +139,7 @@ type NewAMQPLinksArgs struct { // management link for a specific entity path. func NewAMQPLinks(args NewAMQPLinksArgs) AMQPLinks { l := &AMQPLinksImpl{ + tracer: args.Tracer, entityPath: args.EntityPath, managementPath: fmt.Sprintf("%s/$management", args.EntityPath), audience: args.NS.GetEntityAudience(args.EntityPath), @@ -145,6 +153,10 @@ func NewAMQPLinks(args NewAMQPLinksArgs) AMQPLinks { return l } +func (links *AMQPLinksImpl) SetTracer(tracer tracing.Tracer) { + links.tracer = tracer +} + // ManagementPath is the management path for the associated entity. func (links *AMQPLinksImpl) ManagementPath() string { return links.managementPath @@ -315,7 +327,7 @@ func (l *AMQPLinksImpl) Get(ctx context.Context) (*LinksWithID, error) { }, nil } -func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions) error { +func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, so *tracing.SpanOptions) error { var lastID LinkID didQuickRetry := false @@ -324,7 +336,7 @@ func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, oper return links.getRecoveryKindFunc(err) == RecoveryKindFatal } - return utils.Retry(ctx, eventName, links.Prefix()+"("+operation+")", func(ctx context.Context, args *utils.RetryFnArgs) error { + return utils.Retry(ctx, links.tracer, eventName, links.Prefix()+"("+operation+")", func(ctx context.Context, args *utils.RetryFnArgs) error { if err := links.RecoverIfNeeded(ctx, lastID, args.LastErr); err != nil { return err } @@ -366,7 +378,7 @@ func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, oper } return nil - }, isFatalErrorFunc, o) + }, isFatalErrorFunc, o, so) } // EntityPath is the full entity path for the queue/topic/subscription. diff --git a/sdk/messaging/azservicebus/internal/amqpLinks_test.go b/sdk/messaging/azservicebus/internal/amqpLinks_test.go index d78bbded916c..791e6b75a713 100644 --- a/sdk/messaging/azservicebus/internal/amqpLinks_test.go +++ b/sdk/messaging/azservicebus/internal/amqpLinks_test.go @@ -455,7 +455,7 @@ func TestAMQPLinksCBSLinkStillOpen(t *testing.T) { }, exported.RetryOptions{ RetryDelay: -1, MaxRetryDelay: time.Millisecond, - }) + }, nil) defer func() { err := links.Close(context.Background(), true) @@ -629,7 +629,7 @@ func TestAMQPLinksRetry(t *testing.T) { // we do setDefaults() before we run. RetryDelay: time.Millisecond, MaxRetryDelay: time.Millisecond, - }) + }, nil) var connErr *amqp.ConnError require.ErrorAs(t, err, &connErr) diff --git a/sdk/messaging/azservicebus/internal/amqp_test_utils.go b/sdk/messaging/azservicebus/internal/amqp_test_utils.go index 20dbf52cb7a7..a78756934d15 100644 --- a/sdk/messaging/azservicebus/internal/amqp_test_utils.go +++ b/sdk/messaging/azservicebus/internal/amqp_test_utils.go @@ -11,6 +11,7 @@ import ( azlog "github.com/Azure/azure-sdk-for-go/sdk/internal/log" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -85,8 +86,13 @@ type FakeAMQPReceiver struct { } type FakeRPCLink struct { - Resp *amqpwrap.RPCResponse - Error error + Tracer tracing.Tracer + Resp *amqpwrap.RPCResponse + Error error +} + +func (r *FakeRPCLink) SetTracer(tracer tracing.Tracer) { + r.Tracer = tracer } func (r *FakeRPCLink) Close(ctx context.Context) error { @@ -198,7 +204,7 @@ func (l *FakeAMQPLinks) Get(ctx context.Context) (*LinksWithID, error) { } } -func (l *FakeAMQPLinks) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions) error { +func (l *FakeAMQPLinks) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, so *tracing.SpanOptions) error { lwr, err := l.Get(ctx) if err != nil { diff --git a/sdk/messaging/azservicebus/internal/amqplinks_unit_test.go b/sdk/messaging/azservicebus/internal/amqplinks_unit_test.go index dcc7907cd33c..5ba49d3d1ea2 100644 --- a/sdk/messaging/azservicebus/internal/amqplinks_unit_test.go +++ b/sdk/messaging/azservicebus/internal/amqplinks_unit_test.go @@ -80,7 +80,7 @@ func TestAMQPLinksRetriesUnit(t *testing.T) { return testData.Err }, exported.RetryOptions{ RetryDelay: time.Millisecond, - }) + }, nil) require.Equal(t, testData.Err, err) require.Equal(t, testData.Attempts, attempts) @@ -222,7 +222,7 @@ func TestAMQPCloseLinkTimeout_Receiver_CancellationDuringClose(t *testing.T) { err := links.Retry(userCtx, exported.EventConn, "Test", func(ctx context.Context, tmpLWID *LinksWithID, args *utils.RetryFnArgs) error { lwid = tmpLWID return nil - }, exported.RetryOptions{}) + }, exported.RetryOptions{}, nil) require.NoError(t, err) require.NotNil(t, lwid) @@ -241,7 +241,7 @@ func TestAMQPCloseLinkTimeout_Receiver_CancellationDuringClose(t *testing.T) { err = links.Retry(context.Background(), exported.EventConn, "Test", func(ctx context.Context, tmpLWID *LinksWithID, args *utils.RetryFnArgs) error { lwid = tmpLWID return nil - }, exported.RetryOptions{}) + }, exported.RetryOptions{}, nil) require.NoError(t, err) require.NotNil(t, lwid) @@ -280,7 +280,7 @@ func TestAMQPCloseLinkTimeout_Receiver_RecoverIfNeeded(t *testing.T) { err := links.Retry(userCtx, exported.EventConn, "Test", func(ctx context.Context, tmpLWID *LinksWithID, args *utils.RetryFnArgs) error { lwid = tmpLWID return nil - }, exported.RetryOptions{}) + }, exported.RetryOptions{}, nil) require.NoError(t, err) require.NotNil(t, lwid) diff --git a/sdk/messaging/azservicebus/internal/constants.go b/sdk/messaging/azservicebus/internal/constants.go index 7f55b1b7ddd6..8dfe4d417261 100644 --- a/sdk/messaging/azservicebus/internal/constants.go +++ b/sdk/messaging/azservicebus/internal/constants.go @@ -3,7 +3,7 @@ package internal -const ModuleName = "azservicebus" +const ModuleName = "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus" // Version is the semantic version number const Version = "v1.7.4" diff --git a/sdk/messaging/azservicebus/internal/namespace.go b/sdk/messaging/azservicebus/internal/namespace.go index 871a1d7b6c6d..72bd130e7e85 100644 --- a/sdk/messaging/azservicebus/internal/namespace.go +++ b/sdk/messaging/azservicebus/internal/namespace.go @@ -21,6 +21,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/conn" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/sbauth" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -419,7 +420,8 @@ func (ns *Namespace) startNegotiateClaimRenewer(ctx context.Context, return case <-time.After(nextClaimAt): for { - err := utils.Retry(refreshCtx, exported.EventAuth, "NegotiateClaimRefresh", func(ctx context.Context, args *utils.RetryFnArgs) error { + // use a no + err := utils.Retry(refreshCtx, tracing.NewNoOpTracer(), exported.EventAuth, "NegotiateClaimRefresh", func(ctx context.Context, args *utils.RetryFnArgs) error { tmpExpiresOn, err := refreshClaim(ctx) if err != nil { @@ -428,7 +430,7 @@ func (ns *Namespace) startNegotiateClaimRenewer(ctx context.Context, expiresOn = tmpExpiresOn return nil - }, IsFatalSBError, ns.RetryOptions) + }, IsFatalSBError, ns.RetryOptions, nil) if err == nil { break diff --git a/sdk/messaging/azservicebus/internal/tracing/constants.go b/sdk/messaging/azservicebus/internal/tracing/constants.go index 3e55044a61cf..5b0b3232108b 100644 --- a/sdk/messaging/azservicebus/internal/tracing/constants.go +++ b/sdk/messaging/azservicebus/internal/tracing/constants.go @@ -3,8 +3,18 @@ package tracing +type MessagingSpanName string + +const ( + SendSpanName MessagingSpanName = "Sender.SendMessage" + SendBatchSpanName MessagingSpanName = "Sender.SendMessageBatch" + ScheduleSpanName MessagingSpanName = "Sender.ScheduleMessages" + CancelScheduledSpanName MessagingSpanName = "Sender.CancelScheduledMessages" +) + // OTel-specific messaging attributes const ( + ServerAddress = "server.address" MessagingSystem = "messaging.system" OperationName = "messaging.operation.name" BatchMessageCount = "messaging.batch.message_count" @@ -16,9 +26,6 @@ const ( ConversationID = "messaging.message.conversation_id" MessageID = "messaging.message.id" EnqueuedTime = "messaging.servicebus.message.enqueued_time" - - ServerAddress = "server.address" - ServerPort = "server.port" ) type MessagingOperationType string diff --git a/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go b/sdk/messaging/azservicebus/internal/tracing/matcher.go similarity index 96% rename from sdk/messaging/azservicebus/internal/tracing/fake_tracing.go rename to sdk/messaging/azservicebus/internal/tracing/matcher.go index 762b97f2b572..f42b7360a393 100644 --- a/sdk/messaging/azservicebus/internal/tracing/fake_tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/matcher.go @@ -11,8 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -// TODO: stole this from sdk/data/aztables, but this should really be in sdk/azcore/tracing? - const ( SpanStatusUnset = tracing.SpanStatusUnset SpanStatusError = tracing.SpanStatusError diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go index f15f3e3bcc70..5b6f61f3a2fd 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -11,11 +11,36 @@ import ( ) type Attribute = tracing.Attribute +type SpanKind = tracing.SpanKind type Tracer = tracing.Tracer type Provider = tracing.Provider -type StartSpanOptions = runtime.StartSpanOptions +func NewNoOpTracer() Tracer { + return Tracer{} +} + +type SpanOptions struct { + name MessagingSpanName + attributes []Attribute +} + +type SetAttributesFn func([]Attribute) []Attribute -func StartSpan(ctx context.Context, spanName string, tracer Tracer, options *StartSpanOptions) (context.Context, func(error)) { - return runtime.StartSpan(ctx, spanName, tracer, options) +func NewSpanOptions(name MessagingSpanName, options ...SetAttributesFn) *SpanOptions { + so := &SpanOptions{name: name} + for _, fn := range options { + so.attributes = fn(so.attributes) + } + return so } + +// StartSpan creates a span with the specified name and attributes. +// If no span name is provided, no span is created. +func StartSpan(ctx context.Context, tracer Tracer, so *SpanOptions) (context.Context, func(error)) { + if so == nil || so.name == "" { + return ctx, func(error) {} + } + return runtime.StartSpan(ctx, string(so.name), tracer, &StartSpanOptions{Attributes: so.attributes}) +} + +type StartSpanOptions = runtime.StartSpanOptions diff --git a/sdk/messaging/azservicebus/internal/utils/retrier.go b/sdk/messaging/azservicebus/internal/utils/retrier.go index 8d2cb7a118d3..33da126293da 100644 --- a/sdk/messaging/azservicebus/internal/utils/retrier.go +++ b/sdk/messaging/azservicebus/internal/utils/retrier.go @@ -12,6 +12,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/internal/log" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" ) // EventRetry is the name for retry events @@ -37,7 +38,7 @@ func (rf *RetryFnArgs) ResetAttempts() { // Retry runs a standard retry loop. It executes your passed in fn as the body of the loop. // It returns if it exceeds the number of configured retry options or if 'isFatal' returns true. -func Retry(ctx context.Context, eventName log.Event, operation string, fn func(ctx context.Context, args *RetryFnArgs) error, isFatalFn func(err error) bool, o exported.RetryOptions) error { +func Retry(ctx context.Context, tracer tracing.Tracer, eventName log.Event, operation string, fn func(ctx context.Context, args *RetryFnArgs) error, isFatalFn func(err error) bool, o exported.RetryOptions, so *tracing.SpanOptions) error { if isFatalFn == nil { panic("isFatalFn is nil, errors would panic") } @@ -46,6 +47,8 @@ func Retry(ctx context.Context, eventName log.Event, operation string, fn func(c setDefaults(&ro) var err error + ctx, endSpan := tracing.StartSpan(ctx, tracer, so) + defer func() { endSpan(err) }() for i := int32(0); i <= ro.MaxRetries; i++ { if i > 0 { diff --git a/sdk/messaging/azservicebus/internal/utils/retrier_test.go b/sdk/messaging/azservicebus/internal/utils/retrier_test.go index 0334374581b6..04f3be003f0a 100644 --- a/sdk/messaging/azservicebus/internal/utils/retrier_test.go +++ b/sdk/messaging/azservicebus/internal/utils/retrier_test.go @@ -15,6 +15,7 @@ import ( azlog "github.com/Azure/azure-sdk-for-go/sdk/internal/log" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/go-amqp" "github.com/stretchr/testify/require" ) @@ -27,12 +28,12 @@ func TestRetrier(t *testing.T) { called := 0 - err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return nil }, func(err error) bool { panic("won't get called") - }, exported.RetryOptions{}) + }, exported.RetryOptions{}, nil) require.Nil(t, err) require.EqualValues(t, 1, called) @@ -51,7 +52,7 @@ func TestRetrier(t *testing.T) { return false } - err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ if args.I == 3 { @@ -60,7 +61,7 @@ func TestRetrier(t *testing.T) { } return fmt.Errorf("Error, iteration %d", args.I) - }, isFatalFn, fastRetryOptions) + }, isFatalFn, fastRetryOptions, nil) require.EqualValues(t, 4, called) require.EqualValues(t, 3, isFatalCalled) @@ -78,10 +79,10 @@ func TestRetrier(t *testing.T) { return true } - err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return errors.New("isFatalFn says this is a fatal error") - }, isFatalFn, exported.RetryOptions{}) + }, isFatalFn, exported.RetryOptions{}, nil) require.EqualValues(t, "isFatalFn says this is a fatal error", err.Error()) require.EqualValues(t, 1, called) @@ -96,7 +97,7 @@ func TestRetrier(t *testing.T) { maxRetries := int32(2) - err := Retry(context.Background(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { actualAttempts = append(actualAttempts, args.I) if len(actualAttempts) == int(maxRetries+1) { @@ -108,7 +109,7 @@ func TestRetrier(t *testing.T) { MaxRetries: maxRetries, RetryDelay: time.Millisecond, MaxRetryDelay: time.Millisecond, - }) + }, nil) expectedAttempts := []int32{ 0, 1, 2, // we resetted attempts here. @@ -129,10 +130,10 @@ func TestRetrier(t *testing.T) { called := 0 - err := Retry(context.Background(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return errors.New("whatever") - }, isFatalFn, customRetryOptions) + }, isFatalFn, customRetryOptions, nil) require.EqualValues(t, 1, called) require.EqualValues(t, "whatever", err.Error()) @@ -149,12 +150,12 @@ func TestCancellationCancelsSleep(t *testing.T) { called := 0 - err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return errors.New("try again") }, isFatalFn, exported.RetryOptions{ RetryDelay: time.Hour, - }) + }, nil) require.Error(t, err) require.ErrorIs(t, err, context.Canceled) @@ -173,7 +174,7 @@ func TestCancellationFromUserFunc(t *testing.T) { called := 0 - err := Retry(alreadyCancelledCtx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(alreadyCancelledCtx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ select { @@ -182,7 +183,7 @@ func TestCancellationFromUserFunc(t *testing.T) { default: panic("Context should have been cancelled") } - }, isFatalFn, exported.RetryOptions{}) + }, isFatalFn, exported.RetryOptions{}, nil) require.Error(t, err) require.ErrorIs(t, err, canceledfromFunc) @@ -197,13 +198,13 @@ func TestCancellationTimeoutsArentPropagatedToUser(t *testing.T) { tryAgainErr := errors.New("try again") called := 0 - err := Retry(context.Background(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ require.NoError(t, ctx.Err(), "our sleep/timeout doesn't show up for users") return tryAgainErr }, isFatalFn, exported.RetryOptions{ RetryDelay: time.Millisecond, - }) + }, nil) require.Error(t, err) require.ErrorIs(t, err, tryAgainErr, "error should be propagated from user callback") @@ -291,14 +292,14 @@ func TestRetryLogging(t *testing.T) { t.Run("normal error", func(t *testing.T) { logsFn := test.CaptureLogsForTest(false) - err := Retry(context.Background(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func, returning error hello", args.I) return errors.New("hello") }, func(err error) bool { return false }, exported.RetryOptions{ RetryDelay: time.Microsecond, - }) + }, nil) require.EqualError(t, err, "hello") require.Equal(t, []string{ @@ -322,21 +323,21 @@ func TestRetryLogging(t *testing.T) { t.Run("normal error2", func(t *testing.T) { test.EnableStdoutLogging(t) - err := Retry(context.Background(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func, returning error hello", args.I) return errors.New("hello") }, func(err error) bool { return false }, exported.RetryOptions{ RetryDelay: time.Microsecond, - }) + }, nil) require.EqualError(t, err, "hello") }) t.Run("cancellation error", func(t *testing.T) { logsFn := test.CaptureLogsForTest(false) - err := Retry(context.Background(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func", args.I) return context.Canceled @@ -344,7 +345,7 @@ func TestRetryLogging(t *testing.T) { return errors.Is(err, context.Canceled) }, exported.RetryOptions{ RetryDelay: time.Microsecond, - }) + }, nil) require.ErrorIs(t, err, context.Canceled) require.Equal(t, []string{ @@ -356,7 +357,7 @@ func TestRetryLogging(t *testing.T) { t.Run("custom fatal error", func(t *testing.T) { logsFn := test.CaptureLogsForTest(false) - err := Retry(context.Background(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func", args.I) return errors.New("custom fatal error") @@ -364,7 +365,7 @@ func TestRetryLogging(t *testing.T) { return true }, exported.RetryOptions{ RetryDelay: time.Microsecond, - }) + }, nil) require.EqualError(t, err, "custom fatal error") require.Equal(t, []string{ @@ -377,7 +378,7 @@ func TestRetryLogging(t *testing.T) { logsFn := test.CaptureLogsForTest(false) reset := false - err := Retry(context.Background(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func", args.I) if !reset { @@ -398,7 +399,7 @@ func TestRetryLogging(t *testing.T) { return errors.Is(err, &de) }, exported.RetryOptions{ RetryDelay: time.Microsecond, - }) + }, nil) require.Nil(t, err) require.Equal(t, []string{ diff --git a/sdk/messaging/azservicebus/liveTestHelpers_test.go b/sdk/messaging/azservicebus/liveTestHelpers_test.go index 1b63b3d3c052..9bcb34e801a2 100644 --- a/sdk/messaging/azservicebus/liveTestHelpers_test.go +++ b/sdk/messaging/azservicebus/liveTestHelpers_test.go @@ -14,6 +14,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/stretchr/testify/require" ) @@ -186,10 +187,11 @@ func deleteSubscription(t *testing.T, ac *admin.Client, topicName string, subscr // and fails tests otherwise. func peekSingleMessageForTest(t *testing.T, receiver *Receiver) *ReceivedMessage { var msg *ReceivedMessage + // TODO // Peek, unlike Receive, doesn't block until at least one message has arrived, so we have to poll // to get a similar effect. - err := utils.Retry(context.Background(), EventReceiver, "peekSingleForTest", func(ctx context.Context, args *utils.RetryFnArgs) error { + err := utils.Retry(context.Background(), tracing.NewNoOpTracer(), EventReceiver, "peekSingleForTest", func(ctx context.Context, args *utils.RetryFnArgs) error { peekedMessages, err := receiver.PeekMessages(context.Background(), 1, nil) require.NoError(t, err) @@ -201,7 +203,7 @@ func peekSingleMessageForTest(t *testing.T, receiver *Receiver) *ReceivedMessage } }, func(err error) bool { return false - }, RetryOptions{}) + }, RetryOptions{}, nil) require.NoError(t, err) diff --git a/sdk/messaging/azservicebus/messageSettler.go b/sdk/messaging/azservicebus/messageSettler.go index 9b77d06df329..220dd8a7916c 100644 --- a/sdk/messaging/azservicebus/messageSettler.go +++ b/sdk/messaging/azservicebus/messageSettler.go @@ -42,7 +42,7 @@ func (s *messageSettler) settleWithRetries(ctx context.Context, settleFn func(re } return nil - }, RetryOptions{}) + }, RetryOptions{}, nil) return internal.TransformError(err) } diff --git a/sdk/messaging/azservicebus/messageSettler_test.go b/sdk/messaging/azservicebus/messageSettler_test.go index 3cbd7f1372ca..a61eac42f87a 100644 --- a/sdk/messaging/azservicebus/messageSettler_test.go +++ b/sdk/messaging/azservicebus/messageSettler_test.go @@ -8,7 +8,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" - "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" + error2 "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" "github.com/stretchr/testify/require" ) @@ -110,10 +110,10 @@ func TestMessageSettlementUsingReceiverWithReceiveAndDelete(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, messages) - require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.AbandonMessage(ctx, messages[0], nil))) - require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.CompleteMessage(ctx, messages[0], nil))) - require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.DeferMessage(ctx, messages[0], nil))) - require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.DeadLetterMessage(ctx, messages[0], nil))) + require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.AbandonMessage(ctx, messages[0], nil))) + require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.CompleteMessage(ctx, messages[0], nil))) + require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.DeferMessage(ctx, messages[0], nil))) + require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.DeadLetterMessage(ctx, messages[0], nil))) require.EqualError(t, receiver.DeadLetterMessage(ctx, messages[0], nil), "messages that are received in `ReceiveModeReceiveAndDelete` mode are not settleable") } diff --git a/sdk/messaging/azservicebus/receiver.go b/sdk/messaging/azservicebus/receiver.go index 5f836b3373b4..ae023661e16f 100644 --- a/sdk/messaging/azservicebus/receiver.go +++ b/sdk/messaging/azservicebus/receiver.go @@ -236,7 +236,7 @@ func (r *Receiver) ReceiveDeferredMessages(ctx context.Context, sequenceNumbers } return nil - }, r.retryOptions) + }, r.retryOptions, nil) return receivedMessages, internal.TransformError(err) } @@ -290,7 +290,7 @@ func (r *Receiver) PeekMessages(ctx context.Context, maxMessageCount int, option } return nil - }, r.retryOptions) + }, r.retryOptions, nil) return receivedMessages, internal.TransformError(err) } @@ -314,7 +314,7 @@ func (r *Receiver) RenewMessageLock(ctx context.Context, msg *ReceivedMessage, o msg.LockedUntil = &newExpirationTime[0] return nil - }, r.retryOptions) + }, r.retryOptions, nil) return internal.TransformError(err) } @@ -379,7 +379,7 @@ func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, opt err := r.amqpLinks.Retry(ctx, EventReceiver, "receiveMessages.getlinks", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { linksWithID = lwid return nil - }, r.retryOptions) + }, r.retryOptions, nil) if err != nil { return nil, err diff --git a/sdk/messaging/azservicebus/sender.go b/sdk/messaging/azservicebus/sender.go index f5c0437bbad8..4d201f4fe2c6 100644 --- a/sdk/messaging/azservicebus/sender.go +++ b/sdk/messaging/azservicebus/sender.go @@ -6,7 +6,6 @@ package azservicebus import ( "context" "errors" - "fmt" "time" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" @@ -19,7 +18,6 @@ import ( type ( // Sender is used to send messages as well as schedule them to be delivered at a later date. Sender struct { - tracer tracing.Tracer queueOrTopic string cleanupOnClose func() links internal.AMQPLinks @@ -50,7 +48,7 @@ func (s *Sender) NewMessageBatch(ctx context.Context, options *MessageBatchOptio batch = newMessageBatch(maxBytes) return nil - }, s.retryOptions) + }, s.retryOptions, nil) if err != nil { return nil, internal.TransformError(err) @@ -69,12 +67,7 @@ type SendMessageOptions struct { // - [ErrMessageTooLarge] if the message is larger than the maximum allowed link size. // - An [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendMessage(ctx context.Context, message *Message, options *SendMessageOptions) error { - var err error - ctx, endSpan := s.startSpan(ctx, "SendMessage", tracing.SendOperationName, getSpanAttributesForMessage(message)...) - defer func() { endSpan(err) }() - - err = s.sendMessage(ctx, message) - return err + return s.sendMessage(ctx, message) } // SendAMQPAnnotatedMessageOptions contains optional parameters for the SendAMQPAnnotatedMessage function. @@ -89,12 +82,7 @@ type SendAMQPAnnotatedMessageOptions struct { // - [ErrMessageTooLarge] if the message is larger than the maximum allowed link size. // - An [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendAMQPAnnotatedMessage(ctx context.Context, message *AMQPAnnotatedMessage, options *SendAMQPAnnotatedMessageOptions) error { - var err error - ctx, endSpan := s.startSpan(ctx, "SendAMQPAnnotatedMessage", tracing.SendOperationName) - defer func() { endSpan(err) }() - - err = s.sendMessage(ctx, message) - return err + return s.sendMessage(ctx, message) } // SendMessageBatchOptions contains optional parameters for the SendMessageBatch function. @@ -106,15 +94,10 @@ type SendMessageBatchOptions struct { // Message batches can be created using [Sender.NewMessageBatch]. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendMessageBatch(ctx context.Context, batch *MessageBatch, options *SendMessageBatchOptions) error { - var err error - ctx, endSpan := s.startSpan(ctx, "SendMessageBatch", tracing.SendOperationName, - tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(batch.NumMessages())}, - ) - defer func() { endSpan(err) }() - - err = s.links.Retry(ctx, EventSender, "SendMessageBatch", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { + so := tracing.NewSpanOptions(tracing.SendBatchSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageBatchSpanAttributes(int(batch.NumMessages()))) + err := s.links.Retry(ctx, EventSender, "SendMessageBatch", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, batch.toAMQPMessage(), nil) - }, RetryOptions(s.retryOptions)) + }, RetryOptions(s.retryOptions), so) return internal.TransformError(err) } @@ -129,12 +112,6 @@ type ScheduleMessagesOptions struct { // delivered can be cancelled using `Receiver.CancelScheduleMessage(s)` // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) ScheduleMessages(ctx context.Context, messages []*Message, scheduledEnqueueTime time.Time, options *ScheduleMessagesOptions) ([]int64, error) { - var err error - ctx, endSpan := s.startSpan(ctx, "ScheduleMessages", tracing.ScheduleOperationName, - tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(len(messages))}, - ) - defer func() { endSpan(err) }() - sequenceNumbers, err := scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) return sequenceNumbers, err } @@ -149,17 +126,13 @@ type ScheduleAMQPAnnotatedMessagesOptions struct { // delivered can be cancelled using `Receiver.CancelScheduleMessage(s)` // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) ScheduleAMQPAnnotatedMessages(ctx context.Context, messages []*AMQPAnnotatedMessage, scheduledEnqueueTime time.Time, options *ScheduleAMQPAnnotatedMessagesOptions) ([]int64, error) { - var err error - ctx, endSpan := s.startSpan(ctx, "ScheduleAMQPAnnotatedMessages", tracing.ScheduleOperationName, - tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(len(messages))}, - ) - defer func() { endSpan(err) }() - sequenceNumbers, err := scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) return sequenceNumbers, err } func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links internal.AMQPLinks, retryOptions RetryOptions, messages []T, scheduledEnqueueTime time.Time) ([]int64, error) { + so := tracing.NewSpanOptions(tracing.ScheduleSpanName, setSenderSpanAttributes(links.EntityPath(), tracing.ScheduleOperationName), setMessageBatchSpanAttributes(len(messages))) + var amqpMessages []*amqp.Message for _, m := range messages { @@ -176,7 +149,7 @@ func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links intern } sequenceNumbers = sn return nil - }, retryOptions) + }, retryOptions, so) return sequenceNumbers, internal.TransformError(err) } @@ -191,15 +164,11 @@ type CancelScheduledMessagesOptions struct { // CancelScheduledMessages cancels multiple messages that were scheduled. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) CancelScheduledMessages(ctx context.Context, sequenceNumbers []int64, options *CancelScheduledMessagesOptions) error { - var err error - ctx, endSpan := s.startSpan(ctx, "CancelScheduledMessages", tracing.CancelScheduledOperationName, - tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(len(sequenceNumbers))}, - ) - defer func() { endSpan(err) }() + so := tracing.NewSpanOptions(tracing.CancelScheduledSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.CancelScheduledOperationName), setMessageBatchSpanAttributes(len(sequenceNumbers))) - err = s.links.Retry(ctx, EventSender, "CancelScheduledMessages", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { + err := s.links.Retry(ctx, EventSender, "CancelScheduledMessages", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.CancelScheduledMessages(ctx, lwv.RPC, lwv.Sender.LinkName(), sequenceNumbers) - }, s.retryOptions) + }, s.retryOptions, so) return internal.TransformError(err) } @@ -211,9 +180,11 @@ func (s *Sender) Close(ctx context.Context) error { } func (s *Sender) sendMessage(ctx context.Context, message amqpCompatibleMessage) error { + so := tracing.NewSpanOptions(tracing.SendSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageSpanAttributes(message)) + err := s.links.Retry(ctx, EventSender, "SendMessage", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, message.toAMQPMessage(), nil) - }, RetryOptions(s.retryOptions)) + }, RetryOptions(s.retryOptions), so) if amqpErr := (*amqp.Error)(nil); errors.As(err, &amqpErr) && amqpErr.Condition == amqp.ErrCondMessageSizeExceeded { return ErrMessageTooLarge @@ -238,18 +209,6 @@ func (sender *Sender) createSenderLink(ctx context.Context, session amqpwrap.AMQ return amqpSender, nil, nil } -func (s *Sender) startSpan(ctx context.Context, spanName string, operationName tracing.MessagingOperationName, attrs ...tracing.Attribute) (context.Context, func(error)) { - spanName = fmt.Sprintf("Sender.%s", spanName) - attributes := []tracing.Attribute{ - {Key: tracing.OperationName, Value: string(operationName)}, - {Key: tracing.OperationType, Value: string(tracing.SendOperationType)}, - {Key: tracing.DestinationName, Value: s.queueOrTopic}, - } - attributes = append(attributes, attrs...) - - return tracing.StartSpan(ctx, spanName, s.tracer, &tracing.StartSpanOptions{Attributes: attributes}) -} - type newSenderArgs struct { tracer tracing.Tracer ns internal.NamespaceForAMQPLinks @@ -264,13 +223,13 @@ func newSender(args newSenderArgs) (*Sender, error) { } sender := &Sender{ - tracer: args.tracer, queueOrTopic: args.queueOrTopic, cleanupOnClose: args.cleanupOnClose, retryOptions: args.retryOptions, } sender.links = internal.NewAMQPLinks(internal.NewAMQPLinksArgs{ + Tracer: args.tracer, NS: args.ns, EntityPath: args.queueOrTopic, CreateLinkFunc: sender.createSenderLink, diff --git a/sdk/messaging/azservicebus/sender_unit_test.go b/sdk/messaging/azservicebus/sender_unit_test.go index 3494c70d5b10..b4d9f197afd0 100644 --- a/sdk/messaging/azservicebus/sender_unit_test.go +++ b/sdk/messaging/azservicebus/sender_unit_test.go @@ -46,8 +46,21 @@ func TestSender_UserFacingError(t *testing.T) { var asSBError *Error + sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Sender.SendMessage", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "send"}, + {Key: tracing.OperationType, Value: "send"}, + }, + }).NewTracer("module", "version")) + err = sender.SendMessage(context.Background(), &Message{}, nil) + require.ErrorAs(t, err, &asSBError) + require.Equal(t, CodeConnectionLost, asSBError.Code) + msgID := "testID" - sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -56,12 +69,12 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.MessageID, Value: msgID}, }, - }).NewTracer("module", "version") + }).NewTracer("module", "version")) err = sender.SendMessage(context.Background(), &Message{MessageID: &msgID}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) - sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.CancelScheduledMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -70,12 +83,12 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version") + }).NewTracer("module", "version")) err = sender.CancelScheduledMessages(context.Background(), []int64{1}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) - sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.ScheduleMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -84,7 +97,7 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.BatchMessageCount, Value: int64(0)}, }, - }).NewTracer("module", "version") + }).NewTracer("module", "version")) seqNumbers, err := sender.ScheduleMessages(context.Background(), []*Message{}, time.Now(), nil) require.Empty(t, seqNumbers) require.ErrorAs(t, err, &asSBError) @@ -99,7 +112,7 @@ func TestSender_UserFacingError(t *testing.T) { }, nil) require.NoError(t, err) - sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessageBatch", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -108,7 +121,7 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version") + }).NewTracer("module", "version")) err = sender.SendMessageBatch(context.Background(), batch, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) diff --git a/sdk/messaging/azservicebus/session_receiver.go b/sdk/messaging/azservicebus/session_receiver.go index c4cac54b1bb2..30342fd89d1f 100644 --- a/sdk/messaging/azservicebus/session_receiver.go +++ b/sdk/messaging/azservicebus/session_receiver.go @@ -227,7 +227,7 @@ func (sr *SessionReceiver) GetSessionState(ctx context.Context, options *GetSess sessionState = s return nil - }, sr.inner.retryOptions) + }, sr.inner.retryOptions, nil) return sessionState, internal.TransformError(err) } @@ -243,7 +243,7 @@ type SetSessionStateOptions struct { func (sr *SessionReceiver) SetSessionState(ctx context.Context, state []byte, options *SetSessionStateOptions) error { err := sr.inner.amqpLinks.Retry(ctx, EventReceiver, "SetSessionState", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.SetSessionState(ctx, lwv.RPC, lwv.Receiver.LinkName(), sr.SessionID(), state) - }, sr.inner.retryOptions) + }, sr.inner.retryOptions, nil) return internal.TransformError(err) } @@ -266,7 +266,7 @@ func (sr *SessionReceiver) RenewSessionLock(ctx context.Context, options *RenewS sr.lockedUntil = newLockedUntil return nil - }, sr.inner.retryOptions) + }, sr.inner.retryOptions, nil) return internal.TransformError(err) } diff --git a/sdk/messaging/azservicebus/session_receiver_test.go b/sdk/messaging/azservicebus/session_receiver_test.go index 2809d574a564..58c5136c625e 100644 --- a/sdk/messaging/azservicebus/session_receiver_test.go +++ b/sdk/messaging/azservicebus/session_receiver_test.go @@ -13,7 +13,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin" - "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" + internal_errors "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" "github.com/Azure/go-amqp" "github.com/stretchr/testify/require" @@ -139,7 +139,7 @@ func TestSessionReceiver_acceptSessionButAlreadyLocked(t *testing.T) { // messages where the lock token is not a predefined value) receiver, err = client.AcceptSessionForQueue(ctx, queueName, "session-1", nil) - require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(err)) + require.EqualValues(t, internal_errors.RecoveryKindFatal, internal_errors.GetRecoveryKind(err)) require.Nil(t, receiver) } diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go index 25bc7a3614c7..6071100aa199 100644 --- a/sdk/messaging/azservicebus/tracing.go +++ b/sdk/messaging/azservicebus/tracing.go @@ -28,15 +28,37 @@ func newTracer(provider tracing.Provider, hostName string) tracing.Tracer { return tracer } -func getSpanAttributesForMessage(message *Message) []tracing.Attribute { - attrs := []tracing.Attribute{} - if message != nil { - if message.MessageID != nil { - attrs = append(attrs, tracing.Attribute{Key: tracing.MessageID, Value: *message.MessageID}) - } - if message.CorrelationID != nil { - attrs = append(attrs, tracing.Attribute{Key: tracing.ConversationID, Value: *message.CorrelationID}) +func setSenderSpanAttributes(queueOrTopic string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + attrs = append(attrs, + tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}, + tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SendOperationType)}, + tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}, + ) + return attrs + } +} + +func setMessageSpanAttributes(message amqpCompatibleMessage) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + if message != nil { + amqpMessage := message.toAMQPMessage() + if amqpMessage != nil && amqpMessage.Properties != nil { + if amqpMessage.Properties.MessageID != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.MessageID, Value: amqpMessage.Properties.MessageID}) + } + if amqpMessage.Properties.CorrelationID != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.ConversationID, Value: amqpMessage.Properties.CorrelationID}) + } + } } + return attrs + } +} + +func setMessageBatchSpanAttributes(size int) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + attrs = append(attrs, tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(size)}) + return attrs } - return attrs } diff --git a/sdk/messaging/azservicebus/tracing_test.go b/sdk/messaging/azservicebus/tracing_test.go index 35095749a189..4bbdd9d61c9b 100644 --- a/sdk/messaging/azservicebus/tracing_test.go +++ b/sdk/messaging/azservicebus/tracing_test.go @@ -26,23 +26,6 @@ func TestNewTracer(t *testing.T) { require.NotNil(t, tracer) require.True(t, tracer.Enabled()) - _, endSpan := tracing.StartSpan(context.Background(), "TestSpan", tracer, nil) + _, endSpan := tracing.StartSpan(context.Background(), tracer, tracing.NewSpanOptions("TestSpan")) defer func() { endSpan(nil) }() } - -func TestSpanAttributesForMessage(t *testing.T) { - attrs := getSpanAttributesForMessage(nil) - require.Empty(t, attrs) - - msgID := "messageID" - message := &Message{ - MessageID: &msgID, - } - attrs = getSpanAttributesForMessage(message) - require.Equal(t, 1, len(attrs)) - - correlationID := "correlationID" - message.CorrelationID = &correlationID - attrs = getSpanAttributesForMessage(message) - require.Equal(t, 2, len(attrs)) -} From 3d07a0a3251ab78b53f5725f458f1591f7cc7ca0 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Wed, 11 Dec 2024 22:29:47 -0800 Subject: [PATCH 07/15] reverting some files --- sdk/internal/go.mod | 10 +++++----- sdk/internal/go.sum | 8 -------- sdk/messaging/azservicebus/go.mod | 3 --- sdk/messaging/azservicebus/go.sum | 4 ++-- 4 files changed, 7 insertions(+), 18 deletions(-) diff --git a/sdk/internal/go.mod b/sdk/internal/go.mod index 5408fb314683..744ba0aee7bc 100644 --- a/sdk/internal/go.mod +++ b/sdk/internal/go.mod @@ -3,11 +3,11 @@ module github.com/Azure/azure-sdk-for-go/sdk/internal go 1.18 require ( - github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 github.com/stretchr/testify v1.9.0 - golang.org/x/net v0.29.0 - golang.org/x/text v0.18.0 + golang.org/x/net v0.27.0 + golang.org/x/text v0.16.0 ) require ( @@ -20,8 +20,8 @@ require ( github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rogpeppe/go-internal v1.12.0 // indirect - golang.org/x/crypto v0.27.0 // indirect - golang.org/x/sys v0.25.0 // indirect + golang.org/x/crypto v0.25.0 // indirect + golang.org/x/sys v0.22.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/sdk/internal/go.sum b/sdk/internal/go.sum index e6e67c7b21bf..230708319d8f 100644 --- a/sdk/internal/go.sum +++ b/sdk/internal/go.sum @@ -1,7 +1,5 @@ github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 h1:GJHeeA2N7xrG3q30L2UXDyuWRzDM900/65j70wcM4Ww= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0/go.mod h1:l38EPgmsp71HHLq9j7De57JcKOWPyhrsW1Awm1JS6K0= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BLisIzM9dG1R50zUk9C/M= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0/go.mod h1:YL1xnZ6QejvQHWJrX/AvhFl4WW4rqHVoKspWNVwFk0M= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 h1:tfLQ34V6F7tVSwoTf/4lH5sE0o6eCJuNDTmH09nDpbc= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0/go.mod h1:9kIvujWAA58nmPmWB1m23fyWic1kYZMxD9CxaWn4Qpg= github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mxXfQidrMEnLlPk9UMeRtyBTnEFtxkV0kU= @@ -34,19 +32,13 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= -golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70= golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys= golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= -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/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= -golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= -golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= diff --git a/sdk/messaging/azservicebus/go.mod b/sdk/messaging/azservicebus/go.mod index 72e4a3f7309c..1ddb84768278 100644 --- a/sdk/messaging/azservicebus/go.mod +++ b/sdk/messaging/azservicebus/go.mod @@ -40,6 +40,3 @@ require ( golang.org/x/text v0.18.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) - -// TODO: remove this -replace github.com/Azure/azure-sdk-for-go/sdk/internal => github.com/karenychen/azure-sdk-for-go/sdk/internal v0.0.0-20241211234035-1be63f7dd659 diff --git a/sdk/messaging/azservicebus/go.sum b/sdk/messaging/azservicebus/go.sum index 5461add4c7f5..6e3fa14a700a 100644 --- a/sdk/messaging/azservicebus/go.sum +++ b/sdk/messaging/azservicebus/go.sum @@ -2,6 +2,8 @@ github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BL github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0/go.mod h1:YL1xnZ6QejvQHWJrX/AvhFl4WW4rqHVoKspWNVwFk0M= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 h1:tfLQ34V6F7tVSwoTf/4lH5sE0o6eCJuNDTmH09nDpbc= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0/go.mod h1:9kIvujWAA58nmPmWB1m23fyWic1kYZMxD9CxaWn4Qpg= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY= github.com/Azure/go-amqp v1.1.0 h1:XUhx5f4lZFVf6LQc5kBUFECW0iJW9VLxKCYrBeGwl0U= github.com/Azure/go-amqp v1.1.0/go.mod h1:vZAogwdrkbyK3Mla8m/CxSc/aKdnTZ4IbPxl51Y5WZE= github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mxXfQidrMEnLlPk9UMeRtyBTnEFtxkV0kU= @@ -19,8 +21,6 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= -github.com/karenychen/azure-sdk-for-go/sdk/internal v0.0.0-20241211234035-1be63f7dd659 h1:E84nd0UapmfpTQkN8aN2yCGgVtCd/WU1aP4MNeA95PQ= -github.com/karenychen/azure-sdk-for-go/sdk/internal v0.0.0-20241211234035-1be63f7dd659/go.mod h1:XA5QmAjGSl2hLWSxyUSl0pWSxmXATjchXnEl53alEUQ= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= From 1bb68ed433f09e145c0fff04a5d5c04bc2bb573a Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 12 Dec 2024 00:35:33 -0800 Subject: [PATCH 08/15] added receiver traces and some UT --- sdk/messaging/azservicebus/client_test.go | 4 +- .../azservicebus/internal/amqpLinks.go | 13 ++- .../azservicebus/internal/amqp_test_utils.go | 28 ++++-- .../internal/tracing/constants.go | 28 ++++-- .../azservicebus/internal/tracing/tracing.go | 20 ++--- .../azservicebus/internal/utils/retrier.go | 4 +- sdk/messaging/azservicebus/messageSettler.go | 26 ++++-- sdk/messaging/azservicebus/receiver.go | 27 ++++-- .../azservicebus/receiver_simulated_test.go | 85 +++++++++++++++++++ .../azservicebus/receiver_unit_test.go | 28 ++++++ sdk/messaging/azservicebus/sender.go | 16 ++-- sdk/messaging/azservicebus/tracing.go | 55 +++++++++++- sdk/messaging/azservicebus/tracing_test.go | 2 +- 13 files changed, 279 insertions(+), 57 deletions(-) diff --git a/sdk/messaging/azservicebus/client_test.go b/sdk/messaging/azservicebus/client_test.go index 8973ebac34e7..d0083993f86a 100644 --- a/sdk/messaging/azservicebus/client_test.go +++ b/sdk/messaging/azservicebus/client_test.go @@ -497,7 +497,7 @@ func TestNewClientUnitTests(t *testing.T) { require.True(t, client.tracer.Enabled()) // ensure attributes are set up correctly. - _, endSpan := tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanOptions("TestSpan")) + _, endSpan := tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanConfig("TestSpan")) endSpan(nil) // attributes should be set up when using a connection string. @@ -510,7 +510,7 @@ func TestNewClientUnitTests(t *testing.T) { require.True(t, client.tracer.Enabled()) // ensure attributes are set up correctly. - _, endSpan = tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanOptions("TestSpan")) + _, endSpan = tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanConfig("TestSpan")) endSpan(nil) }) diff --git a/sdk/messaging/azservicebus/internal/amqpLinks.go b/sdk/messaging/azservicebus/internal/amqpLinks.go index f381befba623..3cd7d24dcd7d 100644 --- a/sdk/messaging/azservicebus/internal/amqpLinks.go +++ b/sdk/messaging/azservicebus/internal/amqpLinks.go @@ -43,7 +43,7 @@ type AMQPLinks interface { Get(ctx context.Context) (*LinksWithID, error) // Retry will run your callback, recovering links when necessary. - Retry(ctx context.Context, name log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, so *tracing.SpanOptions) error + Retry(ctx context.Context, name log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, sc *tracing.SpanConfig) error // RecoverIfNeeded will check if an error requires recovery, and will recover // the link or, possibly, the connection. @@ -68,6 +68,9 @@ type AMQPLinks interface { // Prefix is the current logging prefix, usable for logging and continuity. Prefix() string + // Tracer returns the tracer for the AMQPLinks instance. + Tracer() tracing.Tracer + // SetTracer sets the tracer for the AMQPLinks instance. SetTracer(tracing.Tracer) } @@ -153,6 +156,10 @@ func NewAMQPLinks(args NewAMQPLinksArgs) AMQPLinks { return l } +func (links *AMQPLinksImpl) Tracer() tracing.Tracer { + return links.tracer +} + func (links *AMQPLinksImpl) SetTracer(tracer tracing.Tracer) { links.tracer = tracer } @@ -327,7 +334,7 @@ func (l *AMQPLinksImpl) Get(ctx context.Context) (*LinksWithID, error) { }, nil } -func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, so *tracing.SpanOptions) error { +func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, sc *tracing.SpanConfig) error { var lastID LinkID didQuickRetry := false @@ -378,7 +385,7 @@ func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, oper } return nil - }, isFatalErrorFunc, o, so) + }, isFatalErrorFunc, o, sc) } // EntityPath is the full entity path for the queue/topic/subscription. diff --git a/sdk/messaging/azservicebus/internal/amqp_test_utils.go b/sdk/messaging/azservicebus/internal/amqp_test_utils.go index a78756934d15..052e65b92825 100644 --- a/sdk/messaging/azservicebus/internal/amqp_test_utils.go +++ b/sdk/messaging/azservicebus/internal/amqp_test_utils.go @@ -43,6 +43,8 @@ type FakeAMQPSession struct { type FakeAMQPLinks struct { AMQPLinks + Tr tracing.Tracer + Closed int CloseIfNeededCalled int @@ -86,13 +88,8 @@ type FakeAMQPReceiver struct { } type FakeRPCLink struct { - Tracer tracing.Tracer - Resp *amqpwrap.RPCResponse - Error error -} - -func (r *FakeRPCLink) SetTracer(tracer tracing.Tracer) { - r.Tracer = tracer + Resp *amqpwrap.RPCResponse + Error error } func (r *FakeRPCLink) Close(ctx context.Context) error { @@ -204,14 +201,19 @@ func (l *FakeAMQPLinks) Get(ctx context.Context) (*LinksWithID, error) { } } -func (l *FakeAMQPLinks) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, so *tracing.SpanOptions) error { +func (l *FakeAMQPLinks) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, sc *tracing.SpanConfig) error { + var err error + ctx, endSpan := tracing.StartSpan(ctx, l.Tr, sc) + defer func() { endSpan(err) }() + lwr, err := l.Get(ctx) if err != nil { return err } - return fn(ctx, lwr, &utils.RetryFnArgs{}) + err = fn(ctx, lwr, &utils.RetryFnArgs{}) + return err } func (l *FakeAMQPLinks) Writef(evt azlog.Event, format string, args ...any) { @@ -222,6 +224,14 @@ func (l *FakeAMQPLinks) Prefix() string { return "prefix" } +func (l *FakeAMQPLinks) Tracer() tracing.Tracer { + return l.Tr +} + +func (l *FakeAMQPLinks) SetTracer(t tracing.Tracer) { + l.Tr = t +} + func (l *FakeAMQPLinks) Close(ctx context.Context, permanently bool) error { if permanently { l.permanently = true diff --git a/sdk/messaging/azservicebus/internal/tracing/constants.go b/sdk/messaging/azservicebus/internal/tracing/constants.go index 5b0b3232108b..2a13881db354 100644 --- a/sdk/messaging/azservicebus/internal/tracing/constants.go +++ b/sdk/messaging/azservicebus/internal/tracing/constants.go @@ -3,13 +3,23 @@ package tracing -type MessagingSpanName string +type SpanName string const ( - SendSpanName MessagingSpanName = "Sender.SendMessage" - SendBatchSpanName MessagingSpanName = "Sender.SendMessageBatch" - ScheduleSpanName MessagingSpanName = "Sender.ScheduleMessages" - CancelScheduledSpanName MessagingSpanName = "Sender.CancelScheduledMessages" + SendSpanName SpanName = "Sender.SendMessage" + SendBatchSpanName SpanName = "Sender.SendMessageBatch" + ScheduleSpanName SpanName = "Sender.ScheduleMessages" + CancelScheduledSpanName SpanName = "Sender.CancelScheduledMessages" + + ReceiveSpanName SpanName = "Receiver.ReceiveMessages" + PeekSpanName SpanName = "Receiver.PeekMessages" + ReceiveDeferredSpanName SpanName = "Receiver.ReceiveDeferredMessages" + RenewMessageLockSpanName SpanName = "Receiver.RenewMessageLock" + + CompleteSpanName SpanName = "Receiver.CompleteMessage" + AbandonSpanName SpanName = "Receiver.AbandonMessage" + DeferSpanName SpanName = "Receiver.DeferMessage" + DeadLetterSpanName SpanName = "Receiver.DeadLetterMessage" ) // OTel-specific messaging attributes @@ -51,6 +61,10 @@ const ( AbandonOperationName MessagingOperationName = "abandon" CompleteOperationName MessagingOperationName = "complete" DeferOperationName MessagingOperationName = "defer" - DeadLetterOperationName MessagingOperationName = "deadletter" - DeleteOperationName MessagingOperationName = "delete" + DeadLetterOperationName MessagingOperationName = "dead_letter" + + AcceptSessionOperationName MessagingOperationName = "accept_session" + GetSessionStateOperationName MessagingOperationName = "get_session_state" + SetSessionStateOperationName MessagingOperationName = "set_session_state" + RenewSessionLockOperationName MessagingOperationName = "renew_session_lock" ) diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go index 5b6f61f3a2fd..8abbe5cbcc77 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microscft Corporation. All rights reserved. // Licensed under the MIT License. package tracing @@ -19,28 +19,28 @@ func NewNoOpTracer() Tracer { return Tracer{} } -type SpanOptions struct { - name MessagingSpanName +type SpanConfig struct { + name SpanName attributes []Attribute } type SetAttributesFn func([]Attribute) []Attribute -func NewSpanOptions(name MessagingSpanName, options ...SetAttributesFn) *SpanOptions { - so := &SpanOptions{name: name} +func NewSpanConfig(name SpanName, options ...SetAttributesFn) *SpanConfig { + sc := &SpanConfig{name: name} for _, fn := range options { - so.attributes = fn(so.attributes) + sc.attributes = fn(sc.attributes) } - return so + return sc } // StartSpan creates a span with the specified name and attributes. // If no span name is provided, no span is created. -func StartSpan(ctx context.Context, tracer Tracer, so *SpanOptions) (context.Context, func(error)) { - if so == nil || so.name == "" { +func StartSpan(ctx context.Context, tracer Tracer, sc *SpanConfig) (context.Context, func(error)) { + if sc == nil || sc.name == "" { return ctx, func(error) {} } - return runtime.StartSpan(ctx, string(so.name), tracer, &StartSpanOptions{Attributes: so.attributes}) + return runtime.StartSpan(ctx, string(sc.name), tracer, &StartSpanOptions{Attributes: sc.attributes}) } type StartSpanOptions = runtime.StartSpanOptions diff --git a/sdk/messaging/azservicebus/internal/utils/retrier.go b/sdk/messaging/azservicebus/internal/utils/retrier.go index 33da126293da..63ca84cc21a4 100644 --- a/sdk/messaging/azservicebus/internal/utils/retrier.go +++ b/sdk/messaging/azservicebus/internal/utils/retrier.go @@ -38,7 +38,7 @@ func (rf *RetryFnArgs) ResetAttempts() { // Retry runs a standard retry loop. It executes your passed in fn as the body of the loop. // It returns if it exceeds the number of configured retry options or if 'isFatal' returns true. -func Retry(ctx context.Context, tracer tracing.Tracer, eventName log.Event, operation string, fn func(ctx context.Context, args *RetryFnArgs) error, isFatalFn func(err error) bool, o exported.RetryOptions, so *tracing.SpanOptions) error { +func Retry(ctx context.Context, tracer tracing.Tracer, eventName log.Event, operation string, fn func(ctx context.Context, args *RetryFnArgs) error, isFatalFn func(err error) bool, o exported.RetryOptions, sc *tracing.SpanConfig) error { if isFatalFn == nil { panic("isFatalFn is nil, errors would panic") } @@ -47,7 +47,7 @@ func Retry(ctx context.Context, tracer tracing.Tracer, eventName log.Event, oper setDefaults(&ro) var err error - ctx, endSpan := tracing.StartSpan(ctx, tracer, so) + ctx, endSpan := tracing.StartSpan(ctx, tracer, sc) defer func() { endSpan(err) }() for i := int32(0); i <= ro.MaxRetries; i++ { diff --git a/sdk/messaging/azservicebus/messageSettler.go b/sdk/messaging/azservicebus/messageSettler.go index 220dd8a7916c..1eb285705201 100644 --- a/sdk/messaging/azservicebus/messageSettler.go +++ b/sdk/messaging/azservicebus/messageSettler.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -31,7 +32,7 @@ func newMessageSettler(links internal.AMQPLinks, retryOptions RetryOptions) *mes } } -func (s *messageSettler) settleWithRetries(ctx context.Context, settleFn func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error) error { +func (s *messageSettler) settleWithRetries(ctx context.Context, sc *tracing.SpanConfig, settleFn func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error) error { if s == nil { return internal.NewErrNonRetriable("messages that are received in `ReceiveModeReceiveAndDelete` mode are not settleable") } @@ -42,7 +43,7 @@ func (s *messageSettler) settleWithRetries(ctx context.Context, settleFn func(re } return nil - }, RetryOptions{}, nil) + }, RetryOptions{}, sc) return internal.TransformError(err) } @@ -54,7 +55,10 @@ type CompleteMessageOptions struct { // CompleteMessage completes a message, deleting it from the queue or subscription. func (ms *messageSettler) CompleteMessage(ctx context.Context, message *ReceivedMessage, options *CompleteMessageOptions) error { - return ms.settleWithRetries(ctx, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + sc := tracing.NewSpanConfig(tracing.CompleteSpanName, + setReceiverSpanAttributes(ms.links.EntityPath(), tracing.CompleteOperationName), + setReceivedMessageSpanAttributes(message)) + return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { var err error if shouldSettleOnReceiver(message) { @@ -85,7 +89,11 @@ type AbandonMessageOptions struct { // This will increment its delivery count, and potentially cause it to be dead lettered // depending on your queue or subscription's configuration. func (ms *messageSettler) AbandonMessage(ctx context.Context, message *ReceivedMessage, options *AbandonMessageOptions) error { - return ms.settleWithRetries(ctx, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + sc := tracing.NewSpanConfig(tracing.AbandonSpanName, + setReceiverSpanAttributes(ms.links.EntityPath(), tracing.AbandonOperationName), + setReceivedMessageSpanAttributes(message)) + + return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { var err error if shouldSettleOnReceiver(message) { @@ -136,7 +144,10 @@ type DeferMessageOptions struct { // DeferMessage will cause a message to be deferred. Deferred messages // can be received using `Receiver.ReceiveDeferredMessages`. func (ms *messageSettler) DeferMessage(ctx context.Context, message *ReceivedMessage, options *DeferMessageOptions) error { - return ms.settleWithRetries(ctx, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + sc := tracing.NewSpanConfig(tracing.DeferSpanName, + setReceiverSpanAttributes(ms.links.EntityPath(), tracing.DeferOperationName), + setReceivedMessageSpanAttributes(message)) + return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { var err error if shouldSettleOnReceiver(message) { @@ -196,7 +207,10 @@ type DeadLetterOptions struct { // queue or subscription. To receive these messages create a receiver with `Client.NewReceiver()` // using the `SubQueue` option. func (ms *messageSettler) DeadLetterMessage(ctx context.Context, message *ReceivedMessage, options *DeadLetterOptions) error { - return ms.settleWithRetries(ctx, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + sc := tracing.NewSpanConfig(tracing.DeadLetterSpanName, + setReceiverSpanAttributes(ms.links.EntityPath(), tracing.DeadLetterOperationName), + setReceivedMessageSpanAttributes(message)) + return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { reason := "" description := "" diff --git a/sdk/messaging/azservicebus/receiver.go b/sdk/messaging/azservicebus/receiver.go index ae023661e16f..a890928e4d26 100644 --- a/sdk/messaging/azservicebus/receiver.go +++ b/sdk/messaging/azservicebus/receiver.go @@ -45,7 +45,6 @@ const ( // Receiver receives messages using pull based functions (ReceiveMessages). type Receiver struct { - tracer tracing.Tracer amqpLinks internal.AMQPLinks cancelReleaser *atomic.Value cleanupOnClose func() @@ -131,7 +130,6 @@ func newReceiver(args newReceiverArgs, options *ReceiverOptions) (*Receiver, err } receiver := &Receiver{ - tracer: args.tracer, cancelReleaser: &atomic.Value{}, cleanupOnClose: args.cleanupOnClose, lastPeekedSequenceNumber: 0, @@ -152,6 +150,7 @@ func newReceiver(args newReceiverArgs, options *ReceiverOptions) (*Receiver, err } receiver.amqpLinks = internal.NewAMQPLinks(internal.NewAMQPLinksArgs{ + Tracer: args.tracer, NS: args.ns, EntityPath: receiver.entityPath, CreateLinkFunc: newLinkFn, @@ -219,6 +218,8 @@ type ReceiveDeferredMessagesOptions struct { // ReceiveDeferredMessages receives messages that were deferred using `Receiver.DeferMessage`. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (r *Receiver) ReceiveDeferredMessages(ctx context.Context, sequenceNumbers []int64, options *ReceiveDeferredMessagesOptions) ([]*ReceivedMessage, error) { + sc := tracing.NewSpanConfig(tracing.ReceiveDeferredSpanName, setReceiverSpanAttributes(r.entityPath, tracing.ReceiveDeferredOperationName), setMessageBatchSpanAttributes(len(sequenceNumbers))) + var receivedMessages []*ReceivedMessage err := r.amqpLinks.Retry(ctx, EventReceiver, "receiveDeferredMessages", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { @@ -236,7 +237,7 @@ func (r *Receiver) ReceiveDeferredMessages(ctx context.Context, sequenceNumbers } return nil - }, r.retryOptions, nil) + }, r.retryOptions, sc) return receivedMessages, internal.TransformError(err) } @@ -261,6 +262,8 @@ type PeekMessagesOptions struct { // // For more information about peeking/message-browsing see https://aka.ms/azsdk/servicebus/message-browsing func (r *Receiver) PeekMessages(ctx context.Context, maxMessageCount int, options *PeekMessagesOptions) ([]*ReceivedMessage, error) { + sc := tracing.NewSpanConfig(tracing.PeekSpanName, setReceiverSpanAttributes(r.entityPath, tracing.PeekOperationName), setMessageBatchSpanAttributes(maxMessageCount)) + var receivedMessages []*ReceivedMessage err := r.amqpLinks.Retry(ctx, EventReceiver, "peekMessages", func(ctx context.Context, links *internal.LinksWithID, args *utils.RetryFnArgs) error { @@ -290,7 +293,7 @@ func (r *Receiver) PeekMessages(ctx context.Context, maxMessageCount int, option } return nil - }, r.retryOptions, nil) + }, r.retryOptions, sc) return receivedMessages, internal.TransformError(err) } @@ -303,6 +306,7 @@ type RenewMessageLockOptions struct { // RenewMessageLock renews the lock on a message, updating the `LockedUntil` field on `msg`. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (r *Receiver) RenewMessageLock(ctx context.Context, msg *ReceivedMessage, options *RenewMessageLockOptions) error { + sc := tracing.NewSpanConfig(tracing.RenewMessageLockSpanName, setReceiverSpanAttributes(r.entityPath, tracing.RenewMessageLockOperationName), setReceivedMessageSpanAttributes(msg)) err := r.amqpLinks.Retry(ctx, EventReceiver, "renewMessageLock", func(ctx context.Context, linksWithVersion *internal.LinksWithID, args *utils.RetryFnArgs) error { newExpirationTime, err := internal.RenewLocks(ctx, linksWithVersion.RPC, msg.linkName, []amqp.UUID{ (amqp.UUID)(msg.LockToken), @@ -314,7 +318,7 @@ func (r *Receiver) RenewMessageLock(ctx context.Context, msg *ReceivedMessage, o msg.LockedUntil = &newExpirationTime[0] return nil - }, r.retryOptions, nil) + }, r.retryOptions, sc) return internal.TransformError(err) } @@ -363,6 +367,12 @@ func (r *Receiver) DeadLetterMessage(ctx context.Context, message *ReceivedMessa } func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, options *ReceiveMessagesOptions) ([]*ReceivedMessage, error) { + var err error + sc := tracing.NewSpanConfig(tracing.ReceiveSpanName, + setReceiverSpanAttributes(r.entityPath, tracing.ReceiveOperationName), setMessageBatchSpanAttributes(maxMessages)) + ctx, endSpan := tracing.StartSpan(ctx, r.amqpLinks.Tracer(), sc) + defer func() { endSpan(err) }() + cancelReleaser := r.cancelReleaser.Swap(emptyCancelFn).(func() string) _ = cancelReleaser() @@ -376,7 +386,7 @@ func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, opt var linksWithID *internal.LinksWithID - err := r.amqpLinks.Retry(ctx, EventReceiver, "receiveMessages.getlinks", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { + err = r.amqpLinks.Retry(ctx, EventReceiver, "receiveMessages.getlinks", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { linksWithID = lwid return nil }, r.retryOptions, nil) @@ -394,7 +404,7 @@ func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, opt if creditsToIssue > 0 { r.amqpLinks.Writef(EventReceiver, "Issuing %d credits, have %d", creditsToIssue, currentReceiverCredits) - if err := linksWithID.Receiver.IssueCredit(uint32(creditsToIssue)); err != nil { + if err = linksWithID.Receiver.IssueCredit(uint32(creditsToIssue)); err != nil { return nil, err } } else { @@ -440,7 +450,8 @@ func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, opt // at that time if len(result.Messages) == 0 { if internal.IsCancelError(result.Error) || rk == internal.RecoveryKindFatal { - return nil, result.Error + err = result.Error + return nil, err } return nil, nil diff --git a/sdk/messaging/azservicebus/receiver_simulated_test.go b/sdk/messaging/azservicebus/receiver_simulated_test.go index 44712d134a9d..7835b98c32dc 100644 --- a/sdk/messaging/azservicebus/receiver_simulated_test.go +++ b/sdk/messaging/azservicebus/receiver_simulated_test.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/mock" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/mock/emulation" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/go-amqp" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -345,18 +346,48 @@ func TestReceiver_UserFacingErrors(t *testing.T) { var asSBError *Error + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.PeekMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "peek"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(1)}, + }, + }).NewTracer("module", "version")) receiveErr = &amqp.LinkError{} messages, err := receiver.PeekMessages(context.Background(), 1, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveDeferredMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "receive_deferred"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(1)}, + }, + }).NewTracer("module", "version")) receiveErr = &amqp.ConnError{} messages, err = receiver.ReceiveDeferredMessages(context.Background(), []int64{1}, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveMessages", + Status: tracing.SpanStatusUnset, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "receive"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(1)}, + }, + }).NewTracer("module", "version")) receiveErr = &amqp.ConnError{} messages, err = receiver.ReceiveMessages(context.Background(), 1, nil) require.NoError(t, err) @@ -376,26 +407,80 @@ func TestReceiver_UserFacingErrors(t *testing.T) { settleOnMgmtLink: true, } + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.AbandonMessage", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "abandon"}, + {Key: tracing.DispositionStatus, Value: "abandon"}, + {Key: tracing.OperationType, Value: "settle"}, + {Key: tracing.DeliveryCount, Value: int64(0)}, + }, + }).NewTracer("module", "version")) err = receiver.AbandonMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.CompleteMessage", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "complete"}, + {Key: tracing.DispositionStatus, Value: "complete"}, + {Key: tracing.OperationType, Value: "settle"}, + {Key: tracing.DeliveryCount, Value: int64(0)}, + }, + }).NewTracer("module", "version")) err = receiver.CompleteMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.DeadLetterMessage", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "dead_letter"}, + {Key: tracing.DispositionStatus, Value: "dead_letter"}, + {Key: tracing.OperationType, Value: "settle"}, + {Key: tracing.DeliveryCount, Value: int64(0)}, + }, + }).NewTracer("module", "version")) err = receiver.DeadLetterMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.DeferMessage", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "defer"}, + {Key: tracing.DispositionStatus, Value: "defer"}, + {Key: tracing.OperationType, Value: "settle"}, + {Key: tracing.DeliveryCount, Value: int64(0)}, + }, + }).NewTracer("module", "version")) err = receiver.DeferMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) + receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.RenewMessageLock", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "renew_message_lock"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.DeliveryCount, Value: int64(0)}, + }, + }).NewTracer("module", "version")) err = receiver.RenewMessageLock(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) diff --git a/sdk/messaging/azservicebus/receiver_unit_test.go b/sdk/messaging/azservicebus/receiver_unit_test.go index af705cd1bb42..9cfa539b3942 100644 --- a/sdk/messaging/azservicebus/receiver_unit_test.go +++ b/sdk/messaging/azservicebus/receiver_unit_test.go @@ -14,12 +14,22 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/go-amqp" "github.com/stretchr/testify/require" ) func TestReceiver_ReceiveMessages_AMQPLinksFailure(t *testing.T) { fakeAMQPLinks := &internal.FakeAMQPLinks{ + Tr: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.OperationName, Value: "receive"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(1)}, + }, + }).NewTracer("module", "version"), Err: internal.NewErrNonRetriable("failed to create links"), } @@ -62,6 +72,15 @@ func TestReceiverCancellationUnitTests(t *testing.T) { t.Run("ImmediatelyCancelled", func(t *testing.T) { r := &Receiver{ amqpLinks: &internal.FakeAMQPLinks{ + Tr: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.OperationName, Value: "receive"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(95)}, + }, + }).NewTracer("module", "version"), Receiver: &internal.FakeAMQPReceiver{}, }, cancelReleaser: &atomic.Value{}, @@ -83,6 +102,15 @@ func TestReceiverCancellationUnitTests(t *testing.T) { r := &Receiver{ amqpLinks: &internal.FakeAMQPLinks{ + Tr: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.OperationName, Value: "receive"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(95)}, + }, + }).NewTracer("module", "version"), Receiver: &internal.FakeAMQPReceiver{ ReceiveFn: func(ctx context.Context) (*amqp.Message, error) { cancel() diff --git a/sdk/messaging/azservicebus/sender.go b/sdk/messaging/azservicebus/sender.go index 4d201f4fe2c6..7777e549a8b4 100644 --- a/sdk/messaging/azservicebus/sender.go +++ b/sdk/messaging/azservicebus/sender.go @@ -94,10 +94,10 @@ type SendMessageBatchOptions struct { // Message batches can be created using [Sender.NewMessageBatch]. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendMessageBatch(ctx context.Context, batch *MessageBatch, options *SendMessageBatchOptions) error { - so := tracing.NewSpanOptions(tracing.SendBatchSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageBatchSpanAttributes(int(batch.NumMessages()))) + sc := tracing.NewSpanConfig(tracing.SendBatchSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageBatchSpanAttributes(int(batch.NumMessages()))) err := s.links.Retry(ctx, EventSender, "SendMessageBatch", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, batch.toAMQPMessage(), nil) - }, RetryOptions(s.retryOptions), so) + }, RetryOptions(s.retryOptions), sc) return internal.TransformError(err) } @@ -131,7 +131,7 @@ func (s *Sender) ScheduleAMQPAnnotatedMessages(ctx context.Context, messages []* } func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links internal.AMQPLinks, retryOptions RetryOptions, messages []T, scheduledEnqueueTime time.Time) ([]int64, error) { - so := tracing.NewSpanOptions(tracing.ScheduleSpanName, setSenderSpanAttributes(links.EntityPath(), tracing.ScheduleOperationName), setMessageBatchSpanAttributes(len(messages))) + sc := tracing.NewSpanConfig(tracing.ScheduleSpanName, setSenderSpanAttributes(links.EntityPath(), tracing.ScheduleOperationName), setMessageBatchSpanAttributes(len(messages))) var amqpMessages []*amqp.Message @@ -149,7 +149,7 @@ func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links intern } sequenceNumbers = sn return nil - }, retryOptions, so) + }, retryOptions, sc) return sequenceNumbers, internal.TransformError(err) } @@ -164,11 +164,11 @@ type CancelScheduledMessagesOptions struct { // CancelScheduledMessages cancels multiple messages that were scheduled. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) CancelScheduledMessages(ctx context.Context, sequenceNumbers []int64, options *CancelScheduledMessagesOptions) error { - so := tracing.NewSpanOptions(tracing.CancelScheduledSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.CancelScheduledOperationName), setMessageBatchSpanAttributes(len(sequenceNumbers))) + sc := tracing.NewSpanConfig(tracing.CancelScheduledSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.CancelScheduledOperationName), setMessageBatchSpanAttributes(len(sequenceNumbers))) err := s.links.Retry(ctx, EventSender, "CancelScheduledMessages", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.CancelScheduledMessages(ctx, lwv.RPC, lwv.Sender.LinkName(), sequenceNumbers) - }, s.retryOptions, so) + }, s.retryOptions, sc) return internal.TransformError(err) } @@ -180,11 +180,11 @@ func (s *Sender) Close(ctx context.Context) error { } func (s *Sender) sendMessage(ctx context.Context, message amqpCompatibleMessage) error { - so := tracing.NewSpanOptions(tracing.SendSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageSpanAttributes(message)) + sc := tracing.NewSpanConfig(tracing.SendSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageSpanAttributes(message)) err := s.links.Retry(ctx, EventSender, "SendMessage", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, message.toAMQPMessage(), nil) - }, RetryOptions(s.retryOptions), so) + }, RetryOptions(s.retryOptions), sc) if amqpErr := (*amqp.Error)(nil); errors.As(err, &amqpErr) && amqpErr.Condition == amqp.ErrCondMessageSizeExceeded { return ErrMessageTooLarge diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go index 6071100aa199..42427da11896 100644 --- a/sdk/messaging/azservicebus/tracing.go +++ b/sdk/messaging/azservicebus/tracing.go @@ -4,6 +4,8 @@ package azservicebus import ( + "strings" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" ) @@ -39,12 +41,35 @@ func setSenderSpanAttributes(queueOrTopic string, operationName tracing.Messagin } } +func setReceiverSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) + + if operationName == tracing.CompleteOperationName || operationName == tracing.AbandonOperationName || + operationName == tracing.DeadLetterOperationName || operationName == tracing.DeferOperationName { + attrs = append(attrs, tracing.Attribute{Key: tracing.DispositionStatus, Value: string(operationName)}) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SettleOperationType)}) + } else { + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.ReceiveOperationType)}) + } + + queueOrTopic, subscription := splitEntityPath(entityPath) + if queueOrTopic != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) + } + if subscription != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.SubscriptionName, Value: subscription}) + } + return attrs + } +} + func setMessageSpanAttributes(message amqpCompatibleMessage) tracing.SetAttributesFn { return func(attrs []tracing.Attribute) []tracing.Attribute { if message != nil { amqpMessage := message.toAMQPMessage() if amqpMessage != nil && amqpMessage.Properties != nil { - if amqpMessage.Properties.MessageID != nil { + if amqpMessage.Properties.MessageID != nil && amqpMessage.Properties.MessageID != "" { attrs = append(attrs, tracing.Attribute{Key: tracing.MessageID, Value: amqpMessage.Properties.MessageID}) } if amqpMessage.Properties.CorrelationID != nil { @@ -56,9 +81,37 @@ func setMessageSpanAttributes(message amqpCompatibleMessage) tracing.SetAttribut } } +func setReceivedMessageSpanAttributes(receivedMessage *ReceivedMessage) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + if receivedMessage != nil { + message := receivedMessage.Message() + attrs = setMessageSpanAttributes(message)(attrs) + attrs = append(attrs, tracing.Attribute{Key: tracing.DeliveryCount, Value: int64(receivedMessage.DeliveryCount)}) + if receivedMessage.EnqueuedTime != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.EnqueuedTime, Value: receivedMessage.EnqueuedTime.Unix()}) + } + } + return attrs + } +} + func setMessageBatchSpanAttributes(size int) tracing.SetAttributesFn { return func(attrs []tracing.Attribute) []tracing.Attribute { attrs = append(attrs, tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(size)}) return attrs } } + +func splitEntityPath(entityPath string) (string, string) { + queueOrTopic := "" + subscription := "" + + path := strings.Split(entityPath, "/") + if len(path) == 1 { + queueOrTopic = path[0] + } else if len(path) == 2 { + queueOrTopic = path[0] + subscription = path[1] + } + return queueOrTopic, subscription +} diff --git a/sdk/messaging/azservicebus/tracing_test.go b/sdk/messaging/azservicebus/tracing_test.go index 4bbdd9d61c9b..7e24dee2f9d7 100644 --- a/sdk/messaging/azservicebus/tracing_test.go +++ b/sdk/messaging/azservicebus/tracing_test.go @@ -26,6 +26,6 @@ func TestNewTracer(t *testing.T) { require.NotNil(t, tracer) require.True(t, tracer.Enabled()) - _, endSpan := tracing.StartSpan(context.Background(), tracer, tracing.NewSpanOptions("TestSpan")) + _, endSpan := tracing.StartSpan(context.Background(), tracer, tracing.NewSpanConfig("TestSpan")) defer func() { endSpan(nil) }() } From e653088157ace921a41dee7f018a83511fc37a0c Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 12 Dec 2024 01:37:26 -0800 Subject: [PATCH 09/15] add session traces --- sdk/messaging/azservicebus/client.go | 9 ++++-- .../internal/tracing/constants.go | 6 ++++ .../azservicebus/receiver_simulated_test.go | 32 +++++++++++++++++++ .../azservicebus/session_receiver.go | 20 +++++++++--- sdk/messaging/azservicebus/tracing.go | 30 +++++++++++++---- 5 files changed, 83 insertions(+), 14 deletions(-) diff --git a/sdk/messaging/azservicebus/client.go b/sdk/messaging/azservicebus/client.go index e7c64ebfb481..3cd767fdd81b 100644 --- a/sdk/messaging/azservicebus/client.go +++ b/sdk/messaging/azservicebus/client.go @@ -175,7 +175,7 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { nsOptions = append(nsOptions, args.NSOptions...) client.namespace, err = internal.NewNamespace(nsOptions...) - client.tracer = newTracer(tracingProvider, getHostName(creds)) + client.tracer = newTracer(tracingProvider, getFullyQualifiedNamespace(creds)) return client, err } @@ -252,6 +252,7 @@ func (client *Client) AcceptSessionForQueue(ctx context.Context, queueName strin sessionReceiver, err := newSessionReceiver( ctx, newSessionReceiverArgs{ + tracer: client.tracer, sessionID: &sessionID, ns: client.namespace, entity: entity{Queue: queueName}, @@ -279,6 +280,7 @@ func (client *Client) AcceptSessionForSubscription(ctx context.Context, topicNam sessionReceiver, err := newSessionReceiver( ctx, newSessionReceiverArgs{ + tracer: client.tracer, sessionID: &sessionID, ns: client.namespace, entity: entity{Topic: topicName, Subscription: subscriptionName}, @@ -348,6 +350,7 @@ func (client *Client) acceptNextSessionForEntity(ctx context.Context, entity ent sessionReceiver, err := newSessionReceiver( ctx, newSessionReceiverArgs{ + tracer: client.tracer, sessionID: nil, ns: client.namespace, entity: entity, @@ -384,10 +387,10 @@ func (client *Client) getCleanupForCloseable() (uint64, func()) { } } -// getHostName returns fullyQualifiedNamespace from clientCreds if it is set. +// getFullyQualifiedNamespace returns fullyQualifiedNamespace from clientCreds if it is set. // Otherwise, it parses the connection string and returns the FullyQualifiedNamespace from it. // If both are empty, it returns an empty string. -func getHostName(creds clientCreds) string { +func getFullyQualifiedNamespace(creds clientCreds) string { if creds.fullyQualifiedNamespace != "" { return creds.fullyQualifiedNamespace } diff --git a/sdk/messaging/azservicebus/internal/tracing/constants.go b/sdk/messaging/azservicebus/internal/tracing/constants.go index 2a13881db354..6751269916eb 100644 --- a/sdk/messaging/azservicebus/internal/tracing/constants.go +++ b/sdk/messaging/azservicebus/internal/tracing/constants.go @@ -20,6 +20,11 @@ const ( AbandonSpanName SpanName = "Receiver.AbandonMessage" DeferSpanName SpanName = "Receiver.DeferMessage" DeadLetterSpanName SpanName = "Receiver.DeadLetterMessage" + + AcceptSessionSpanName SpanName = "SessionReceiver.AcceptSession" + GetSessionStateSpanName SpanName = "SessionReceiver.GetSessionState" + SetSessionStateSpanName SpanName = "SessionReceiver.SetSessionState" + RenewSessionLockSpanName SpanName = "SessionReceiver.RenewSessionLock" ) // OTel-specific messaging attributes @@ -44,6 +49,7 @@ const ( SendOperationType MessagingOperationType = "send" ReceiveOperationType MessagingOperationType = "receive" SettleOperationType MessagingOperationType = "settle" + SessionOperationType MessagingOperationType = "session" ) type MessagingOperationName string diff --git a/sdk/messaging/azservicebus/receiver_simulated_test.go b/sdk/messaging/azservicebus/receiver_simulated_test.go index 7835b98c32dc..b9332052a446 100644 --- a/sdk/messaging/azservicebus/receiver_simulated_test.go +++ b/sdk/messaging/azservicebus/receiver_simulated_test.go @@ -806,6 +806,14 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { // we'll return valid responses for the mgmt link since we need // that to get a session receiver. + client.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "SessionReceiver.AcceptSession", + Status: tracing.SpanStatusUnset, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "accept_session"}, + {Key: tracing.OperationType, Value: "session"}, + }}).NewTracer("module", "version") receiver, err := client.AcceptSessionForQueue(context.Background(), "queue", "session ID", nil) require.NoError(t, err) @@ -814,15 +822,39 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { lockLost = true + receiver.inner.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "SessionReceiver.GetSessionState", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "get_session_state"}, + {Key: tracing.OperationType, Value: "session"}, + }}).NewTracer("module", "version")) state, err := receiver.GetSessionState(context.Background(), nil) require.Nil(t, state) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) + receiver.inner.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "SessionReceiver.SetSessionState", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "set_session_state"}, + {Key: tracing.OperationType, Value: "session"}, + }}).NewTracer("module", "version")) err = receiver.SetSessionState(context.Background(), []byte{}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) + receiver.inner.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "SessionReceiver.RenewSessionLock", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "queue"}, + {Key: tracing.OperationName, Value: "renew_session_lock"}, + {Key: tracing.OperationType, Value: "session"}, + }}).NewTracer("module", "version")) err = receiver.RenewSessionLock(context.Background(), nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) diff --git a/sdk/messaging/azservicebus/session_receiver.go b/sdk/messaging/azservicebus/session_receiver.go index 30342fd89d1f..f1afad9687a2 100644 --- a/sdk/messaging/azservicebus/session_receiver.go +++ b/sdk/messaging/azservicebus/session_receiver.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/amqpwrap" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -51,6 +52,7 @@ func toReceiverOptions(sropts *SessionReceiverOptions) *ReceiverOptions { } type newSessionReceiverArgs struct { + tracer tracing.Tracer sessionID *string ns internal.NamespaceForAMQPLinks entity entity @@ -66,6 +68,7 @@ func newSessionReceiver(ctx context.Context, args newSessionReceiverArgs, option } r, err := newReceiver(newReceiverArgs{ + tracer: args.tracer, ns: args.ns, entity: args.entity, cleanupOnClose: args.cleanupOnClose, @@ -216,6 +219,8 @@ type GetSessionStateOptions struct { // GetSessionState retrieves state associated with the session. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (sr *SessionReceiver) GetSessionState(ctx context.Context, options *GetSessionStateOptions) ([]byte, error) { + sc := tracing.NewSpanConfig(tracing.GetSessionStateSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.GetSessionStateOperationName)) + var sessionState []byte err := sr.inner.amqpLinks.Retry(ctx, EventReceiver, "GetSessionState", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { @@ -227,7 +232,7 @@ func (sr *SessionReceiver) GetSessionState(ctx context.Context, options *GetSess sessionState = s return nil - }, sr.inner.retryOptions, nil) + }, sr.inner.retryOptions, sc) return sessionState, internal.TransformError(err) } @@ -241,9 +246,10 @@ type SetSessionStateOptions struct { // Pass nil for the state parameter to clear the stored session state. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (sr *SessionReceiver) SetSessionState(ctx context.Context, state []byte, options *SetSessionStateOptions) error { + sc := tracing.NewSpanConfig(tracing.SetSessionStateSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.SetSessionStateOperationName)) err := sr.inner.amqpLinks.Retry(ctx, EventReceiver, "SetSessionState", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.SetSessionState(ctx, lwv.RPC, lwv.Receiver.LinkName(), sr.SessionID(), state) - }, sr.inner.retryOptions, nil) + }, sr.inner.retryOptions, sc) return internal.TransformError(err) } @@ -257,6 +263,7 @@ type RenewSessionLockOptions struct { // using `LockedUntil`. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (sr *SessionReceiver) RenewSessionLock(ctx context.Context, options *RenewSessionLockOptions) error { + sc := tracing.NewSpanConfig(tracing.RenewSessionLockSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.RenewSessionLockOperationName)) err := sr.inner.amqpLinks.Retry(ctx, EventReceiver, "RenewSessionLock", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { newLockedUntil, err := internal.RenewSessionLock(ctx, lwv.RPC, lwv.Receiver.LinkName(), *sr.sessionID) @@ -266,14 +273,19 @@ func (sr *SessionReceiver) RenewSessionLock(ctx context.Context, options *RenewS sr.lockedUntil = newLockedUntil return nil - }, sr.inner.retryOptions, nil) + }, sr.inner.retryOptions, sc) return internal.TransformError(err) } // init ensures the link was created, guaranteeing that we get our expected session lock. func (sr *SessionReceiver) init(ctx context.Context) error { + var err error + sc := tracing.NewSpanConfig(tracing.AcceptSessionSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.AcceptSessionOperationName)) + ctx, endSpan := tracing.StartSpan(ctx, sr.inner.amqpLinks.Tracer(), sc) + defer func() { endSpan(err) }() + // initialize the links - _, err := sr.inner.amqpLinks.Get(ctx) + _, err = sr.inner.amqpLinks.Get(ctx) return internal.TransformError(err) } diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go index 42427da11896..c849e6344326 100644 --- a/sdk/messaging/azservicebus/tracing.go +++ b/sdk/messaging/azservicebus/tracing.go @@ -43,6 +43,7 @@ func setSenderSpanAttributes(queueOrTopic string, operationName tracing.Messagin func setReceiverSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { return func(attrs []tracing.Attribute) []tracing.Attribute { + attrs = setEntityPathAttributes(entityPath)(attrs) attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) if operationName == tracing.CompleteOperationName || operationName == tracing.AbandonOperationName || @@ -53,13 +54,15 @@ func setReceiverSpanAttributes(entityPath string, operationName tracing.Messagin attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.ReceiveOperationType)}) } - queueOrTopic, subscription := splitEntityPath(entityPath) - if queueOrTopic != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) - } - if subscription != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.SubscriptionName, Value: subscription}) - } + return attrs + } +} + +func setSessionSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + attrs = setEntityPathAttributes(entityPath)(attrs) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SessionOperationType)}) return attrs } } @@ -102,6 +105,19 @@ func setMessageBatchSpanAttributes(size int) tracing.SetAttributesFn { } } +func setEntityPathAttributes(entityPath string) tracing.SetAttributesFn { + return func(attrs []tracing.Attribute) []tracing.Attribute { + queueOrTopic, subscription := splitEntityPath(entityPath) + if queueOrTopic != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) + } + if subscription != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.SubscriptionName, Value: subscription}) + } + return attrs + } +} + func splitEntityPath(entityPath string) (string, string) { queueOrTopic := "" subscription := "" From 992d9b9fcee1d26e880b5c29d57a6e2046e4cfed Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 12 Dec 2024 10:45:44 -0800 Subject: [PATCH 10/15] linting --- sdk/messaging/azservicebus/internal/namespace.go | 1 - sdk/messaging/azservicebus/internal/tracing/tracing.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/messaging/azservicebus/internal/namespace.go b/sdk/messaging/azservicebus/internal/namespace.go index 72bd130e7e85..4302da4a4f20 100644 --- a/sdk/messaging/azservicebus/internal/namespace.go +++ b/sdk/messaging/azservicebus/internal/namespace.go @@ -420,7 +420,6 @@ func (ns *Namespace) startNegotiateClaimRenewer(ctx context.Context, return case <-time.After(nextClaimAt): for { - // use a no err := utils.Retry(refreshCtx, tracing.NewNoOpTracer(), exported.EventAuth, "NegotiateClaimRefresh", func(ctx context.Context, args *utils.RetryFnArgs) error { tmpExpiresOn, err := refreshClaim(ctx) diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go index 8abbe5cbcc77..23fe35f278fe 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -1,4 +1,4 @@ -// Copyright (c) Microscft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. package tracing From af668bfd0263a84c6c06a32cd4ee9ddbe3513a18 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 12 Dec 2024 12:24:39 -0800 Subject: [PATCH 11/15] reverting some files --- sdk/messaging/azservicebus/messageSettler_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/messaging/azservicebus/messageSettler_test.go b/sdk/messaging/azservicebus/messageSettler_test.go index a61eac42f87a..3cbd7f1372ca 100644 --- a/sdk/messaging/azservicebus/messageSettler_test.go +++ b/sdk/messaging/azservicebus/messageSettler_test.go @@ -8,7 +8,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" - error2 "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" "github.com/stretchr/testify/require" ) @@ -110,10 +110,10 @@ func TestMessageSettlementUsingReceiverWithReceiveAndDelete(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, messages) - require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.AbandonMessage(ctx, messages[0], nil))) - require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.CompleteMessage(ctx, messages[0], nil))) - require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.DeferMessage(ctx, messages[0], nil))) - require.EqualValues(t, error2.RecoveryKindFatal, error2.GetRecoveryKind(receiver.DeadLetterMessage(ctx, messages[0], nil))) + require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.AbandonMessage(ctx, messages[0], nil))) + require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.CompleteMessage(ctx, messages[0], nil))) + require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.DeferMessage(ctx, messages[0], nil))) + require.EqualValues(t, internal.RecoveryKindFatal, internal.GetRecoveryKind(receiver.DeadLetterMessage(ctx, messages[0], nil))) require.EqualError(t, receiver.DeadLetterMessage(ctx, messages[0], nil), "messages that are received in `ReceiveModeReceiveAndDelete` mode are not settleable") } From bd50b2a1e4c72b1d22ba11314d315d939796c201 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Mon, 13 Jan 2025 13:14:09 -0800 Subject: [PATCH 12/15] linting --- sdk/messaging/azservicebus/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/messaging/azservicebus/client_test.go b/sdk/messaging/azservicebus/client_test.go index ff0b13938412..6c10e7c4ae38 100644 --- a/sdk/messaging/azservicebus/client_test.go +++ b/sdk/messaging/azservicebus/client_test.go @@ -19,8 +19,8 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/sas" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" - "github.com/coder/websocket" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" + "github.com/coder/websocket" "github.com/stretchr/testify/require" ) From ab81102ebb44fe15f1e9739d7b4e88b43efe3acd Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 16 Jan 2025 17:58:04 -0800 Subject: [PATCH 13/15] move tracer to sender/receiver level, refractor SetAttrFn patter, add unit test --- sdk/messaging/azservicebus/client_test.go | 10 +- .../azservicebus/internal/amqpLinks.go | 26 +--- .../azservicebus/internal/amqp_test_utils.go | 14 +- .../azservicebus/internal/namespace.go | 3 +- .../azservicebus/internal/tracing/tracing.go | 29 ++-- .../azservicebus/internal/utils/retrier.go | 6 +- .../internal/utils/retrier_test.go | 27 ++-- .../azservicebus/liveTestHelpers_test.go | 3 +- sdk/messaging/azservicebus/messageSettler.go | 57 ++++--- sdk/messaging/azservicebus/receiver.go | 48 ++++-- .../azservicebus/receiver_simulated_test.go | 44 +++--- .../azservicebus/receiver_unit_test.go | 46 +++--- sdk/messaging/azservicebus/sender.go | 51 +++++-- .../azservicebus/sender_unit_test.go | 20 +-- .../azservicebus/session_receiver.go | 33 +++- sdk/messaging/azservicebus/tracing.go | 118 +++++++------- sdk/messaging/azservicebus/tracing_test.go | 144 +++++++++++++++++- 17 files changed, 429 insertions(+), 250 deletions(-) diff --git a/sdk/messaging/azservicebus/client_test.go b/sdk/messaging/azservicebus/client_test.go index 6c10e7c4ae38..405b866c478e 100644 --- a/sdk/messaging/azservicebus/client_test.go +++ b/sdk/messaging/azservicebus/client_test.go @@ -497,7 +497,10 @@ func TestNewClientUnitTests(t *testing.T) { require.True(t, client.tracer.Enabled()) // ensure attributes are set up correctly. - _, endSpan := tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanConfig("TestSpan")) + _, endSpan := tracing.StartSpan(context.Background(), &tracing.TracerOptions{ + Tracer: client.tracer, + SpanName: "TestSpan", + }) endSpan(nil) // attributes should be set up when using a connection string. @@ -510,7 +513,10 @@ func TestNewClientUnitTests(t *testing.T) { require.True(t, client.tracer.Enabled()) // ensure attributes are set up correctly. - _, endSpan = tracing.StartSpan(context.Background(), client.tracer, tracing.NewSpanConfig("TestSpan")) + _, endSpan = tracing.StartSpan(context.Background(), &tracing.TracerOptions{ + Tracer: client.tracer, + SpanName: "TestSpan", + }) endSpan(nil) }) diff --git a/sdk/messaging/azservicebus/internal/amqpLinks.go b/sdk/messaging/azservicebus/internal/amqpLinks.go index 3cd7d24dcd7d..4b2b7013b95e 100644 --- a/sdk/messaging/azservicebus/internal/amqpLinks.go +++ b/sdk/messaging/azservicebus/internal/amqpLinks.go @@ -43,7 +43,7 @@ type AMQPLinks interface { Get(ctx context.Context) (*LinksWithID, error) // Retry will run your callback, recovering links when necessary. - Retry(ctx context.Context, name log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, sc *tracing.SpanConfig) error + Retry(ctx context.Context, name log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, to *tracing.TracerOptions) error // RecoverIfNeeded will check if an error requires recovery, and will recover // the link or, possibly, the connection. @@ -67,12 +67,6 @@ type AMQPLinks interface { // Prefix is the current logging prefix, usable for logging and continuity. Prefix() string - - // Tracer returns the tracer for the AMQPLinks instance. - Tracer() tracing.Tracer - - // SetTracer sets the tracer for the AMQPLinks instance. - SetTracer(tracing.Tracer) } // AMQPLinksImpl manages the set of AMQP links (and detritus) typically needed to work @@ -92,8 +86,6 @@ type AMQPLinksImpl struct { // PR: https://github.com/Azure/azure-sdk-for-go/pull/16847 id LinkID - tracer tracing.Tracer - entityPath string managementPath string audience string @@ -131,7 +123,6 @@ type AMQPLinksImpl struct { type CreateLinkFunc func(ctx context.Context, session amqpwrap.AMQPSession) (amqpwrap.AMQPSenderCloser, amqpwrap.AMQPReceiverCloser, error) type NewAMQPLinksArgs struct { - Tracer tracing.Tracer NS NamespaceForAMQPLinks EntityPath string CreateLinkFunc CreateLinkFunc @@ -142,7 +133,6 @@ type NewAMQPLinksArgs struct { // management link for a specific entity path. func NewAMQPLinks(args NewAMQPLinksArgs) AMQPLinks { l := &AMQPLinksImpl{ - tracer: args.Tracer, entityPath: args.EntityPath, managementPath: fmt.Sprintf("%s/$management", args.EntityPath), audience: args.NS.GetEntityAudience(args.EntityPath), @@ -156,14 +146,6 @@ func NewAMQPLinks(args NewAMQPLinksArgs) AMQPLinks { return l } -func (links *AMQPLinksImpl) Tracer() tracing.Tracer { - return links.tracer -} - -func (links *AMQPLinksImpl) SetTracer(tracer tracing.Tracer) { - links.tracer = tracer -} - // ManagementPath is the management path for the associated entity. func (links *AMQPLinksImpl) ManagementPath() string { return links.managementPath @@ -334,7 +316,7 @@ func (l *AMQPLinksImpl) Get(ctx context.Context) (*LinksWithID, error) { }, nil } -func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, sc *tracing.SpanConfig) error { +func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, to *tracing.TracerOptions) error { var lastID LinkID didQuickRetry := false @@ -343,7 +325,7 @@ func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, oper return links.getRecoveryKindFunc(err) == RecoveryKindFatal } - return utils.Retry(ctx, links.tracer, eventName, links.Prefix()+"("+operation+")", func(ctx context.Context, args *utils.RetryFnArgs) error { + return utils.Retry(ctx, eventName, links.Prefix()+"("+operation+")", func(ctx context.Context, args *utils.RetryFnArgs) error { if err := links.RecoverIfNeeded(ctx, lastID, args.LastErr); err != nil { return err } @@ -385,7 +367,7 @@ func (links *AMQPLinksImpl) Retry(ctx context.Context, eventName log.Event, oper } return nil - }, isFatalErrorFunc, o, sc) + }, isFatalErrorFunc, o, to) } // EntityPath is the full entity path for the queue/topic/subscription. diff --git a/sdk/messaging/azservicebus/internal/amqp_test_utils.go b/sdk/messaging/azservicebus/internal/amqp_test_utils.go index 052e65b92825..7a8fefb1ec41 100644 --- a/sdk/messaging/azservicebus/internal/amqp_test_utils.go +++ b/sdk/messaging/azservicebus/internal/amqp_test_utils.go @@ -43,8 +43,6 @@ type FakeAMQPSession struct { type FakeAMQPLinks struct { AMQPLinks - Tr tracing.Tracer - Closed int CloseIfNeededCalled int @@ -201,9 +199,9 @@ func (l *FakeAMQPLinks) Get(ctx context.Context) (*LinksWithID, error) { } } -func (l *FakeAMQPLinks) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, sc *tracing.SpanConfig) error { +func (l *FakeAMQPLinks) Retry(ctx context.Context, eventName log.Event, operation string, fn RetryWithLinksFn, o exported.RetryOptions, to *tracing.TracerOptions) error { var err error - ctx, endSpan := tracing.StartSpan(ctx, l.Tr, sc) + ctx, endSpan := tracing.StartSpan(ctx, to) defer func() { endSpan(err) }() lwr, err := l.Get(ctx) @@ -224,14 +222,6 @@ func (l *FakeAMQPLinks) Prefix() string { return "prefix" } -func (l *FakeAMQPLinks) Tracer() tracing.Tracer { - return l.Tr -} - -func (l *FakeAMQPLinks) SetTracer(t tracing.Tracer) { - l.Tr = t -} - func (l *FakeAMQPLinks) Close(ctx context.Context, permanently bool) error { if permanently { l.permanently = true diff --git a/sdk/messaging/azservicebus/internal/namespace.go b/sdk/messaging/azservicebus/internal/namespace.go index ca58de0756e1..f8af8a073e7f 100644 --- a/sdk/messaging/azservicebus/internal/namespace.go +++ b/sdk/messaging/azservicebus/internal/namespace.go @@ -21,7 +21,6 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/conn" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/sbauth" - "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -434,7 +433,7 @@ func (ns *Namespace) startNegotiateClaimRenewer(ctx context.Context, return case <-time.After(nextClaimAt): for { - err := utils.Retry(refreshCtx, tracing.NewNoOpTracer(), exported.EventAuth, "NegotiateClaimRefresh", func(ctx context.Context, args *utils.RetryFnArgs) error { + err := utils.Retry(refreshCtx, exported.EventAuth, "NegotiateClaimRefresh", func(ctx context.Context, args *utils.RetryFnArgs) error { tmpExpiresOn, err := refreshClaim(ctx) if err != nil { diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go index 23fe35f278fe..ba9aa77732ce 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -15,32 +15,21 @@ type SpanKind = tracing.SpanKind type Tracer = tracing.Tracer type Provider = tracing.Provider -func NewNoOpTracer() Tracer { - return Tracer{} +func NewNoOpTracer() *Tracer { + return &Tracer{} } -type SpanConfig struct { - name SpanName - attributes []Attribute -} - -type SetAttributesFn func([]Attribute) []Attribute - -func NewSpanConfig(name SpanName, options ...SetAttributesFn) *SpanConfig { - sc := &SpanConfig{name: name} - for _, fn := range options { - sc.attributes = fn(sc.attributes) - } - return sc +type TracerOptions struct { + Tracer Tracer + SpanName SpanName + Attributes []Attribute } // StartSpan creates a span with the specified name and attributes. // If no span name is provided, no span is created. -func StartSpan(ctx context.Context, tracer Tracer, sc *SpanConfig) (context.Context, func(error)) { - if sc == nil || sc.name == "" { +func StartSpan(ctx context.Context, options *TracerOptions) (context.Context, func(error)) { + if options == nil || options.SpanName == "" { return ctx, func(error) {} } - return runtime.StartSpan(ctx, string(sc.name), tracer, &StartSpanOptions{Attributes: sc.attributes}) + return runtime.StartSpan(ctx, string(options.SpanName), options.Tracer, &runtime.StartSpanOptions{Attributes: options.Attributes}) } - -type StartSpanOptions = runtime.StartSpanOptions diff --git a/sdk/messaging/azservicebus/internal/utils/retrier.go b/sdk/messaging/azservicebus/internal/utils/retrier.go index 63ca84cc21a4..b587fe1c75c6 100644 --- a/sdk/messaging/azservicebus/internal/utils/retrier.go +++ b/sdk/messaging/azservicebus/internal/utils/retrier.go @@ -38,7 +38,7 @@ func (rf *RetryFnArgs) ResetAttempts() { // Retry runs a standard retry loop. It executes your passed in fn as the body of the loop. // It returns if it exceeds the number of configured retry options or if 'isFatal' returns true. -func Retry(ctx context.Context, tracer tracing.Tracer, eventName log.Event, operation string, fn func(ctx context.Context, args *RetryFnArgs) error, isFatalFn func(err error) bool, o exported.RetryOptions, sc *tracing.SpanConfig) error { +func Retry(ctx context.Context, eventName log.Event, operation string, fn func(ctx context.Context, args *RetryFnArgs) error, isFatalFn func(err error) bool, o exported.RetryOptions, to *tracing.TracerOptions) error { if isFatalFn == nil { panic("isFatalFn is nil, errors would panic") } @@ -47,7 +47,7 @@ func Retry(ctx context.Context, tracer tracing.Tracer, eventName log.Event, oper setDefaults(&ro) var err error - ctx, endSpan := tracing.StartSpan(ctx, tracer, sc) + ctx, endSpan := tracing.StartSpan(ctx, to) defer func() { endSpan(err) }() for i := int32(0); i <= ro.MaxRetries; i++ { @@ -119,7 +119,7 @@ func setDefaults(o *exported.RetryOptions) { } } -// (adapted from from azcore/policy_retry) +// (adapted from azcore/policy_retry) func calcDelay(o exported.RetryOptions, try int32) time.Duration { // try is >=1; never 0 // avoid overflow when shifting left factor := time.Duration(math.MaxInt64) diff --git a/sdk/messaging/azservicebus/internal/utils/retrier_test.go b/sdk/messaging/azservicebus/internal/utils/retrier_test.go index 04f3be003f0a..3322df83066f 100644 --- a/sdk/messaging/azservicebus/internal/utils/retrier_test.go +++ b/sdk/messaging/azservicebus/internal/utils/retrier_test.go @@ -15,7 +15,6 @@ import ( azlog "github.com/Azure/azure-sdk-for-go/sdk/internal/log" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" - "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/go-amqp" "github.com/stretchr/testify/require" ) @@ -28,7 +27,7 @@ func TestRetrier(t *testing.T) { called := 0 - err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return nil }, func(err error) bool { @@ -52,7 +51,7 @@ func TestRetrier(t *testing.T) { return false } - err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ if args.I == 3 { @@ -79,7 +78,7 @@ func TestRetrier(t *testing.T) { return true } - err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return errors.New("isFatalFn says this is a fatal error") }, isFatalFn, exported.RetryOptions{}, nil) @@ -97,7 +96,7 @@ func TestRetrier(t *testing.T) { maxRetries := int32(2) - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { actualAttempts = append(actualAttempts, args.I) if len(actualAttempts) == int(maxRetries+1) { @@ -130,7 +129,7 @@ func TestRetrier(t *testing.T) { called := 0 - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return errors.New("whatever") }, isFatalFn, customRetryOptions, nil) @@ -150,7 +149,7 @@ func TestCancellationCancelsSleep(t *testing.T) { called := 0 - err := Retry(ctx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(ctx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ return errors.New("try again") }, isFatalFn, exported.RetryOptions{ @@ -174,7 +173,7 @@ func TestCancellationFromUserFunc(t *testing.T) { called := 0 - err := Retry(alreadyCancelledCtx, tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(alreadyCancelledCtx, testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ select { @@ -198,7 +197,7 @@ func TestCancellationTimeoutsArentPropagatedToUser(t *testing.T) { tryAgainErr := errors.New("try again") called := 0 - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "notused", func(ctx context.Context, args *RetryFnArgs) error { called++ require.NoError(t, ctx.Err(), "our sleep/timeout doesn't show up for users") return tryAgainErr @@ -292,7 +291,7 @@ func TestRetryLogging(t *testing.T) { t.Run("normal error", func(t *testing.T) { logsFn := test.CaptureLogsForTest(false) - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func, returning error hello", args.I) return errors.New("hello") }, func(err error) bool { @@ -323,7 +322,7 @@ func TestRetryLogging(t *testing.T) { t.Run("normal error2", func(t *testing.T) { test.EnableStdoutLogging(t) - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "(my_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func, returning error hello", args.I) return errors.New("hello") }, func(err error) bool { @@ -337,7 +336,7 @@ func TestRetryLogging(t *testing.T) { t.Run("cancellation error", func(t *testing.T) { logsFn := test.CaptureLogsForTest(false) - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func", args.I) return context.Canceled @@ -357,7 +356,7 @@ func TestRetryLogging(t *testing.T) { t.Run("custom fatal error", func(t *testing.T) { logsFn := test.CaptureLogsForTest(false) - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func", args.I) return errors.New("custom fatal error") @@ -378,7 +377,7 @@ func TestRetryLogging(t *testing.T) { logsFn := test.CaptureLogsForTest(false) reset := false - err := Retry(context.Background(), tracing.NewNoOpTracer(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { + err := Retry(context.Background(), testLogEvent, "(test_operation)", func(ctx context.Context, args *RetryFnArgs) error { azlog.Writef("TestFunc", "Attempt %d, within test func", args.I) if !reset { diff --git a/sdk/messaging/azservicebus/liveTestHelpers_test.go b/sdk/messaging/azservicebus/liveTestHelpers_test.go index 9bcb34e801a2..2a583cb0a546 100644 --- a/sdk/messaging/azservicebus/liveTestHelpers_test.go +++ b/sdk/messaging/azservicebus/liveTestHelpers_test.go @@ -14,7 +14,6 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/admin" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" - "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/stretchr/testify/require" ) @@ -191,7 +190,7 @@ func peekSingleMessageForTest(t *testing.T, receiver *Receiver) *ReceivedMessage // Peek, unlike Receive, doesn't block until at least one message has arrived, so we have to poll // to get a similar effect. - err := utils.Retry(context.Background(), tracing.NewNoOpTracer(), EventReceiver, "peekSingleForTest", func(ctx context.Context, args *utils.RetryFnArgs) error { + err := utils.Retry(context.Background(), EventReceiver, "peekSingleForTest", func(ctx context.Context, args *utils.RetryFnArgs) error { peekedMessages, err := receiver.PeekMessages(context.Background(), 1, nil) require.NoError(t, err) diff --git a/sdk/messaging/azservicebus/messageSettler.go b/sdk/messaging/azservicebus/messageSettler.go index 1eb285705201..aba03e5e4025 100644 --- a/sdk/messaging/azservicebus/messageSettler.go +++ b/sdk/messaging/azservicebus/messageSettler.go @@ -15,6 +15,7 @@ import ( ) type messageSettler struct { + tracer tracing.Tracer links internal.AMQPLinks retryOptions RetryOptions @@ -23,8 +24,9 @@ type messageSettler struct { notifySettleOnManagement func(message *ReceivedMessage) } -func newMessageSettler(links internal.AMQPLinks, retryOptions RetryOptions) *messageSettler { +func newMessageSettler(tracer tracing.Tracer, links internal.AMQPLinks, retryOptions RetryOptions) *messageSettler { return &messageSettler{ + tracer: tracer, links: links, retryOptions: retryOptions, notifySettleOnLink: func(message *ReceivedMessage) {}, @@ -32,7 +34,7 @@ func newMessageSettler(links internal.AMQPLinks, retryOptions RetryOptions) *mes } } -func (s *messageSettler) settleWithRetries(ctx context.Context, sc *tracing.SpanConfig, settleFn func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error) error { +func (s *messageSettler) settleWithRetries(ctx context.Context, to *tracing.TracerOptions, settleFn func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error) error { if s == nil { return internal.NewErrNonRetriable("messages that are received in `ReceiveModeReceiveAndDelete` mode are not settleable") } @@ -43,7 +45,7 @@ func (s *messageSettler) settleWithRetries(ctx context.Context, sc *tracing.Span } return nil - }, RetryOptions{}, sc) + }, RetryOptions{}, to) return internal.TransformError(err) } @@ -55,10 +57,14 @@ type CompleteMessageOptions struct { // CompleteMessage completes a message, deleting it from the queue or subscription. func (ms *messageSettler) CompleteMessage(ctx context.Context, message *ReceivedMessage, options *CompleteMessageOptions) error { - sc := tracing.NewSpanConfig(tracing.CompleteSpanName, - setReceiverSpanAttributes(ms.links.EntityPath(), tracing.CompleteOperationName), - setReceivedMessageSpanAttributes(message)) - return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + to := &tracing.TracerOptions{ + Tracer: ms.tracer, + SpanName: tracing.CompleteSpanName, + Attributes: append( + getReceiverSpanAttributes(ms.links.EntityPath(), tracing.CompleteOperationName), + getReceivedMessageSpanAttributes(message)...), + } + return ms.settleWithRetries(ctx, to, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { var err error if shouldSettleOnReceiver(message) { @@ -89,11 +95,14 @@ type AbandonMessageOptions struct { // This will increment its delivery count, and potentially cause it to be dead lettered // depending on your queue or subscription's configuration. func (ms *messageSettler) AbandonMessage(ctx context.Context, message *ReceivedMessage, options *AbandonMessageOptions) error { - sc := tracing.NewSpanConfig(tracing.AbandonSpanName, - setReceiverSpanAttributes(ms.links.EntityPath(), tracing.AbandonOperationName), - setReceivedMessageSpanAttributes(message)) - - return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + to := &tracing.TracerOptions{ + Tracer: ms.tracer, + SpanName: tracing.AbandonSpanName, + Attributes: append( + getReceiverSpanAttributes(ms.links.EntityPath(), tracing.AbandonOperationName), + getReceivedMessageSpanAttributes(message)...), + } + return ms.settleWithRetries(ctx, to, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { var err error if shouldSettleOnReceiver(message) { @@ -144,10 +153,14 @@ type DeferMessageOptions struct { // DeferMessage will cause a message to be deferred. Deferred messages // can be received using `Receiver.ReceiveDeferredMessages`. func (ms *messageSettler) DeferMessage(ctx context.Context, message *ReceivedMessage, options *DeferMessageOptions) error { - sc := tracing.NewSpanConfig(tracing.DeferSpanName, - setReceiverSpanAttributes(ms.links.EntityPath(), tracing.DeferOperationName), - setReceivedMessageSpanAttributes(message)) - return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + to := &tracing.TracerOptions{ + Tracer: ms.tracer, + SpanName: tracing.DeferSpanName, + Attributes: append( + getReceiverSpanAttributes(ms.links.EntityPath(), tracing.DeferOperationName), + getReceivedMessageSpanAttributes(message)...), + } + return ms.settleWithRetries(ctx, to, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { var err error if shouldSettleOnReceiver(message) { @@ -207,10 +220,14 @@ type DeadLetterOptions struct { // queue or subscription. To receive these messages create a receiver with `Client.NewReceiver()` // using the `SubQueue` option. func (ms *messageSettler) DeadLetterMessage(ctx context.Context, message *ReceivedMessage, options *DeadLetterOptions) error { - sc := tracing.NewSpanConfig(tracing.DeadLetterSpanName, - setReceiverSpanAttributes(ms.links.EntityPath(), tracing.DeadLetterOperationName), - setReceivedMessageSpanAttributes(message)) - return ms.settleWithRetries(ctx, sc, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { + to := &tracing.TracerOptions{ + Tracer: ms.tracer, + SpanName: tracing.DeadLetterSpanName, + Attributes: append( + getReceiverSpanAttributes(ms.links.EntityPath(), tracing.DeadLetterOperationName), + getReceivedMessageSpanAttributes(message)...), + } + return ms.settleWithRetries(ctx, to, func(receiver amqpwrap.AMQPReceiver, rpcLink amqpwrap.RPCLink) error { reason := "" description := "" diff --git a/sdk/messaging/azservicebus/receiver.go b/sdk/messaging/azservicebus/receiver.go index 98f727e63c17..d5c13f1094f4 100644 --- a/sdk/messaging/azservicebus/receiver.go +++ b/sdk/messaging/azservicebus/receiver.go @@ -45,6 +45,7 @@ const ( // Receiver receives messages using pull based functions (ReceiveMessages). type Receiver struct { + tracer tracing.Tracer amqpLinks internal.AMQPLinks cancelReleaser *atomic.Value cleanupOnClose func() @@ -131,6 +132,7 @@ func newReceiver(args newReceiverArgs, options *ReceiverOptions) (*Receiver, err } receiver := &Receiver{ + tracer: args.tracer, cancelReleaser: &atomic.Value{}, cleanupOnClose: args.cleanupOnClose, lastPeekedSequenceNumber: 0, @@ -152,7 +154,6 @@ func newReceiver(args newReceiverArgs, options *ReceiverOptions) (*Receiver, err } receiver.amqpLinks = internal.NewAMQPLinks(internal.NewAMQPLinksArgs{ - Tracer: args.tracer, NS: args.ns, EntityPath: receiver.entityPath, CreateLinkFunc: newLinkFn, @@ -161,7 +162,7 @@ func newReceiver(args newReceiverArgs, options *ReceiverOptions) (*Receiver, err // 'nil' settler handles returning an error message for receiveAndDelete links. if receiver.receiveMode == ReceiveModePeekLock { - receiver.settler = newMessageSettler(receiver.amqpLinks, receiver.retryOptions) + receiver.settler = newMessageSettler(args.tracer, receiver.amqpLinks, receiver.retryOptions) } else { receiver.settler = (*messageSettler)(nil) } @@ -220,7 +221,13 @@ type ReceiveDeferredMessagesOptions struct { // ReceiveDeferredMessages receives messages that were deferred using `Receiver.DeferMessage`. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (r *Receiver) ReceiveDeferredMessages(ctx context.Context, sequenceNumbers []int64, options *ReceiveDeferredMessagesOptions) ([]*ReceivedMessage, error) { - sc := tracing.NewSpanConfig(tracing.ReceiveDeferredSpanName, setReceiverSpanAttributes(r.entityPath, tracing.ReceiveDeferredOperationName), setMessageBatchSpanAttributes(len(sequenceNumbers))) + to := &tracing.TracerOptions{ + Tracer: r.tracer, + SpanName: tracing.ReceiveDeferredSpanName, + Attributes: append( + getReceiverSpanAttributes(r.entityPath, tracing.ReceiveDeferredOperationName), + getMessageBatchSpanAttributes(len(sequenceNumbers))...), + } var receivedMessages []*ReceivedMessage @@ -239,7 +246,7 @@ func (r *Receiver) ReceiveDeferredMessages(ctx context.Context, sequenceNumbers } return nil - }, r.retryOptions, sc) + }, r.retryOptions, to) return receivedMessages, internal.TransformError(err) } @@ -264,7 +271,13 @@ type PeekMessagesOptions struct { // // For more information about peeking/message-browsing see https://aka.ms/azsdk/servicebus/message-browsing func (r *Receiver) PeekMessages(ctx context.Context, maxMessageCount int, options *PeekMessagesOptions) ([]*ReceivedMessage, error) { - sc := tracing.NewSpanConfig(tracing.PeekSpanName, setReceiverSpanAttributes(r.entityPath, tracing.PeekOperationName), setMessageBatchSpanAttributes(maxMessageCount)) + to := &tracing.TracerOptions{ + Tracer: r.tracer, + SpanName: tracing.PeekSpanName, + Attributes: append( + getReceiverSpanAttributes(r.entityPath, tracing.PeekOperationName), + getMessageBatchSpanAttributes(maxMessageCount)...), + } var receivedMessages []*ReceivedMessage @@ -295,7 +308,7 @@ func (r *Receiver) PeekMessages(ctx context.Context, maxMessageCount int, option } return nil - }, r.retryOptions, sc) + }, r.retryOptions, to) return receivedMessages, internal.TransformError(err) } @@ -308,7 +321,14 @@ type RenewMessageLockOptions struct { // RenewMessageLock renews the lock on a message, updating the `LockedUntil` field on `msg`. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (r *Receiver) RenewMessageLock(ctx context.Context, msg *ReceivedMessage, options *RenewMessageLockOptions) error { - sc := tracing.NewSpanConfig(tracing.RenewMessageLockSpanName, setReceiverSpanAttributes(r.entityPath, tracing.RenewMessageLockOperationName), setReceivedMessageSpanAttributes(msg)) + to := &tracing.TracerOptions{ + Tracer: r.tracer, + SpanName: tracing.RenewMessageLockSpanName, + Attributes: append( + getReceiverSpanAttributes(r.entityPath, tracing.RenewMessageLockOperationName), + getReceivedMessageSpanAttributes(msg)...), + } + err := r.amqpLinks.Retry(ctx, EventReceiver, "renewMessageLock", func(ctx context.Context, linksWithVersion *internal.LinksWithID, args *utils.RetryFnArgs) error { newExpirationTime, err := internal.RenewLocks(ctx, linksWithVersion.RPC, msg.linkName, []amqp.UUID{ (amqp.UUID)(msg.LockToken), @@ -320,7 +340,7 @@ func (r *Receiver) RenewMessageLock(ctx context.Context, msg *ReceivedMessage, o msg.LockedUntil = &newExpirationTime[0] return nil - }, r.retryOptions, sc) + }, r.retryOptions, to) return internal.TransformError(err) } @@ -370,9 +390,15 @@ func (r *Receiver) DeadLetterMessage(ctx context.Context, message *ReceivedMessa func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, options *ReceiveMessagesOptions) ([]*ReceivedMessage, error) { var err error - sc := tracing.NewSpanConfig(tracing.ReceiveSpanName, - setReceiverSpanAttributes(r.entityPath, tracing.ReceiveOperationName), setMessageBatchSpanAttributes(maxMessages)) - ctx, endSpan := tracing.StartSpan(ctx, r.amqpLinks.Tracer(), sc) + to := &tracing.TracerOptions{ + Tracer: r.tracer, + SpanName: tracing.ReceiveSpanName, + Attributes: append( + getReceiverSpanAttributes(r.entityPath, tracing.ReceiveOperationName), + getMessageBatchSpanAttributes(maxMessages)...), + } + + ctx, endSpan := tracing.StartSpan(ctx, to) defer func() { endSpan(err) }() cancelReleaser := r.cancelReleaser.Swap(emptyCancelFn).(func() string) diff --git a/sdk/messaging/azservicebus/receiver_simulated_test.go b/sdk/messaging/azservicebus/receiver_simulated_test.go index b9332052a446..3e1481c22788 100644 --- a/sdk/messaging/azservicebus/receiver_simulated_test.go +++ b/sdk/messaging/azservicebus/receiver_simulated_test.go @@ -346,7 +346,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { var asSBError *Error - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.PeekMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -355,14 +355,14 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "receive"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") receiveErr = &amqp.LinkError{} messages, err := receiver.PeekMessages(context.Background(), 1, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveDeferredMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -371,14 +371,14 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "receive"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") receiveErr = &amqp.ConnError{} messages, err = receiver.ReceiveDeferredMessages(context.Background(), []int64{1}, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveMessages", Status: tracing.SpanStatusUnset, Attributes: []tracing.Attribute{ @@ -387,7 +387,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "receive"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") receiveErr = &amqp.ConnError{} messages, err = receiver.ReceiveMessages(context.Background(), 1, nil) require.NoError(t, err) @@ -407,7 +407,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { settleOnMgmtLink: true, } - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.AbandonMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -417,13 +417,13 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "settle"}, {Key: tracing.DeliveryCount, Value: int64(0)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = receiver.AbandonMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.CompleteMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -433,13 +433,13 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "settle"}, {Key: tracing.DeliveryCount, Value: int64(0)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = receiver.CompleteMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.DeadLetterMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -449,13 +449,13 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "settle"}, {Key: tracing.DeliveryCount, Value: int64(0)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = receiver.DeadLetterMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.DeferMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -465,13 +465,13 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "settle"}, {Key: tracing.DeliveryCount, Value: int64(0)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = receiver.DeferMessage(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) - receiver.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.RenewMessageLock", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -480,7 +480,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { {Key: tracing.OperationType, Value: "receive"}, {Key: tracing.DeliveryCount, Value: int64(0)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = receiver.RenewMessageLock(context.Background(), msg, nil) require.Empty(t, messages) require.ErrorAs(t, err, &asSBError) @@ -822,39 +822,39 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { lockLost = true - receiver.inner.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.inner.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.GetSessionState", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, {Key: tracing.OperationName, Value: "get_session_state"}, {Key: tracing.OperationType, Value: "session"}, - }}).NewTracer("module", "version")) + }}).NewTracer("module", "version") state, err := receiver.GetSessionState(context.Background(), nil) require.Nil(t, state) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) - receiver.inner.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.inner.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.SetSessionState", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, {Key: tracing.OperationName, Value: "set_session_state"}, {Key: tracing.OperationType, Value: "session"}, - }}).NewTracer("module", "version")) + }}).NewTracer("module", "version") err = receiver.SetSessionState(context.Background(), []byte{}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) - receiver.inner.amqpLinks.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + receiver.inner.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.RenewSessionLock", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, {Key: tracing.OperationName, Value: "renew_session_lock"}, {Key: tracing.OperationType, Value: "session"}, - }}).NewTracer("module", "version")) + }}).NewTracer("module", "version") err = receiver.RenewSessionLock(context.Background(), nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeLockLost, asSBError.Code) diff --git a/sdk/messaging/azservicebus/receiver_unit_test.go b/sdk/messaging/azservicebus/receiver_unit_test.go index 79ab0a3a77d4..7a43b8e9bfb5 100644 --- a/sdk/messaging/azservicebus/receiver_unit_test.go +++ b/sdk/messaging/azservicebus/receiver_unit_test.go @@ -23,7 +23,11 @@ import ( func TestReceiver_ReceiveMessages_AMQPLinksFailure(t *testing.T) { fakeAMQPLinks := &internal.FakeAMQPLinks{ - Tr: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Err: internal.NewErrNonRetriable("failed to create links"), + } + + receiver := &Receiver{ + tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -32,10 +36,6 @@ func TestReceiver_ReceiveMessages_AMQPLinksFailure(t *testing.T) { {Key: tracing.BatchMessageCount, Value: int64(1)}, }, }).NewTracer("module", "version"), - Err: internal.NewErrNonRetriable("failed to create links"), - } - - receiver := &Receiver{ amqpLinks: fakeAMQPLinks, receiveMode: ReceiveModePeekLock, // TODO: need to make this test rely less on stubbing. @@ -73,16 +73,16 @@ func ReceiveModeString(mode ReceiveMode) string { func TestReceiverCancellationUnitTests(t *testing.T) { t.Run("ImmediatelyCancelled", func(t *testing.T) { r := &Receiver{ + tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.OperationName, Value: "receive"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(95)}, + }, + }).NewTracer("module", "version"), amqpLinks: &internal.FakeAMQPLinks{ - Tr: tracing.NewSpanValidator(t, tracing.SpanMatcher{ - Name: "Receiver.ReceiveMessages", - Status: tracing.SpanStatusError, - Attributes: []tracing.Attribute{ - {Key: tracing.OperationName, Value: "receive"}, - {Key: tracing.OperationType, Value: "receive"}, - {Key: tracing.BatchMessageCount, Value: int64(95)}, - }, - }).NewTracer("module", "version"), Receiver: &internal.FakeAMQPReceiver{}, }, cancelReleaser: &atomic.Value{}, @@ -103,16 +103,16 @@ func TestReceiverCancellationUnitTests(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) r := &Receiver{ + tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Receiver.ReceiveMessages", + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.OperationName, Value: "receive"}, + {Key: tracing.OperationType, Value: "receive"}, + {Key: tracing.BatchMessageCount, Value: int64(95)}, + }, + }).NewTracer("module", "version"), amqpLinks: &internal.FakeAMQPLinks{ - Tr: tracing.NewSpanValidator(t, tracing.SpanMatcher{ - Name: "Receiver.ReceiveMessages", - Status: tracing.SpanStatusError, - Attributes: []tracing.Attribute{ - {Key: tracing.OperationName, Value: "receive"}, - {Key: tracing.OperationType, Value: "receive"}, - {Key: tracing.BatchMessageCount, Value: int64(95)}, - }, - }).NewTracer("module", "version"), Receiver: &internal.FakeAMQPReceiver{ ReceiveFn: func(ctx context.Context) (*amqp.Message, error) { cancel() diff --git a/sdk/messaging/azservicebus/sender.go b/sdk/messaging/azservicebus/sender.go index 7777e549a8b4..f2c4fa4be418 100644 --- a/sdk/messaging/azservicebus/sender.go +++ b/sdk/messaging/azservicebus/sender.go @@ -18,6 +18,7 @@ import ( type ( // Sender is used to send messages as well as schedule them to be delivered at a later date. Sender struct { + tracer tracing.Tracer queueOrTopic string cleanupOnClose func() links internal.AMQPLinks @@ -94,10 +95,18 @@ type SendMessageBatchOptions struct { // Message batches can be created using [Sender.NewMessageBatch]. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) SendMessageBatch(ctx context.Context, batch *MessageBatch, options *SendMessageBatchOptions) error { - sc := tracing.NewSpanConfig(tracing.SendBatchSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageBatchSpanAttributes(int(batch.NumMessages()))) + to := &tracing.TracerOptions{ + Tracer: s.tracer, + SpanName: tracing.SendBatchSpanName, + Attributes: append( + getSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), + getMessageBatchSpanAttributes(int(batch.NumMessages()))..., + ), + } + err := s.links.Retry(ctx, EventSender, "SendMessageBatch", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, batch.toAMQPMessage(), nil) - }, RetryOptions(s.retryOptions), sc) + }, RetryOptions(s.retryOptions), to) return internal.TransformError(err) } @@ -112,7 +121,7 @@ type ScheduleMessagesOptions struct { // delivered can be cancelled using `Receiver.CancelScheduleMessage(s)` // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) ScheduleMessages(ctx context.Context, messages []*Message, scheduledEnqueueTime time.Time, options *ScheduleMessagesOptions) ([]int64, error) { - sequenceNumbers, err := scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) + sequenceNumbers, err := scheduleMessages(ctx, s.tracer, s.links, s.retryOptions, messages, scheduledEnqueueTime) return sequenceNumbers, err } @@ -126,12 +135,18 @@ type ScheduleAMQPAnnotatedMessagesOptions struct { // delivered can be cancelled using `Receiver.CancelScheduleMessage(s)` // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) ScheduleAMQPAnnotatedMessages(ctx context.Context, messages []*AMQPAnnotatedMessage, scheduledEnqueueTime time.Time, options *ScheduleAMQPAnnotatedMessagesOptions) ([]int64, error) { - sequenceNumbers, err := scheduleMessages(ctx, s.links, s.retryOptions, messages, scheduledEnqueueTime) + sequenceNumbers, err := scheduleMessages(ctx, s.tracer, s.links, s.retryOptions, messages, scheduledEnqueueTime) return sequenceNumbers, err } -func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links internal.AMQPLinks, retryOptions RetryOptions, messages []T, scheduledEnqueueTime time.Time) ([]int64, error) { - sc := tracing.NewSpanConfig(tracing.ScheduleSpanName, setSenderSpanAttributes(links.EntityPath(), tracing.ScheduleOperationName), setMessageBatchSpanAttributes(len(messages))) +func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, tracer tracing.Tracer, links internal.AMQPLinks, retryOptions RetryOptions, messages []T, scheduledEnqueueTime time.Time) ([]int64, error) { + to := &tracing.TracerOptions{ + Tracer: tracer, + SpanName: tracing.ScheduleSpanName, + Attributes: append( + getSenderSpanAttributes(links.EntityPath(), tracing.ScheduleOperationName), + getMessageBatchSpanAttributes(len(messages))...), + } var amqpMessages []*amqp.Message @@ -149,7 +164,7 @@ func scheduleMessages[T amqpCompatibleMessage](ctx context.Context, links intern } sequenceNumbers = sn return nil - }, retryOptions, sc) + }, retryOptions, to) return sequenceNumbers, internal.TransformError(err) } @@ -164,11 +179,17 @@ type CancelScheduledMessagesOptions struct { // CancelScheduledMessages cancels multiple messages that were scheduled. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (s *Sender) CancelScheduledMessages(ctx context.Context, sequenceNumbers []int64, options *CancelScheduledMessagesOptions) error { - sc := tracing.NewSpanConfig(tracing.CancelScheduledSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.CancelScheduledOperationName), setMessageBatchSpanAttributes(len(sequenceNumbers))) + to := &tracing.TracerOptions{ + Tracer: s.tracer, + SpanName: tracing.CancelScheduledSpanName, + Attributes: append( + getSenderSpanAttributes(s.queueOrTopic, tracing.CancelScheduledOperationName), + getMessageBatchSpanAttributes(len(sequenceNumbers))...), + } err := s.links.Retry(ctx, EventSender, "CancelScheduledMessages", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.CancelScheduledMessages(ctx, lwv.RPC, lwv.Sender.LinkName(), sequenceNumbers) - }, s.retryOptions, sc) + }, s.retryOptions, to) return internal.TransformError(err) } @@ -180,11 +201,17 @@ func (s *Sender) Close(ctx context.Context) error { } func (s *Sender) sendMessage(ctx context.Context, message amqpCompatibleMessage) error { - sc := tracing.NewSpanConfig(tracing.SendSpanName, setSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), setMessageSpanAttributes(message)) + to := &tracing.TracerOptions{ + Tracer: s.tracer, + SpanName: tracing.SendSpanName, + Attributes: append( + getSenderSpanAttributes(s.queueOrTopic, tracing.SendOperationName), + getMessageSpanAttributes(message)...), + } err := s.links.Retry(ctx, EventSender, "SendMessage", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { return lwid.Sender.Send(ctx, message.toAMQPMessage(), nil) - }, RetryOptions(s.retryOptions), sc) + }, RetryOptions(s.retryOptions), to) if amqpErr := (*amqp.Error)(nil); errors.As(err, &amqpErr) && amqpErr.Condition == amqp.ErrCondMessageSizeExceeded { return ErrMessageTooLarge @@ -223,13 +250,13 @@ func newSender(args newSenderArgs) (*Sender, error) { } sender := &Sender{ + tracer: args.tracer, queueOrTopic: args.queueOrTopic, cleanupOnClose: args.cleanupOnClose, retryOptions: args.retryOptions, } sender.links = internal.NewAMQPLinks(internal.NewAMQPLinksArgs{ - Tracer: args.tracer, NS: args.ns, EntityPath: args.queueOrTopic, CreateLinkFunc: sender.createSenderLink, diff --git a/sdk/messaging/azservicebus/sender_unit_test.go b/sdk/messaging/azservicebus/sender_unit_test.go index b4d9f197afd0..1c27d11bf78c 100644 --- a/sdk/messaging/azservicebus/sender_unit_test.go +++ b/sdk/messaging/azservicebus/sender_unit_test.go @@ -46,7 +46,7 @@ func TestSender_UserFacingError(t *testing.T) { var asSBError *Error - sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -54,13 +54,13 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationName, Value: "send"}, {Key: tracing.OperationType, Value: "send"}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = sender.SendMessage(context.Background(), &Message{}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) msgID := "testID" - sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessage", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -69,12 +69,12 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.MessageID, Value: msgID}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = sender.SendMessage(context.Background(), &Message{MessageID: &msgID}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) - sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.CancelScheduledMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -83,12 +83,12 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = sender.CancelScheduledMessages(context.Background(), []int64{1}, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) - sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.ScheduleMessages", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -97,7 +97,7 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.BatchMessageCount, Value: int64(0)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") seqNumbers, err := sender.ScheduleMessages(context.Background(), []*Message{}, time.Now(), nil) require.Empty(t, seqNumbers) require.ErrorAs(t, err, &asSBError) @@ -112,7 +112,7 @@ func TestSender_UserFacingError(t *testing.T) { }, nil) require.NoError(t, err) - sender.links.SetTracer(tracing.NewSpanValidator(t, tracing.SpanMatcher{ + sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessageBatch", Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ @@ -121,7 +121,7 @@ func TestSender_UserFacingError(t *testing.T) { {Key: tracing.OperationType, Value: "send"}, {Key: tracing.BatchMessageCount, Value: int64(1)}, }, - }).NewTracer("module", "version")) + }).NewTracer("module", "version") err = sender.SendMessageBatch(context.Background(), batch, nil) require.ErrorAs(t, err, &asSBError) require.Equal(t, CodeConnectionLost, asSBError.Code) diff --git a/sdk/messaging/azservicebus/session_receiver.go b/sdk/messaging/azservicebus/session_receiver.go index f1afad9687a2..e9904ccc9287 100644 --- a/sdk/messaging/azservicebus/session_receiver.go +++ b/sdk/messaging/azservicebus/session_receiver.go @@ -219,7 +219,11 @@ type GetSessionStateOptions struct { // GetSessionState retrieves state associated with the session. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (sr *SessionReceiver) GetSessionState(ctx context.Context, options *GetSessionStateOptions) ([]byte, error) { - sc := tracing.NewSpanConfig(tracing.GetSessionStateSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.GetSessionStateOperationName)) + to := &tracing.TracerOptions{ + Tracer: sr.inner.tracer, + SpanName: tracing.GetSessionStateSpanName, + Attributes: getSessionSpanAttributes(sr.inner.entityPath, tracing.GetSessionStateOperationName), + } var sessionState []byte @@ -232,7 +236,7 @@ func (sr *SessionReceiver) GetSessionState(ctx context.Context, options *GetSess sessionState = s return nil - }, sr.inner.retryOptions, sc) + }, sr.inner.retryOptions, to) return sessionState, internal.TransformError(err) } @@ -246,10 +250,15 @@ type SetSessionStateOptions struct { // Pass nil for the state parameter to clear the stored session state. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (sr *SessionReceiver) SetSessionState(ctx context.Context, state []byte, options *SetSessionStateOptions) error { - sc := tracing.NewSpanConfig(tracing.SetSessionStateSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.SetSessionStateOperationName)) + to := &tracing.TracerOptions{ + Tracer: sr.inner.tracer, + SpanName: tracing.SetSessionStateSpanName, + Attributes: getSessionSpanAttributes(sr.inner.entityPath, tracing.SetSessionStateOperationName), + } + err := sr.inner.amqpLinks.Retry(ctx, EventReceiver, "SetSessionState", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { return internal.SetSessionState(ctx, lwv.RPC, lwv.Receiver.LinkName(), sr.SessionID(), state) - }, sr.inner.retryOptions, sc) + }, sr.inner.retryOptions, to) return internal.TransformError(err) } @@ -263,7 +272,11 @@ type RenewSessionLockOptions struct { // using `LockedUntil`. // If the operation fails it can return an [*azservicebus.Error] type if the failure is actionable. func (sr *SessionReceiver) RenewSessionLock(ctx context.Context, options *RenewSessionLockOptions) error { - sc := tracing.NewSpanConfig(tracing.RenewSessionLockSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.RenewSessionLockOperationName)) + to := &tracing.TracerOptions{ + Tracer: sr.inner.tracer, + SpanName: tracing.RenewSessionLockSpanName, + Attributes: getSessionSpanAttributes(sr.inner.entityPath, tracing.RenewSessionLockOperationName), + } err := sr.inner.amqpLinks.Retry(ctx, EventReceiver, "RenewSessionLock", func(ctx context.Context, lwv *internal.LinksWithID, args *utils.RetryFnArgs) error { newLockedUntil, err := internal.RenewSessionLock(ctx, lwv.RPC, lwv.Receiver.LinkName(), *sr.sessionID) @@ -273,7 +286,7 @@ func (sr *SessionReceiver) RenewSessionLock(ctx context.Context, options *RenewS sr.lockedUntil = newLockedUntil return nil - }, sr.inner.retryOptions, sc) + }, sr.inner.retryOptions, to) return internal.TransformError(err) } @@ -281,8 +294,12 @@ func (sr *SessionReceiver) RenewSessionLock(ctx context.Context, options *RenewS // init ensures the link was created, guaranteeing that we get our expected session lock. func (sr *SessionReceiver) init(ctx context.Context) error { var err error - sc := tracing.NewSpanConfig(tracing.AcceptSessionSpanName, setSessionSpanAttributes(sr.inner.entityPath, tracing.AcceptSessionOperationName)) - ctx, endSpan := tracing.StartSpan(ctx, sr.inner.amqpLinks.Tracer(), sc) + to := &tracing.TracerOptions{ + Tracer: sr.inner.tracer, + SpanName: tracing.AcceptSessionSpanName, + Attributes: getSessionSpanAttributes(sr.inner.entityPath, tracing.AcceptSessionOperationName), + } + ctx, endSpan := tracing.StartSpan(ctx, to) defer func() { endSpan(err) }() // initialize the links diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go index c849e6344326..b4cb8e0949be 100644 --- a/sdk/messaging/azservicebus/tracing.go +++ b/sdk/messaging/azservicebus/tracing.go @@ -30,92 +30,78 @@ func newTracer(provider tracing.Provider, hostName string) tracing.Tracer { return tracer } -func setSenderSpanAttributes(queueOrTopic string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - attrs = append(attrs, - tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}, - tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SendOperationType)}, - tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}, - ) - return attrs - } +func getSenderSpanAttributes(queueOrTopic string, operationName tracing.MessagingOperationName) []tracing.Attribute { + return append([]tracing.Attribute{}, + tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}, + tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SendOperationType)}, + tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}, + ) } -func setReceiverSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - attrs = setEntityPathAttributes(entityPath)(attrs) - attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) - - if operationName == tracing.CompleteOperationName || operationName == tracing.AbandonOperationName || - operationName == tracing.DeadLetterOperationName || operationName == tracing.DeferOperationName { - attrs = append(attrs, tracing.Attribute{Key: tracing.DispositionStatus, Value: string(operationName)}) - attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SettleOperationType)}) - } else { - attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.ReceiveOperationType)}) - } +func getReceiverSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) []tracing.Attribute { + attrs := getEntityPathAttributes(entityPath) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) - return attrs + if operationName == tracing.CompleteOperationName || operationName == tracing.AbandonOperationName || + operationName == tracing.DeadLetterOperationName || operationName == tracing.DeferOperationName { + attrs = append(attrs, tracing.Attribute{Key: tracing.DispositionStatus, Value: string(operationName)}) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SettleOperationType)}) + } else { + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.ReceiveOperationType)}) } + return attrs } -func setSessionSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - attrs = setEntityPathAttributes(entityPath)(attrs) - attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) - attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SessionOperationType)}) - return attrs - } +func getSessionSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) []tracing.Attribute { + attrs := getEntityPathAttributes(entityPath) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) + attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SessionOperationType)}) + return attrs } -func setMessageSpanAttributes(message amqpCompatibleMessage) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - if message != nil { - amqpMessage := message.toAMQPMessage() - if amqpMessage != nil && amqpMessage.Properties != nil { - if amqpMessage.Properties.MessageID != nil && amqpMessage.Properties.MessageID != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.MessageID, Value: amqpMessage.Properties.MessageID}) - } - if amqpMessage.Properties.CorrelationID != nil { - attrs = append(attrs, tracing.Attribute{Key: tracing.ConversationID, Value: amqpMessage.Properties.CorrelationID}) - } +func getMessageSpanAttributes(message amqpCompatibleMessage) []tracing.Attribute { + attrs := []tracing.Attribute{} + if message != nil { + amqpMessage := message.toAMQPMessage() + if amqpMessage != nil && amqpMessage.Properties != nil { + if amqpMessage.Properties.MessageID != nil && amqpMessage.Properties.MessageID != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.MessageID, Value: amqpMessage.Properties.MessageID}) + } + if amqpMessage.Properties.CorrelationID != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.ConversationID, Value: amqpMessage.Properties.CorrelationID}) } } - return attrs } + return attrs } -func setReceivedMessageSpanAttributes(receivedMessage *ReceivedMessage) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - if receivedMessage != nil { - message := receivedMessage.Message() - attrs = setMessageSpanAttributes(message)(attrs) - attrs = append(attrs, tracing.Attribute{Key: tracing.DeliveryCount, Value: int64(receivedMessage.DeliveryCount)}) - if receivedMessage.EnqueuedTime != nil { - attrs = append(attrs, tracing.Attribute{Key: tracing.EnqueuedTime, Value: receivedMessage.EnqueuedTime.Unix()}) - } +func getReceivedMessageSpanAttributes(receivedMessage *ReceivedMessage) []tracing.Attribute { + attrs := []tracing.Attribute{} + if receivedMessage != nil { + message := receivedMessage.Message() + attrs = getMessageSpanAttributes(message) + attrs = append(attrs, tracing.Attribute{Key: tracing.DeliveryCount, Value: int64(receivedMessage.DeliveryCount)}) + if receivedMessage.EnqueuedTime != nil { + attrs = append(attrs, tracing.Attribute{Key: tracing.EnqueuedTime, Value: receivedMessage.EnqueuedTime.Unix()}) } - return attrs } + return attrs } -func setMessageBatchSpanAttributes(size int) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - attrs = append(attrs, tracing.Attribute{Key: tracing.BatchMessageCount, Value: int64(size)}) - return attrs - } +func getMessageBatchSpanAttributes(size int) []tracing.Attribute { + return []tracing.Attribute{{Key: tracing.BatchMessageCount, Value: int64(size)}} } -func setEntityPathAttributes(entityPath string) tracing.SetAttributesFn { - return func(attrs []tracing.Attribute) []tracing.Attribute { - queueOrTopic, subscription := splitEntityPath(entityPath) - if queueOrTopic != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) - } - if subscription != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.SubscriptionName, Value: subscription}) - } - return attrs +func getEntityPathAttributes(entityPath string) []tracing.Attribute { + attrs := []tracing.Attribute{} + queueOrTopic, subscription := splitEntityPath(entityPath) + if queueOrTopic != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) + } + if subscription != "" { + attrs = append(attrs, tracing.Attribute{Key: tracing.SubscriptionName, Value: subscription}) } + return attrs } func splitEntityPath(entityPath string) (string, string) { diff --git a/sdk/messaging/azservicebus/tracing_test.go b/sdk/messaging/azservicebus/tracing_test.go index 7e24dee2f9d7..51e05034135b 100644 --- a/sdk/messaging/azservicebus/tracing_test.go +++ b/sdk/messaging/azservicebus/tracing_test.go @@ -7,6 +7,7 @@ package azservicebus import ( "context" "testing" + "time" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/stretchr/testify/require" @@ -26,6 +27,147 @@ func TestNewTracer(t *testing.T) { require.NotNil(t, tracer) require.True(t, tracer.Enabled()) - _, endSpan := tracing.StartSpan(context.Background(), tracer, tracing.NewSpanConfig("TestSpan")) + _, endSpan := tracing.StartSpan(context.Background(), &tracing.TracerOptions{ + Tracer: tracer, + SpanName: "TestSpan", + }) defer func() { endSpan(nil) }() } + +func TestGetSenderSpanAttributes(t *testing.T) { + queueOrTopic := "queue" + operationName := tracing.SendOperationName + expectedAttrs := []tracing.Attribute{ + {Key: tracing.DestinationName, Value: queueOrTopic}, + {Key: tracing.OperationType, Value: string(tracing.SendOperationType)}, + {Key: tracing.OperationName, Value: string(operationName)}, + } + + result := getSenderSpanAttributes(queueOrTopic, operationName) + require.ElementsMatch(t, expectedAttrs, result) +} + +func TestGetReceiverSpanAttributes(t *testing.T) { + entityPath := "entity" + operationName := tracing.ReceiveOperationName + expectedAttrs := []tracing.Attribute{ + {Key: tracing.DestinationName, Value: entityPath}, + {Key: tracing.OperationName, Value: string(operationName)}, + {Key: tracing.OperationType, Value: string(tracing.ReceiveOperationType)}, + } + + result := getReceiverSpanAttributes(entityPath, operationName) + require.ElementsMatch(t, expectedAttrs, result) +} + +func TestGetSessionSpanAttributes(t *testing.T) { + entityPath := "entity" + operationName := tracing.GetSessionStateOperationName + expectedAttrs := []tracing.Attribute{ + {Key: tracing.DestinationName, Value: entityPath}, + {Key: tracing.OperationName, Value: string(operationName)}, + {Key: tracing.OperationType, Value: string(tracing.SessionOperationType)}, + } + + result := getSessionSpanAttributes(entityPath, operationName) + require.ElementsMatch(t, expectedAttrs, result) +} + +func TestGetMessageSpanAttributes(t *testing.T) { + messageId := "message-id" + correlationId := "correlation-id" + + // can parse empty message + message := &Message{} + expectedAttrs := []tracing.Attribute{} + result := getMessageSpanAttributes(message) + require.ElementsMatch(t, expectedAttrs, result) + + // can parse message with messageId + message = &Message{ + MessageID: &messageId, + } + expectedAttrs = []tracing.Attribute{ + {Key: tracing.MessageID, Value: messageId}, + } + result = getMessageSpanAttributes(message) + require.ElementsMatch(t, expectedAttrs, result) + + // can parse message with correlationId + message = &Message{ + CorrelationID: &correlationId, + } + expectedAttrs = []tracing.Attribute{ + {Key: tracing.ConversationID, Value: correlationId}, + } + result = getMessageSpanAttributes(message) + require.ElementsMatch(t, expectedAttrs, result) + + // can parse message with both messageId and correlationId + message = &Message{ + MessageID: &messageId, + CorrelationID: &correlationId, + } + expectedAttrs = []tracing.Attribute{ + {Key: tracing.MessageID, Value: messageId}, + {Key: tracing.ConversationID, Value: correlationId}, + } + result = getMessageSpanAttributes(message) + require.ElementsMatch(t, expectedAttrs, result) +} + +func TestGetReceivedMessageSpanAttributes(t *testing.T) { + messageId := "message-id" + correlationId := "correlation-id" + deliveryCount := 1 + enqueuedTime := time.Now() + + // can parse empty message + receivedMessage := &ReceivedMessage{} + expectedAttrs := []tracing.Attribute{ + {Key: tracing.DeliveryCount, Value: int64(0)}, + } + result := getReceivedMessageSpanAttributes(receivedMessage) + require.ElementsMatch(t, expectedAttrs, result) + + // can parse message with enqueued time + receivedMessage = &ReceivedMessage{ + MessageID: messageId, + CorrelationID: &correlationId, + DeliveryCount: uint32(deliveryCount), + EnqueuedTime: &enqueuedTime, + } + expectedAttrs = []tracing.Attribute{ + {Key: tracing.MessageID, Value: messageId}, + {Key: tracing.ConversationID, Value: correlationId}, + {Key: tracing.DeliveryCount, Value: int64(1)}, + {Key: tracing.EnqueuedTime, Value: enqueuedTime.Unix()}, + } + result = getReceivedMessageSpanAttributes(receivedMessage) + require.ElementsMatch(t, expectedAttrs, result) +} + +func TestGetMessageBatchSpanAttributes(t *testing.T) { + expectedAttrs := []tracing.Attribute{ + {Key: tracing.BatchMessageCount, Value: int64(1)}, + } + result := getMessageBatchSpanAttributes(1) + require.ElementsMatch(t, expectedAttrs, result) +} + +func TestGetEntityPathAttributes(t *testing.T) { + entityPath := "queue" + expectedAttrs := []tracing.Attribute{ + {Key: tracing.DestinationName, Value: entityPath}, + } + result := getEntityPathAttributes(entityPath) + require.ElementsMatch(t, expectedAttrs, result) + + entityPath = "topic/subscription" + expectedAttrs = []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "topic"}, + {Key: tracing.SubscriptionName, Value: "subscription"}, + } + result = getEntityPathAttributes(entityPath) + require.ElementsMatch(t, expectedAttrs, result) +} From 09f971ea6db0c03e29d0a2bebac7e392abc9f825 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 16 Jan 2025 19:53:17 -0800 Subject: [PATCH 14/15] include span kind in our spans and tests + more unit test --- .../internal/tracing/constants.go | 11 +++ .../azservicebus/internal/tracing/matcher.go | 9 ++- .../internal/tracing/matcher_test.go | 69 +++++++++++++++++++ .../azservicebus/internal/tracing/tracing.go | 20 ++++-- .../internal/tracing/tracing_test.go | 44 ++++++++++++ sdk/messaging/azservicebus/receiver.go | 26 +++---- .../azservicebus/receiver_simulated_test.go | 12 ++++ .../azservicebus/receiver_unit_test.go | 3 + .../azservicebus/sender_unit_test.go | 5 ++ sdk/messaging/azservicebus/tracing.go | 6 +- 10 files changed, 182 insertions(+), 23 deletions(-) create mode 100644 sdk/messaging/azservicebus/internal/tracing/matcher_test.go create mode 100644 sdk/messaging/azservicebus/internal/tracing/tracing_test.go diff --git a/sdk/messaging/azservicebus/internal/tracing/constants.go b/sdk/messaging/azservicebus/internal/tracing/constants.go index 6751269916eb..c673cedca516 100644 --- a/sdk/messaging/azservicebus/internal/tracing/constants.go +++ b/sdk/messaging/azservicebus/internal/tracing/constants.go @@ -3,6 +3,8 @@ package tracing +import "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + type SpanName string const ( @@ -27,6 +29,15 @@ const ( RenewSessionLockSpanName SpanName = "SessionReceiver.RenewSessionLock" ) +type SpanKind = tracing.SpanKind + +const ( + SpanKindInternal tracing.SpanKind = tracing.SpanKindInternal + SpanKindClient tracing.SpanKind = tracing.SpanKindClient + SpanKindProducer tracing.SpanKind = tracing.SpanKindProducer + SpanKindConsumer tracing.SpanKind = tracing.SpanKindConsumer +) + // OTel-specific messaging attributes const ( ServerAddress = "server.address" diff --git a/sdk/messaging/azservicebus/internal/tracing/matcher.go b/sdk/messaging/azservicebus/internal/tracing/matcher.go index f42b7360a393..eea38808216b 100644 --- a/sdk/messaging/azservicebus/internal/tracing/matcher.go +++ b/sdk/messaging/azservicebus/internal/tracing/matcher.go @@ -20,6 +20,7 @@ const ( type SpanStatus = tracing.SpanStatus // NewSpanValidator creates a Provider that verifies a span was created that matches the specified SpanMatcher. +// The returned Provider can be used to create a client with a tracing provider that will validate spans in unit tests. func NewSpanValidator(t *testing.T, matcher SpanMatcher) Provider { return tracing.NewProvider(func(name, version string) Tracer { tt := matchingTracer{ @@ -29,6 +30,9 @@ func NewSpanValidator(t *testing.T, matcher SpanMatcher) Provider { t.Cleanup(func() { require.NotNil(t, tt.match, "didn't find a span with name %s", tt.matcher.Name) require.True(t, tt.match.ended, "span wasn't ended") + if tt.matcher.Kind != 0 { + require.EqualValues(t, tt.matcher.Kind, tt.match.kind, "span kind values don't match") + } require.EqualValues(t, matcher.Status, tt.match.status, "span status values don't match") require.ElementsMatch(t, matcher.Attributes, tt.match.attrs, "span attributes don't match") }) @@ -48,6 +52,7 @@ func NewSpanValidator(t *testing.T, matcher SpanMatcher) Provider { // SpanMatcher contains the values to match when a span is created. type SpanMatcher struct { Name string + Kind SpanKind Status SpanStatus Attributes []Attribute } @@ -57,13 +62,14 @@ type matchingTracer struct { match *span } -func (mt *matchingTracer) Start(ctx context.Context, spanName string, kind tracing.SpanKind, attrs []Attribute) (context.Context, tracing.Span) { +func (mt *matchingTracer) Start(ctx context.Context, spanName string, kind SpanKind, attrs []Attribute) (context.Context, tracing.Span) { if spanName != mt.matcher.Name { return ctx, tracing.Span{} } // span name matches our matcher, track it mt.match = &span{ name: spanName, + kind: kind, attrs: attrs, } return ctx, tracing.NewSpan(tracing.SpanImpl{ @@ -75,6 +81,7 @@ func (mt *matchingTracer) Start(ctx context.Context, spanName string, kind traci type span struct { name string + kind SpanKind status SpanStatus desc string attrs []Attribute diff --git a/sdk/messaging/azservicebus/internal/tracing/matcher_test.go b/sdk/messaging/azservicebus/internal/tracing/matcher_test.go new file mode 100644 index 000000000000..ab4fcd865ace --- /dev/null +++ b/sdk/messaging/azservicebus/internal/tracing/matcher_test.go @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +import ( + "context" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" + "github.com/stretchr/testify/require" +) + +// write unit test for matcher.go + +func TestNewSpanValidator(t *testing.T) { + hostName := "fake.servicebus.windows.net" + provider := NewSpanValidator(t, SpanMatcher{ + Name: "TestSpan", + Kind: SpanKindProducer, + Status: SpanStatusUnset, + Attributes: []Attribute{ + {Key: ServerAddress, Value: hostName}, + }, + }) + tracer := provider.NewTracer("module", "version") + require.NotNil(t, tracer) + require.True(t, tracer.Enabled()) + + _, endSpan := runtime.StartSpan(context.Background(), "TestSpan", tracer, &runtime.StartSpanOptions{ + Kind: tracing.SpanKindProducer, + Attributes: []Attribute{ + {Key: ServerAddress, Value: hostName}, + }, + }) + defer func() { endSpan(nil) }() +} + +func TestMatchingTracerStart(t *testing.T) { + hostName := "fake.servicebus.windows.net" + matcher := SpanMatcher{ + Name: "TestSpan", + Kind: SpanKindProducer, + Status: SpanStatusUnset, + Attributes: []Attribute{ + {Key: MessagingSystem, Value: "servicebus"}, + {Key: ServerAddress, Value: hostName}, + }, + } + tracer := matchingTracer{ + matcher: matcher, + } + ctx := context.Background() + // no-op when SpanName doesn't match + _, spn := tracer.Start(ctx, "BadSpanName", SpanKindProducer, nil) + require.EqualValues(t, spn, tracing.Span{}) + // tracks span when SpanName matches + _, spn = tracer.Start(ctx, "TestSpan", SpanKindProducer, []Attribute{ + {Key: MessagingSystem, Value: "servicebus"}, + {Key: ServerAddress, Value: hostName}, + }) + require.NotNil(t, spn) + spn.SetAttributes(tracing.Attribute{ + Key: "TestAttributeKey", + Value: "TestAttributeValue", + }) + spn.SetStatus(SpanStatusOK, "ok") +} diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go index ba9aa77732ce..782254835d52 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -5,20 +5,16 @@ package tracing import ( "context" + "strings" "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" ) type Attribute = tracing.Attribute -type SpanKind = tracing.SpanKind type Tracer = tracing.Tracer type Provider = tracing.Provider -func NewNoOpTracer() *Tracer { - return &Tracer{} -} - type TracerOptions struct { Tracer Tracer SpanName SpanName @@ -31,5 +27,17 @@ func StartSpan(ctx context.Context, options *TracerOptions) (context.Context, fu if options == nil || options.SpanName == "" { return ctx, func(error) {} } - return runtime.StartSpan(ctx, string(options.SpanName), options.Tracer, &runtime.StartSpanOptions{Attributes: options.Attributes}) + spanKind := SpanKindInternal + spanCaller := strings.Split(string(options.SpanName), ".")[0] + if spanCaller == "Sender" { + spanKind = SpanKindProducer + } else if spanCaller == "Receiver" || spanCaller == "SessionReceiver" { + spanKind = SpanKindConsumer + } + + return runtime.StartSpan(ctx, string(options.SpanName), options.Tracer, + &runtime.StartSpanOptions{ + Kind: spanKind, + Attributes: options.Attributes, + }) } diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing_test.go b/sdk/messaging/azservicebus/internal/tracing/tracing_test.go new file mode 100644 index 000000000000..94fb369bea85 --- /dev/null +++ b/sdk/messaging/azservicebus/internal/tracing/tracing_test.go @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestStartSpan(t *testing.T) { + // no-op when TracerOptions is nil + ctx := context.Background() + subCtx, _ := StartSpan(ctx, nil) + require.Equal(t, ctx, subCtx) + + // no-op when TracerOptions is empty + subCtx, _ = StartSpan(ctx, &TracerOptions{}) + require.Equal(t, ctx, subCtx) + + // no-op when SpanName is empty + subCtx, _ = StartSpan(ctx, &TracerOptions{SpanName: ""}) + require.Equal(t, ctx, subCtx) + + // creates a span when both tracer and SpanName are set + tracer := NewSpanValidator(t, SpanMatcher{ + Name: "test", + Kind: SpanKindInternal, + }).NewTracer("module", "version") + subCtx1, endSpan1 := StartSpan(ctx, &TracerOptions{Tracer: tracer, SpanName: "test"}) + defer endSpan1(nil) + require.NotEqual(t, ctx, subCtx1) + + // creates a span when span name has sender prefix + tracer = NewSpanValidator(t, SpanMatcher{ + Name: "Sender.TestSpan", + Kind: SpanKindProducer, + }).NewTracer("module", "version") + subCtx2, endSpan2 := StartSpan(ctx, &TracerOptions{Tracer: tracer, SpanName: "Sender.TestSpan"}) + defer endSpan2(nil) + require.NotEqual(t, ctx, subCtx2) +} diff --git a/sdk/messaging/azservicebus/receiver.go b/sdk/messaging/azservicebus/receiver.go index d5c13f1094f4..237aa24e9a11 100644 --- a/sdk/messaging/azservicebus/receiver.go +++ b/sdk/messaging/azservicebus/receiver.go @@ -209,6 +209,18 @@ func (r *Receiver) ReceiveMessages(ctx context.Context, maxMessages int, options return nil, errors.New("receiver is already receiving messages. ReceiveMessages() cannot be called concurrently") } + var err error + to := &tracing.TracerOptions{ + Tracer: r.tracer, + SpanName: tracing.ReceiveSpanName, + Attributes: append( + getReceiverSpanAttributes(r.entityPath, tracing.ReceiveOperationName), + getMessageBatchSpanAttributes(maxMessages)...), + } + + ctx, endSpan := tracing.StartSpan(ctx, to) + defer func() { endSpan(err) }() + messages, err := r.receiveMessagesImpl(ctx, maxMessages, options) return messages, internal.TransformError(err) } @@ -389,18 +401,6 @@ func (r *Receiver) DeadLetterMessage(ctx context.Context, message *ReceivedMessa } func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, options *ReceiveMessagesOptions) ([]*ReceivedMessage, error) { - var err error - to := &tracing.TracerOptions{ - Tracer: r.tracer, - SpanName: tracing.ReceiveSpanName, - Attributes: append( - getReceiverSpanAttributes(r.entityPath, tracing.ReceiveOperationName), - getMessageBatchSpanAttributes(maxMessages)...), - } - - ctx, endSpan := tracing.StartSpan(ctx, to) - defer func() { endSpan(err) }() - cancelReleaser := r.cancelReleaser.Swap(emptyCancelFn).(func() string) _ = cancelReleaser() @@ -414,7 +414,7 @@ func (r *Receiver) receiveMessagesImpl(ctx context.Context, maxMessages int, opt var linksWithID *internal.LinksWithID - err = r.amqpLinks.Retry(ctx, EventReceiver, "receiveMessages.getlinks", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { + err := r.amqpLinks.Retry(ctx, EventReceiver, "receiveMessages.getlinks", func(ctx context.Context, lwid *internal.LinksWithID, args *utils.RetryFnArgs) error { linksWithID = lwid return nil }, r.retryOptions, nil) diff --git a/sdk/messaging/azservicebus/receiver_simulated_test.go b/sdk/messaging/azservicebus/receiver_simulated_test.go index 3e1481c22788..efe9195b61aa 100644 --- a/sdk/messaging/azservicebus/receiver_simulated_test.go +++ b/sdk/messaging/azservicebus/receiver_simulated_test.go @@ -348,6 +348,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.PeekMessages", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -364,6 +365,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveDeferredMessages", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -380,6 +382,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveMessages", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusUnset, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -409,6 +412,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.AbandonMessage", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -425,6 +429,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.CompleteMessage", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -441,6 +446,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.DeadLetterMessage", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -457,6 +463,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.settler.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.DeferMessage", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -473,6 +480,7 @@ func TestReceiver_UserFacingErrors(t *testing.T) { receiver.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.RenewMessageLock", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -808,6 +816,7 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { // that to get a session receiver. client.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.AcceptSession", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusUnset, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -824,6 +833,7 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { receiver.inner.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.GetSessionState", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -837,6 +847,7 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { receiver.inner.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.SetSessionState", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -849,6 +860,7 @@ func TestSessionReceiverUserFacingErrors_Methods(t *testing.T) { receiver.inner.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "SessionReceiver.RenewSessionLock", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, diff --git a/sdk/messaging/azservicebus/receiver_unit_test.go b/sdk/messaging/azservicebus/receiver_unit_test.go index 7a43b8e9bfb5..2501b2ea88b0 100644 --- a/sdk/messaging/azservicebus/receiver_unit_test.go +++ b/sdk/messaging/azservicebus/receiver_unit_test.go @@ -29,6 +29,7 @@ func TestReceiver_ReceiveMessages_AMQPLinksFailure(t *testing.T) { receiver := &Receiver{ tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveMessages", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.OperationName, Value: "receive"}, @@ -75,6 +76,7 @@ func TestReceiverCancellationUnitTests(t *testing.T) { r := &Receiver{ tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveMessages", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.OperationName, Value: "receive"}, @@ -105,6 +107,7 @@ func TestReceiverCancellationUnitTests(t *testing.T) { r := &Receiver{ tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Receiver.ReceiveMessages", + Kind: tracing.SpanKindConsumer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.OperationName, Value: "receive"}, diff --git a/sdk/messaging/azservicebus/sender_unit_test.go b/sdk/messaging/azservicebus/sender_unit_test.go index 1c27d11bf78c..d57816b7faf5 100644 --- a/sdk/messaging/azservicebus/sender_unit_test.go +++ b/sdk/messaging/azservicebus/sender_unit_test.go @@ -48,6 +48,7 @@ func TestSender_UserFacingError(t *testing.T) { sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessage", + Kind: tracing.SpanKindProducer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -62,6 +63,7 @@ func TestSender_UserFacingError(t *testing.T) { msgID := "testID" sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessage", + Kind: tracing.SpanKindProducer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -76,6 +78,7 @@ func TestSender_UserFacingError(t *testing.T) { sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.CancelScheduledMessages", + Kind: tracing.SpanKindProducer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -90,6 +93,7 @@ func TestSender_UserFacingError(t *testing.T) { sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.ScheduleMessages", + Kind: tracing.SpanKindProducer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, @@ -114,6 +118,7 @@ func TestSender_UserFacingError(t *testing.T) { sender.tracer = tracing.NewSpanValidator(t, tracing.SpanMatcher{ Name: "Sender.SendMessageBatch", + Kind: tracing.SpanKindProducer, Status: tracing.SpanStatusError, Attributes: []tracing.Attribute{ {Key: tracing.DestinationName, Value: "queue"}, diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go index b4cb8e0949be..f57fad19ca9a 100644 --- a/sdk/messaging/azservicebus/tracing.go +++ b/sdk/messaging/azservicebus/tracing.go @@ -60,7 +60,7 @@ func getSessionSpanAttributes(entityPath string, operationName tracing.Messaging } func getMessageSpanAttributes(message amqpCompatibleMessage) []tracing.Attribute { - attrs := []tracing.Attribute{} + var attrs []tracing.Attribute if message != nil { amqpMessage := message.toAMQPMessage() if amqpMessage != nil && amqpMessage.Properties != nil { @@ -76,7 +76,7 @@ func getMessageSpanAttributes(message amqpCompatibleMessage) []tracing.Attribute } func getReceivedMessageSpanAttributes(receivedMessage *ReceivedMessage) []tracing.Attribute { - attrs := []tracing.Attribute{} + var attrs []tracing.Attribute if receivedMessage != nil { message := receivedMessage.Message() attrs = getMessageSpanAttributes(message) @@ -93,7 +93,7 @@ func getMessageBatchSpanAttributes(size int) []tracing.Attribute { } func getEntityPathAttributes(entityPath string) []tracing.Attribute { - attrs := []tracing.Attribute{} + var attrs []tracing.Attribute queueOrTopic, subscription := splitEntityPath(entityPath) if queueOrTopic != "" { attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) From 14c65768b99174df266c47fbc04f390bd8c09031 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Thu, 16 Jan 2025 20:24:58 -0800 Subject: [PATCH 15/15] add internal tracing for NegotiateClaim --- sdk/messaging/azservicebus/client.go | 5 ++- .../azservicebus/internal/namespace.go | 18 ++++++++++- .../azservicebus/internal/namespace_test.go | 9 ++++++ .../internal/tracing/constants.go | 2 ++ .../azservicebus/internal/tracing/tracing.go | 26 +++++++++++++++ .../internal/tracing/tracing_test.go | 18 +++++++++++ sdk/messaging/azservicebus/tracing.go | 32 ++----------------- sdk/messaging/azservicebus/tracing_test.go | 17 ---------- 8 files changed, 78 insertions(+), 49 deletions(-) diff --git a/sdk/messaging/azservicebus/client.go b/sdk/messaging/azservicebus/client.go index 3619a7b527d5..d8b0de7323eb 100644 --- a/sdk/messaging/azservicebus/client.go +++ b/sdk/messaging/azservicebus/client.go @@ -155,6 +155,7 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { if args.ClientOptions != nil { tracingProvider = args.ClientOptions.TracingProvider + client.retryOptions = args.ClientOptions.RetryOptions if args.ClientOptions.TLSConfig != nil { @@ -176,10 +177,12 @@ func newClientImpl(creds clientCreds, args clientImplArgs) (*Client, error) { nsOptions = append(nsOptions, internal.NamespaceWithRetryOptions(args.ClientOptions.RetryOptions)) } + client.tracer = newTracer(tracingProvider, getFullyQualifiedNamespace(creds)) + nsOptions = append(nsOptions, internal.NamespaceWithTracer(client.tracer)) + nsOptions = append(nsOptions, args.NSOptions...) client.namespace, err = internal.NewNamespace(nsOptions...) - client.tracer = newTracer(tracingProvider, getFullyQualifiedNamespace(creds)) return client, err } diff --git a/sdk/messaging/azservicebus/internal/namespace.go b/sdk/messaging/azservicebus/internal/namespace.go index f8af8a073e7f..1789d148a507 100644 --- a/sdk/messaging/azservicebus/internal/namespace.go +++ b/sdk/messaging/azservicebus/internal/namespace.go @@ -21,6 +21,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/conn" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/exported" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/sbauth" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/utils" "github.com/Azure/go-amqp" ) @@ -38,6 +39,8 @@ type ( // PR: https://github.com/Azure/azure-sdk-for-go/pull/16847 connID uint64 + tracer tracing.Tracer + FQDN string TokenProvider *sbauth.TokenProvider tlsConfig *tls.Config @@ -168,6 +171,13 @@ func NamespaceWithNewClientFn(fn func(ctx context.Context) (amqpwrap.AMQPClient, } } +func NamespaceWithTracer(tracer tracing.Tracer) NamespaceOption { + return func(ns *Namespace) error { + ns.tracer = tracer + return nil + } +} + // NewNamespace creates a new namespace configured through NamespaceOption(s) func NewNamespace(opts ...NamespaceOption) (*Namespace, error) { ns := &Namespace{} @@ -428,6 +438,12 @@ func (ns *Namespace) startNegotiateClaimRenewer(ctx context.Context, log.Writef(exported.EventAuth, "(%s) next refresh in %s", entityPath, nextClaimAt) + to := &tracing.TracerOptions{ + Tracer: ns.tracer, + SpanName: tracing.NegotiateClaimSpanName, + Attributes: tracing.GetEntityPathAttributes(entityPath), + } + select { case <-refreshCtx.Done(): return @@ -442,7 +458,7 @@ func (ns *Namespace) startNegotiateClaimRenewer(ctx context.Context, expiresOn = tmpExpiresOn return nil - }, IsFatalSBError, ns.RetryOptions, nil) + }, IsFatalSBError, ns.RetryOptions, to) if err == nil { break diff --git a/sdk/messaging/azservicebus/internal/namespace_test.go b/sdk/messaging/azservicebus/internal/namespace_test.go index d4f3403402f7..7e33838958b4 100644 --- a/sdk/messaging/azservicebus/internal/namespace_test.go +++ b/sdk/messaging/azservicebus/internal/namespace_test.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/auth" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/sbauth" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/test" + "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" "github.com/Azure/go-amqp" "github.com/stretchr/testify/require" ) @@ -230,6 +231,14 @@ func TestNamespaceNegotiateClaimFails(t *testing.T) { func TestNamespaceNegotiateClaimFatalErrors(t *testing.T) { ns := &Namespace{ + tracer: tracing.NewSpanValidator(t, tracing.SpanMatcher{ + Name: "Namespace.NegotiateClaim", + Kind: tracing.SpanKindInternal, + Status: tracing.SpanStatusError, + Attributes: []tracing.Attribute{ + {Key: tracing.DestinationName, Value: "entity path"}, + }, + }).NewTracer("module", "version"), TokenProvider: sbauth.NewTokenProvider(&fakeTokenCredential{expires: time.Now()}), } diff --git a/sdk/messaging/azservicebus/internal/tracing/constants.go b/sdk/messaging/azservicebus/internal/tracing/constants.go index c673cedca516..8e3f07267ee3 100644 --- a/sdk/messaging/azservicebus/internal/tracing/constants.go +++ b/sdk/messaging/azservicebus/internal/tracing/constants.go @@ -8,6 +8,8 @@ import "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" type SpanName string const ( + NegotiateClaimSpanName SpanName = "Namespace.NegotiateClaim" + SendSpanName SpanName = "Sender.SendMessage" SendBatchSpanName SpanName = "Sender.SendMessageBatch" ScheduleSpanName SpanName = "Sender.ScheduleMessages" diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing.go b/sdk/messaging/azservicebus/internal/tracing/tracing.go index 782254835d52..1ab6c48394ae 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing.go @@ -41,3 +41,29 @@ func StartSpan(ctx context.Context, options *TracerOptions) (context.Context, fu Attributes: options.Attributes, }) } + +func GetEntityPathAttributes(entityPath string) []tracing.Attribute { + var attrs []tracing.Attribute + queueOrTopic, subscription := splitEntityPath(entityPath) + if queueOrTopic != "" { + attrs = append(attrs, tracing.Attribute{Key: DestinationName, Value: queueOrTopic}) + } + if subscription != "" { + attrs = append(attrs, tracing.Attribute{Key: SubscriptionName, Value: subscription}) + } + return attrs +} + +func splitEntityPath(entityPath string) (string, string) { + queueOrTopic := "" + subscription := "" + + path := strings.Split(entityPath, "/") + if len(path) >= 1 { + queueOrTopic = path[0] + } + if len(path) >= 3 && path[1] == "Subscriptions" { + subscription = path[2] + } + return queueOrTopic, subscription +} diff --git a/sdk/messaging/azservicebus/internal/tracing/tracing_test.go b/sdk/messaging/azservicebus/internal/tracing/tracing_test.go index 94fb369bea85..aec8c1e2153d 100644 --- a/sdk/messaging/azservicebus/internal/tracing/tracing_test.go +++ b/sdk/messaging/azservicebus/internal/tracing/tracing_test.go @@ -7,6 +7,7 @@ import ( "context" "testing" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing" "github.com/stretchr/testify/require" ) @@ -42,3 +43,20 @@ func TestStartSpan(t *testing.T) { defer endSpan2(nil) require.NotEqual(t, ctx, subCtx2) } + +func TestGetEntityPathAttributes(t *testing.T) { + entityPath := "queueName" + expectedAttrs := []tracing.Attribute{ + {Key: DestinationName, Value: entityPath}, + } + result := GetEntityPathAttributes(entityPath) + require.ElementsMatch(t, expectedAttrs, result) + + entityPath = "topicName/Subscriptions/subscriptionName" + expectedAttrs = []tracing.Attribute{ + {Key: DestinationName, Value: "topicName"}, + {Key: SubscriptionName, Value: "subscriptionName"}, + } + result = GetEntityPathAttributes(entityPath) + require.ElementsMatch(t, expectedAttrs, result) +} diff --git a/sdk/messaging/azservicebus/tracing.go b/sdk/messaging/azservicebus/tracing.go index f57fad19ca9a..ed822517521e 100644 --- a/sdk/messaging/azservicebus/tracing.go +++ b/sdk/messaging/azservicebus/tracing.go @@ -4,8 +4,6 @@ package azservicebus import ( - "strings" - "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal" "github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus/internal/tracing" ) @@ -39,7 +37,7 @@ func getSenderSpanAttributes(queueOrTopic string, operationName tracing.Messagin } func getReceiverSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) []tracing.Attribute { - attrs := getEntityPathAttributes(entityPath) + attrs := tracing.GetEntityPathAttributes(entityPath) attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) if operationName == tracing.CompleteOperationName || operationName == tracing.AbandonOperationName || @@ -53,7 +51,7 @@ func getReceiverSpanAttributes(entityPath string, operationName tracing.Messagin } func getSessionSpanAttributes(entityPath string, operationName tracing.MessagingOperationName) []tracing.Attribute { - attrs := getEntityPathAttributes(entityPath) + attrs := tracing.GetEntityPathAttributes(entityPath) attrs = append(attrs, tracing.Attribute{Key: tracing.OperationName, Value: string(operationName)}) attrs = append(attrs, tracing.Attribute{Key: tracing.OperationType, Value: string(tracing.SessionOperationType)}) return attrs @@ -91,29 +89,3 @@ func getReceivedMessageSpanAttributes(receivedMessage *ReceivedMessage) []tracin func getMessageBatchSpanAttributes(size int) []tracing.Attribute { return []tracing.Attribute{{Key: tracing.BatchMessageCount, Value: int64(size)}} } - -func getEntityPathAttributes(entityPath string) []tracing.Attribute { - var attrs []tracing.Attribute - queueOrTopic, subscription := splitEntityPath(entityPath) - if queueOrTopic != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.DestinationName, Value: queueOrTopic}) - } - if subscription != "" { - attrs = append(attrs, tracing.Attribute{Key: tracing.SubscriptionName, Value: subscription}) - } - return attrs -} - -func splitEntityPath(entityPath string) (string, string) { - queueOrTopic := "" - subscription := "" - - path := strings.Split(entityPath, "/") - if len(path) == 1 { - queueOrTopic = path[0] - } else if len(path) == 2 { - queueOrTopic = path[0] - subscription = path[1] - } - return queueOrTopic, subscription -} diff --git a/sdk/messaging/azservicebus/tracing_test.go b/sdk/messaging/azservicebus/tracing_test.go index 51e05034135b..40363204b56a 100644 --- a/sdk/messaging/azservicebus/tracing_test.go +++ b/sdk/messaging/azservicebus/tracing_test.go @@ -154,20 +154,3 @@ func TestGetMessageBatchSpanAttributes(t *testing.T) { result := getMessageBatchSpanAttributes(1) require.ElementsMatch(t, expectedAttrs, result) } - -func TestGetEntityPathAttributes(t *testing.T) { - entityPath := "queue" - expectedAttrs := []tracing.Attribute{ - {Key: tracing.DestinationName, Value: entityPath}, - } - result := getEntityPathAttributes(entityPath) - require.ElementsMatch(t, expectedAttrs, result) - - entityPath = "topic/subscription" - expectedAttrs = []tracing.Attribute{ - {Key: tracing.DestinationName, Value: "topic"}, - {Key: tracing.SubscriptionName, Value: "subscription"}, - } - result = getEntityPathAttributes(entityPath) - require.ElementsMatch(t, expectedAttrs, result) -}