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

dnsdist: Deduplicate frontends entries with carbon and prometheus #7934

Merged
merged 2 commits into from Jun 17, 2019

Conversation

@rgacogne
Copy link
Member

@rgacogne rgacogne commented Jun 14, 2019

Short description

This is a work-around, and not a very pretty one, to avoid messing up our carbon/prometheus metrics when we add the same frontends several times with addLocal(), addTLSLocal() or addDOHLocal() and reuseport.
The proper fix would be to be able to provide the number of threads directly as a parameter to the add*Local() and provide aggregated metrics (but also per-thread metrics, at least for the number of queries, so we can check that the distribution done by the kernel is working correctly), but this would require an important refactoring and we are too late in the 1.4.0 release process for that IMHO.

We will need to implement the deduplication for the DoH metrics in prometheus as well, either in this PR if #7933 is merged first, or in #7933 otherwise.

Fixes #7358.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
@rgacogne rgacogne added this to the dnsdist-1.4.0 milestone Jun 14, 2019
@rgacogne rgacogne requested a review from pieterlexis Jun 17, 2019
Copy link
Member

@pieterlexis pieterlexis left a comment

not tested, code looks good, with some nits.

pdns/dnsdist-carbon.cc Outdated Show resolved Hide resolved
pdns/dnsdist-carbon.cc Outdated Show resolved Hide resolved
pdns/dnsdist-carbon.cc Show resolved Hide resolved
@rgacogne rgacogne merged commit 2b4c903 into PowerDNS:master Jun 17, 2019
21 checks passed
@rgacogne rgacogne deleted the dnsdist-fix-dup-metrics branch Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants