From feffdf85f0ed46190ef0091168853803d65e0a48 Mon Sep 17 00:00:00 2001 From: Karen Chen Date: Tue, 10 Dec 2024 17:19:04 -0800 Subject: [PATCH 01/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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") }