feat: Provide dependency names for dependencies & requests

[pim-simons]

• Requests are now logged with an operation name so they don't show in Application Insights as ''. For example:
  
  After discussion with @stijnmoreels we decided to add 2 overloads to be able to specify a custom operataion name as well.
• Names have been added to all dependancies so that they don't show up in Application Insights 'n/a'. For example:
  

[netlify]

✔️ Deploy Preview for arcus-observability canceled.

🔨 Explore the source changes: 4fb6cf0

🔍 Inspect the deploy log: https://app.netlify.com/sites/arcus-observability/deploys/614c94efd7e0a000071ad210

[tomkerkhove]

If the PR is only for the request, can we remove the changes for the dependencies please?

[tomkerkhove]

If you keep the dependency changes, can you please use the convention from https://github.com/arcus-azure/arcus-service-to-service-correlation-poc/blob/d1526c963c04b35243d71a65f9eefd3eeba1dcee/src/Arcus.POC.Observability.Telemetry.Serilog.Sinks.ApplicationInsights/Extensions/ILoggerExtensions.cs#L118 for Service Bus?

I didn't include the test but the goal is to align with the official SDK notation.

[pim-simons]

If you keep the dependency changes, can you please use the convention from https://github.com/arcus-azure/arcus-service-to-service-correlation-poc/blob/d1526c963c04b35243d71a65f9eefd3eeba1dcee/src/Arcus.POC.Observability.Telemetry.Serilog.Sinks.ApplicationInsights/Extensions/ILoggerExtensions.cs#L118 for Service Bus?

I didn't include the test but the goal is to align with the official SDK notation.

Thanks for the feedback!
Checked the code, the requested convention is $"{serviceBusNamespaceEndpoint} | {queueName}" but the Service Bus Namespace is currently not available on the Service Bus logging methods, only the type (Queue, Topic or Unknown) and name of the entity.
I could create new overloads for the Service Bus logging to include the namespace but then how do we set the dependencyName for the current methods without the namespace? Maybe we could do something like $"{entityType} | {entityName}" instead of just the entityName?
Let me know if I missed anything!

[stijnmoreels]

I'll have a look later, as I'm not allowed to work long for Arcus for more than a day in a week 😅, (shame).

Thanks for your PR! Very much appreciated! 🙏🥇
Great to have another enthusiastic contributor in our Arcus family.

[tomkerkhove]

If you keep the dependency changes, can you please use the convention from arcus-azure/arcus-service-to-service-correlation-poc@d1526c9/src/Arcus.POC.Observability.Telemetry.Serilog.Sinks.ApplicationInsights/Extensions/ILoggerExtensions.cs#L118 for Service Bus?
I didn't include the test but the goal is to align with the official SDK notation.

Thanks for the feedback!
Checked the code, the requested convention is $"{serviceBusNamespaceEndpoint} | {queueName}" but the Service Bus Namespace is currently not available on the Service Bus logging methods, only the type (Queue, Topic or Unknown) and name of the entity.
I could create new overloads for the Service Bus logging to include the namespace but then how do we set the dependencyName for the current methods without the namespace? Maybe we could do something like $"{entityType} | {entityName}" instead of just the entityName?
Let me know if I missed anything!

We should have them indeed, but let's use a separate PR for that.

[tomkerkhove]

Looks like a good start, thank you!

I see two things we should do:

• Provide unit tests for dependencies
• Move the conversion to our OperationContextConverter

[stijnmoreels]

Great first step! You really have successfully found the necessary places to incorporate your requested changes. Like Tom said, we usually provide unit tests for every publicly available member (I'm sure you'll find the logging unit tests for requests).

• Provide unit tests for requests (and dependencies if you don't create a separate PR for them)
• Update the dependency integration tests to include the passed-along operation name

Let us know @pim-simons , if you need any help! You're doing a great job!

[stijnmoreels]

We're almost there! Let's have @tomkerkhove another look, especially on the casting/child classes. Otherwise, great job! 🥇

[stijnmoreels]

I'll approve on my part. Great stuff!

[tomkerkhove]

LGTM, thank you! 🎉

[tomkerkhove]

I think we are good to go, except for the integration tests - Are these on your radar?

[pim-simons]

I think we are good to go, except for the integration tests - Are these on your radar?

I updated the integration tests to check for the dependencyname and added some integration tests for the requestlogging to test specifying a custom operation name.
Am I missing any?

[tomkerkhove]

They seem to be failing, can you please have a look?

[pim-simons]

They seem to be failing, can you please have a look?

Worst argument ever, but works on my machine 😉

I can't really find a reason why these specific tests would fail, is there any way to manually restart them?

[tomkerkhove]

/azp run CI - Arcus.Observability

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[pim-simons]

It's all looking green now 👍🏻 seems like the tests worked this time

[tomkerkhove]

🙌 Thank you for the nice contribution!