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

[core] Create spans using automatic span propagation #31019

Merged

Conversation

maorleger
Copy link
Member

@maorleger maorleger commented Sep 6, 2024

Packages impacted by this PR

@azure/core-rest-pipeline
@typespec/ts-http-runtime

Issues associated with this PR

N/A

Describe the problem that is addressed by this PR

Back when we were migrating to the new core tracing we wanted to be cautious and
bail as early as possible in our core tracing policy. This meant that we:

  1. Guard against any errors
  2. Ignore automatic span propagation, instead expecting HLC libraries to call
    tracingClient.withSpan to kick off the manual span propagation (context
    parameter passing in options)

Now that we have RLC which does not have a tracingClient, and having had this
code in production for quite some time, we can remove the optimization that
aassumes the tracingContext will be added upstream.

This is a non-impactful change for high-level clients or anyone who has not opted-in to OTel tracing.

For users who use rest-level clients and have opted into OTel, they will now see HTTP spans from core added correctly

Are there test cases added in this PR? (If not, why?)

Updated test expectation

Provide a list of related PRs (if any)

Command used to generate this PR: (Applicable only to SDK release request PRs)

Checklists

  • Added impacted package name to the issue description.
  • Does this PR need any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here.)
  • Added a changelog (if necessary).

@maorleger maorleger marked this pull request as ready for review September 6, 2024 19:12
@maorleger maorleger requested a review from a team as a code owner September 6, 2024 19:12
Copy link
Member

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
@maorleger maorleger enabled auto-merge (squash) September 9, 2024 17:56
@maorleger maorleger merged commit d4ead16 into Azure:main Sep 9, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants