Skip to content

fix: untraced only works with parent-based sampler#1412

Merged
fbogsany merged 8 commits intomainfrom
fix-untraced
Jan 12, 2023
Merged

fix: untraced only works with parent-based sampler#1412
fbogsany merged 8 commits intomainfrom
fix-untraced

Conversation

@fbogsany
Copy link
Copy Markdown
Contributor

@fbogsany fbogsany commented Dec 9, 2022

The untraced helper currently only works when a parent-based sampler is installed. As an example, it will not suppress spans if the ALWAYS_ON sampler is installed.

This fixes untraced to work regardless of the sampler. It does this by communicating via a private Context key. An additional helper untraced? is introduced to query the state of the key from the Context. The SDK Tracer#start_span implementation is updated to check for untraced? before delegating either to the API Tracer#start_span method or to TracerProvider#internal_start_span.

The OpenTelemetry JS implementation offers the same functionality implemented in the same way, and serves as inspiration for the implementation in this PR.

Copy link
Copy Markdown
Contributor

@arielvalentin arielvalentin left a comment

Choose a reason for hiding this comment

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

Assuming linter errors are addressed.

@fbogsany
Copy link
Copy Markdown
Contributor Author

fbogsany commented Dec 9, 2022

Assuming linter errors are addressed.

I blame Copilot.

@fbogsany
Copy link
Copy Markdown
Contributor Author

fbogsany commented Dec 9, 2022

We'll have to be careful with the version bumps when releasing this. We've added a method to the -common gem that the -sdk now depends on, but we haven't changed the -sdk interface.

Copy link
Copy Markdown
Contributor

@ahayworth ahayworth left a comment

Choose a reason for hiding this comment

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

Shouldn't we add a dependency on -common to the sdk gemspec?

@fbogsany
Copy link
Copy Markdown
Contributor Author

fbogsany commented Dec 9, 2022

Shouldn't we add a dependency on -common to the sdk gemspec?

It's already there.

@ahayworth
Copy link
Copy Markdown
Contributor

Shouldn't we add a dependency on -common to the sdk gemspec?
It's already there.

🤦 Wow - it is indeed, I must have skipped over it when I was hunting for it. 🤦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants