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

Add flag to disable tracing activation #2191

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

periklis
Copy link
Collaborator

@periklis periklis commented Jun 6, 2020

What this PR does / why we need it:
This PR addresses the capability to disable tracing per configuration flag in enviroments where the Jaeger env variables are in place and thus auto-activate tracing. The config flag comes in hand as an user provided override to disable tracing if required.

Which issue(s) this PR fixes:
Fixes #2183

Checklist

  • Documentation added

@CLAassistant
Copy link

CLAassistant commented Jun 6, 2020

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2020

Codecov Report

Merging #2191 into master will increase coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2191      +/-   ##
==========================================
+ Coverage   61.60%   61.66%   +0.05%     
==========================================
  Files         154      154              
  Lines       12310    12311       +1     
==========================================
+ Hits         7583     7591       +8     
+ Misses       4120     4108      -12     
- Partials      607      612       +5     
Impacted Files Coverage Δ
pkg/loki/loki.go 0.00% <0.00%> (ø)
pkg/promtail/targets/tailer.go 73.86% <0.00%> (-4.55%) ⬇️
pkg/promtail/targets/filetarget.go 68.67% <0.00%> (-1.81%) ⬇️
pkg/ingester/transfer.go 65.49% <0.00%> (-1.41%) ⬇️
pkg/logql/evaluator.go 90.56% <0.00%> (+0.53%) ⬆️
pkg/promtail/positions/positions.go 60.71% <0.00%> (+13.39%) ⬆️

@@ -43,3 +43,6 @@ chunk_store_config:
table_manager:
retention_deletes_enabled: false
retention_period: 0s

tracing:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to change the local config.

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

I just don't think we need to change the current config, but this seems like a good addition, I don't really like ENV var configuration so this is better.

Thank you !

@cyriltovena
Copy link
Contributor

btw you need to sign the CLA.

@periklis
Copy link
Collaborator Author

periklis commented Jun 8, 2020

Interesting that you say that, the cla bot seems to not work if I commit from my secondary e-mail which is my red hat e-mail.

@cyriltovena cyriltovena merged commit 6e55277 into grafana:master Jun 8, 2020
@periklis periklis deleted the tracing-flags branch June 9, 2020 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loki makes GET requests to localhost:5778 every 1 minute
4 participants