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

Suppress nested SDK public method calls #28998

Merged

Conversation

lmolkova
Copy link
Member

@lmolkova lmolkova commented May 20, 2022

To support building convenience methods on top of DPG clients better, we want to avoid having multiple spans for public API calls and came up with the following guidance (not yet documented, but on the way):

"one SDK client span per customer method invocation with however many nested HTTP spans underneath”

(client span here refers to SDK client, not to span kind which can be internal or client for such calls).

This change suppresses internal or client spans nested under other internal or client spans.

Example:

  • EventHubs.process (server/consumer spans)
    • [user code] BlobClient.Upload
      • HTTP HEAD /blob 200
      • HTTP PUT /blob 200
    • [eventhubs code] EventHubs.Checkpoint
      • [suppressed with this PR] BlobClient.SetMetada
      • HTTP PUT /metadata 200

or

  • MetricAdvisor.convenienceCall (internal)
    • [suppressed with this PR] MetricAdvisor.autoGeneratedMethodCall (internal)
      • HTTP GET (becomes a child of convenienceCall)

In theory this is a breaking change in behavior, but since azure-core-tracing-opentelemetry package is in beta and the change is contained there, we're still able to make it

@lmolkova lmolkova requested a review from srnagar May 20, 2022 18:05
@ghost ghost added the Azure.Core azure-core label May 20, 2022
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.core.tracing.opentelemetry.implementation;
Copy link
Member Author

Choose a reason for hiding this comment

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

it's not needed anymore. This code was interacting with OTel Java agent, but now the agent contains Azure SDK instrumentation (thanks to Trask 🥳) and we don't need the suppression here.

This code was also noop because of OTel instrumentation API changes.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

👍

@lmolkova lmolkova merged commit e849033 into Azure:main May 27, 2022
anushkasingh16 pushed a commit to anushkasingh16/azure-sdk-for-java that referenced this pull request Jun 6, 2022
* Suppress nested SDK public method calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core azure-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants