search

Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.36k stars 4.75k forks source link

Add test coverage for distributed tracing of LROs when nested span suppression is disabled #29215

Closed annelo-msft closed 9 months ago

annelo-msft commented 2 years ago

@pshao25 identified that distributed tracing spans for LROs are incorrect when nested span suppression is disabled. Her investigation is described here: https://github.com/Azure/autorest.csharp/issues/2297

Distributed tracing span scopes for LROs are described in the following two issues:

Test for the LRO distributed tracing spans are currently here: https://github.com/Azure/autorest.csharp/blob/feature/v3/test/AutoRest.TestServerLowLevel.Tests/dpg-customization.cs#L96 Validation of diagnostic scopes should be moved to Azure.Core when the Operation implementation methods move to Azure.Core.

We should add tests validating correct behavior for all cases:

The above four cases should be validated for each of the following conditions:

m-redding commented 1 year ago

A set of written tests attempting to add test coverage for distributed tracing is held in https://github.com/Azure/autorest.csharp/pull/2961

As of now, there doesn't seem to be a clear consensus about what the correct behavior is for each of these test cases from the information gathered in various linked GitHub discussions. In addition, it is unclear if these are tests that should be added across languages to the test server repo (or just .NET). It is also unclear if these cases are important for generated DPG libraries, since I was not able to find any real scenarios necessitating these tests in our generated libraries. If one is found though, I'm happy to move forward with the closed PR above.

pallavit commented 9 months ago

@m-redding given your investigation can we close this issue for now? @annelo-msft as FYI.

m-redding commented 9 months ago

Yeah I think we should be able to close this.