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

[improvement] Set the default sampler to random with 1% #158

Merged
merged 2 commits into from
May 21, 2019
Merged

[improvement] Set the default sampler to random with 1% #158

merged 2 commits into from
May 21, 2019

Conversation

diogoholanda
Copy link
Contributor

Before this PR

The default sampler would sample all traces. I would claim this is not a great default and is somewhat error prone. It expects all consumers to override it to an appropriate value, with the penalty to failing to doing so being a high number of tracing logs that puts a lot of pressure in logging pipelines.

After this PR

The default sampler is random with 1% probability of sampling.

@diogoholanda diogoholanda requested a review from a team as a code owner May 21, 2019 13:30
@carterkozak
Copy link
Contributor

The default always-sampler has caused several issues for products I support getting spammed by custom java applications that do not override the default.

I support moving the default sampler from 100% to 1%. Implementing #32 would be a better solution, but requires an API change. @markelliot and @uschi2000 may have opinions and I'd like to hear their thoughts before proceeding.

@ferozco
Copy link
Contributor

ferozco commented May 21, 2019

👍 In support, Its unlikely anyone was intentionally using the current default

@bulldozer-bot bulldozer-bot bot merged commit 7ad39a2 into palantir:develop May 21, 2019
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.

3 participants