Fix Azure SDK activity sources and minor fixes #163
Conversation
…mples, fix nits in docs,
lmolkova
force-pushed
the
azure-component-fixes
branch
from
af506fa
to
3c09093
Compare
| @@ -6,10 +6,6 @@ namespace OrderProcessor; | |||
|
|
|||
| public class OrderProcessingWorker : BackgroundService | |||
| { | |||
| public static readonly string ActivitySourceName = "Worker"; | |||
|
|
|||
| public static ActivitySource ActivitySource { get; } = new ActivitySource(ActivitySourceName); | |||
There was a problem hiding this comment.
Why are you removing the custom ActivitySource here? I don't think we would want user code to add random tags to Activities that are already established.
There was a problem hiding this comment.
Because it's duplication and perf hit. Also, I see no problem with random tags on ServiceBus activity - this is a common pattern people do.
There was a problem hiding this comment.
Because it's duplication
It's not duplication. If this were a real app, you would want to be able to isolate your business logic's trace information. If there were multiple queues, i.e. for other workflows, I would need to search all the queue processors when looking for my order processing traces.
There was a problem hiding this comment.
Because it's duplication
It's not duplication. If this were a real app, you would want to be able to isolate your business logic's trace information. If there were multiple queues, i.e. for other workflows, I would need to search all the queue processors when looking for my order processing traces.
I can still query for SB consumer spans coming from specific SB/entity. I don't think there is a universal answer here.
We hopefully wouldn't recommend adding activity per controller call only to report business-logic part.
with custom activity, we'd report extra span to backend which start time/duration/status/extra context are already reported by SB activity. Users pay for it (~$2 per GB). Is clean separation between business and tech telemetry worth it?
May be, but this is a sample code, those who need it can create extra activity. I'd say it's not justified by default.
There was a problem hiding this comment.
Agree, an activity shouldn't be needed in this case if the sdk is already making one per message.
Azure SDK uses
<packageName>.<clientName>convention for activity sources and recommends enabling<packageName>.*sources by default (to avoid listing multiple possible clients that are not always used directly by developers).See Azure/azure-sdk-for-net#31921 and Azure/azure-sdk-for-net#39166 for the context.
This change
Things work just fine: