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.

Sign up for GitHub

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

Jump to bottom

Add environment variable to suppress HTTP span creation #17969

Closed
maorleger opened this issue Sep 30, 2021 · 2 comments · Fixed by #20776
Closed

Add environment variable to suppress HTTP span creation #17969

maorleger opened this issue Sep 30, 2021 · 2 comments · Fixed by #20776
Assignees
@maorleger
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Milestone
[2022] April

Comments

@maorleger
Copy link
Member

maorleger commented Sep 30, 2021 •
edited
Loading

In order to mitigate duplication of AZ SDK HTTP spans with auto-instrumentation HTTP spans the tracing crew is adding AZURE_HTTP_TRACING_DISABLED env var support to disable downstream spans from being recorded after our tracing policy

This issue tracks that effort for JS
@maorleger maorleger added Azure.Core Client This issue points to a problem in the data-plane of the library. labels Sep 30, 2021
@maorleger maorleger added this to the [2021] November milestone Sep 30, 2021
@maorleger maorleger self-assigned this Sep 30, 2021
@maorleger maorleger modified the milestones: [2021] December, Backlog Nov 30, 2021
@maorleger
Copy link
Member Author

@lmolkova I'm reviewing our remaining issues and was wondering if this is still needed?

@maorleger maorleger modified the milestones: Backlog, [2022] April Feb 8, 2022
@lmolkova
Copy link
Member

lmolkova commented Feb 9, 2022 •
edited
Loading

I believe it it's relevant (as I believe we see duplicates if we enable auto-collection for our http client in otel), except I'd love to explore this suppression path instead:

https://github.com/open-telemetry/opentelemetry-js/blob/0dc4c3d8eb4f52b839365f964bb7aaac08dcefb2/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts#L60

it's defined in otel-core package
https://github.com/open-telemetry/opentelemetry-js/blob/c85fbe6682b9a8fd52e4b99bdbb079833b04e845/packages/opentelemetry-core/src/trace/suppress-tracing.ts#L23

and we can do it after we start http span, along with making it current.

@maorleger maorleger mentioned this issue Mar 10, 2022
Merged
3 tasks
maorleger added a commit that referenced this issue Mar 15, 2022
@maorleger
[instrumentation] - Suppress tracing using environment variables (#20776
5caf03e 
)

### Packages impacted by this PR
@azure/opentelemetry-instrumentation-azure-sdk

### Issues associated with this PR
Resolves #17969

### Describe the problem that is addressed by this PR
In order to mitigate duplication of AZ SDK HTTP spans with auto-instrumentation
HTTP spans, the tracing feature crew would like to add the
AZURE_HTTP_TRACING_DISABLED environment variable to disable any dowstream
 HTTP spans from being created. 

In addition, I realized that when migrating off-of `createSpan` we did not
migrate the logic to suppress _all_ tracing using AZURE_TRACING_DISABLED
environment variable.

This PR fixes both of these issues.

### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
The alternatives really were:
- Add this logic in core-tracing for global suppression
- Add this logic in tracingPolicy for HTTP span suppression

Since we use a no-op instrumenter by default there's no reason to add additional
logic to core-tracing. Further, using it in the OTel package allows us to take
advantage of OTel provided APIs which will be honored downstream as well.

Quick summary of requirements here:
- When AZURE_TRACING_DISABLED is set, we want to suppress our Azure SDK spans from being recorded
- When AZURE_HTTP_TRACING_DISABLED is set, we want to suppress any children of our SDK's HTTP spans from being recorded, this avoids the HTTP span duplication problem while OTel works out a solution on their end
- Finally, if _both_ are set, we suppress our Azure SDK spans from being recorded, but _not_ suppress any children of our HTTP spans (since there's no duplication)
WeiJun428 pushed a commit to WeiJun428/azure-sdk-for-js that referenced this issue Mar 20, 2022
@maorleger @WeiJun428
[instrumentation] - Suppress tracing using environment variables (Azu…
a1eb6c2 
…re#20776)

### Packages impacted by this PR
@azure/opentelemetry-instrumentation-azure-sdk

### Issues associated with this PR
Resolves Azure#17969

### Describe the problem that is addressed by this PR
In order to mitigate duplication of AZ SDK HTTP spans with auto-instrumentation
HTTP spans, the tracing feature crew would like to add the
AZURE_HTTP_TRACING_DISABLED environment variable to disable any dowstream
 HTTP spans from being created. 

In addition, I realized that when migrating off-of `createSpan` we did not
migrate the logic to suppress _all_ tracing using AZURE_TRACING_DISABLED
environment variable.

This PR fixes both of these issues.

### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
The alternatives really were:
- Add this logic in core-tracing for global suppression
- Add this logic in tracingPolicy for HTTP span suppression

Since we use a no-op instrumenter by default there's no reason to add additional
logic to core-tracing. Further, using it in the OTel package allows us to take
advantage of OTel provided APIs which will be honored downstream as well.

Quick summary of requirements here:
- When AZURE_TRACING_DISABLED is set, we want to suppress our Azure SDK spans from being recorded
- When AZURE_HTTP_TRACING_DISABLED is set, we want to suppress any children of our SDK's HTTP spans from being recorded, this avoids the HTTP span duplication problem while OTel works out a solution on their end
- Finally, if _both_ are set, we suppress our Azure SDK spans from being recorded, but _not_ suppress any children of our HTTP spans (since there's no duplication)
WeiJun428 pushed a commit to WeiJun428/azure-sdk-for-js that referenced this issue Mar 20, 2022
@maorleger @WeiJun428
[instrumentation] - Suppress tracing using environment variables (Azu…
f33643f 
…re#20776)

### Packages impacted by this PR
@azure/opentelemetry-instrumentation-azure-sdk

### Issues associated with this PR
Resolves Azure#17969

### Describe the problem that is addressed by this PR
In order to mitigate duplication of AZ SDK HTTP spans with auto-instrumentation
HTTP spans, the tracing feature crew would like to add the
AZURE_HTTP_TRACING_DISABLED environment variable to disable any dowstream
 HTTP spans from being created. 

In addition, I realized that when migrating off-of `createSpan` we did not
migrate the logic to suppress _all_ tracing using AZURE_TRACING_DISABLED
environment variable.

This PR fixes both of these issues.

### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
The alternatives really were:
- Add this logic in core-tracing for global suppression
- Add this logic in tracingPolicy for HTTP span suppression

Since we use a no-op instrumenter by default there's no reason to add additional
logic to core-tracing. Further, using it in the OTel package allows us to take
advantage of OTel provided APIs which will be honored downstream as well.

Quick summary of requirements here:
- When AZURE_TRACING_DISABLED is set, we want to suppress our Azure SDK spans from being recorded
- When AZURE_HTTP_TRACING_DISABLED is set, we want to suppress any children of our SDK's HTTP spans from being recorded, this avoids the HTTP span duplication problem while OTel works out a solution on their end
- Finally, if _both_ are set, we suppress our Azure SDK spans from being recorded, but _not_ suppress any children of our HTTP spans (since there's no duplication)
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@maorleger maorleger

Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Milestone
[2022] April
Development

Successfully merging a pull request may close this issue.

[instrumentation] - Suppress tracing using environment variables maorleger/azure-sdk-for-js
2 participants
@maorleger @lmolkova