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

.*: pkg/tracing: Disable Elastic APM default tracer #2614

Merged
merged 2 commits into from May 18, 2020

Conversation

Sayrus
Copy link
Contributor

@Sayrus Sayrus commented May 15, 2020

Disable Elastic APM Default Tracer that would otherwise gather metrics
and send them on localhost even if no tracing backend was specified.

Fix #2610

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

I don't think the change is relevant as I don't see anyone using the default metric gatherer without enabling tracing. However feel free to edit it if you think the new behavior is considered breaking changes.
The worse case scenario is: someone is using the default gatherer to gather Go Metrics without having tracing enabled. As far as I understand it, this is undocumented and unsupported.

Changes

Disable Elastic APM default tracer on initialization

Verification

Start thanos with ELASTIC_APM_LOG_LEVEL=debug and ELASTIC_APM_LOG_FILE=stdout. Elastic APM does not try to gather and send metrics anymore.

@Sayrus Sayrus force-pushed the close-elastic-apm-default-tracer branch 3 times, most recently from e6651f2 to d0da235 Compare May 15, 2020 08:59
@Sayrus Sayrus marked this pull request as ready for review May 15, 2020 09:17
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Yuck, thanks for this! LGTM. Could you please rebase on master? I think we should not note this as a breaking change since this was never documented and there is a proper way to load a tracer configuration into Thanos that it supposed to be used.

Disable Elastic APM Default Tracer that would otherwise gather metrics
and send them on localhost even if no tracing backend was specified.

Fix thanos-io#2610

Signed-off-by: Mathis Raguin <mathis@cri.epita.fr>
@Sayrus Sayrus force-pushed the close-elastic-apm-default-tracer branch from d0da235 to 28eccfa Compare May 18, 2020 06:14
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bwplotka bwplotka merged commit 439aad3 into thanos-io:master May 18, 2020
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.

.*: Elastic APM Go Agent is enabled even without tracing configuration
3 participants