Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
annelo-msft opened this issue Jun 9, 2022 · 3 comments
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. DPG MQ This issue is part of a "milestone of quality" initiative. OpenTelemetry OpenTelemetry instrumentation (not Monitor-specific)
Milestone

Comments

@annelo-msft
Copy link
Member

annelo-msft commented Jun 9, 2022

@pshao25 identified that distributed tracing spans for LROs are incorrect when nested span suppression is disabled. Her investigation is described here: Azure/autorest.csharp#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:

  • User calls LRO method with WaitUntil.Completed
  • User calls LRO method with WaitUntil.Started and
    • UpdateStatus
    • WaitForCompletion
    • WaitForCompletionResponse

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

  • Operation and Operation<T>
  • Protocol and Convenience methods
  • Nested span supression on/off
@annelo-msft annelo-msft added Azure.Core Client This issue points to a problem in the data-plane of the library. DPG labels Jun 9, 2022
@annelo-msft annelo-msft added the MQ This issue is part of a "milestone of quality" initiative. label Jun 9, 2022
@lmolkova lmolkova added the OpenTelemetry OpenTelemetry instrumentation (not Monitor-specific) label Jun 16, 2022
@jsquire jsquire added this to the Backlog milestone Jun 28, 2022
@m-redding m-redding self-assigned this Dec 5, 2022
@m-redding
Copy link
Member

m-redding commented Feb 10, 2023

A set of written tests attempting to add test coverage for distributed tracing is held in Azure/autorest.csharp#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.

@m-redding m-redding removed their assignment Feb 10, 2023
@pallavit
Copy link
Contributor

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

@m-redding
Copy link
Member

Yeah I think we should be able to close this.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. DPG MQ This issue is part of a "milestone of quality" initiative. OpenTelemetry OpenTelemetry instrumentation (not Monitor-specific)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants