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

Query/Ruler: Add metric for number of endpoints resolved for each configured address. #1260

Merged
merged 1 commit into from Jun 18, 2019

Conversation

devnev
Copy link
Contributor

@devnev devnev commented Jun 18, 2019

Changes

Add thanos_querier_store_apis_dns_provider_results / thanos_ruler_query_apis_dns_provider_results metrics.

count(dns_provider_results) tells us how many addresses have been
configured through flags and file-SD, while sum(dns_provider_results)
gives us the number of endpoints actually produced from these addresses.

Verification

Ran thanos query --store=localhost:10901, navigated to localhost:10902/metrics and found the entry:

# HELP thanos_querier_store_apis_dns_provider_results The number of resolved endpoints for each configured address
# TYPE thanos_querier_store_apis_dns_provider_results gauge
thanos_querier_store_apis_dns_provider_results{addr="localhost:10901"} 1

`count(dns_provider_results)` tells us how many addresses have been
configured through flags and file-SD, while `sum(dns_provider_results)`
gives us the number of endpoints actually produced from these addresses.
@@ -91,6 +97,7 @@ func (p *Provider) Resolve(ctx context.Context, addrs []string) {
p.resolverLookupsCount.Inc()
if err != nil {
// The DNS resolution failed. Continue without modifying the old records.
p.resolved[addr] = p.resolved[addr] // Ensure metrics capture the result even if empty.
Copy link
Member

@bwplotka bwplotka Jun 18, 2019

Choose a reason for hiding this comment

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

I don't really get this part is, it intentional? Isn't this noop?
It's tricky what should be there TBH ;p Should we set results for both query to empty and metrics to 0? should we only adjust metrics? Not sure yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ensures that the map contains an entry for the address. So when we iterate over the map keys for the metric, we report that there was an address configured regardless of whether we got some results here, got no results, or continue using previous results.

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, except one thing, commented below.

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 c7da9f2 into master Jun 18, 2019
@brancz brancz deleted the thanos-dns-addrs-metric branch June 18, 2019 15:57
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.

None yet

2 participants