Skip to content

Conversation

kerolasa
Copy link
Contributor

This pull request has three minor changes that are so trivial they hopefully do not need much discussion.

Main purpose of the change set is to add -threads=false command-line option support, that will make the thread specific exports to be summary counters. Level of detail is just too much in cases such as a company I work for, where we have lots of threads and lots of installations. That leads to prometheus server to reserve a lot of memory to keep track of these metrics, that are always summarised when visualised. Notice that the default per thread counting is kept as-is, new summary output is only for people who really want to merge the metrics.

Fixes following staticcheck warning.

    $ staticcheck ./unbound_exporter.go
    unbound_exporter.go:279:22: should use raw string (`...`) with
    regexp.MustCompile to avoid having to escape twice (S1007)
Per thread metrics can be unnecessarily detailed in environments where there
are lots of unbound instances, and prometheus server memory is short in supply.
return err
}

if !threads && strings.HasPrefix(matches[0], "thread") {

Choose a reason for hiding this comment

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

Can you guarantee that matches actually has this length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can. There is matches != nil check before ParseFloat(), so there should always be at least first entry. Assuming the first entry tells thread all of the expressions have three fields. That should make both the matches[1] assignment and matches[1:] reference safe.

@pgporada
Copy link
Member

Gotcha, here's the old (default) behavior:

root@46dbabba90c5:/golang/unbound-pr-thread# curl -s localhost:9167/metrics | grep -v '#' | grep thread
go_threads 14
unbound_cache_hits_total{thread="0"} 3
unbound_cache_misses_total{thread="0"} 0
unbound_prefetches_total{thread="0"} 0
unbound_queries_total{thread="0"} 3
unbound_recursive_replies_total{thread="0"} 0
unbound_request_list_current_all{thread="0"} 0
unbound_request_list_current_user{thread="0"} 0
unbound_request_list_exceeded_total{thread="0"} 0
unbound_request_list_overwritten_total{thread="0"} 0

and behavior with the new flag

root@46dbabba90c5:/golang/unbound-pr-thread# curl -s localhost:9167/metrics | grep -v '#' | grep thread
go_threads 8
unbound_cache_hits_total{thread="sum"} 3
unbound_cache_misses_total{thread="sum"} 0
unbound_prefetches_total{thread="sum"} 0
unbound_queries_total{thread="sum"} 3
unbound_recursive_replies_total{thread="sum"} 0
unbound_request_list_current_all{thread="sum"} 0
unbound_request_list_current_user{thread="sum"} 0
unbound_request_list_exceeded_total{thread="sum"} 0
unbound_request_list_overwritten_total{thread="sum"} 0

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.

3 participants