You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Metrics should not contain duplicate data, summing all metrics of
a given name (regardless of labels) needs to be meaningful. Therefore
the counter metrics based on different DNS aspects have been split into
separate metric names.
1. The elapsed time (`nsd_time_elapsed_seconds`) doesn't have much value
currently, and doesn't reset when `nsd-control stats` is used, so I would
probably remove it. However, it could be beneficial to reset this time
whenever `nsd-control` resets the stats.
I disabled the elapsed time metric, until I point it to the remote-control time value.
2. For the zonestats block: Should the label name "zone" maybe renamed to
"zonegroup" or something else?
I used zonestats, to copy the name from the config file.
3. Currently, the metrics don't have any HELP text (see [Prometheus Exposition Formats](https://prometheus.io/docs/instrumenting/exposition_formats/#comments-help-text-and-type-information))
The reason will be displayed to describe this comment to others. Learn more.
The code looks good, and in general I like it, but I would want to change the copy of the statistics management code from remote.c into metrics.c, instead expose one or more static routines from remote.c and call them from metrics.c, making the code more understandable and maintainable.
I'm adding a separate test for this because the metrics endpoint would
print a time from the remote control endpoint, which would be
uninitialized when remote-control is disabled and should therefore be
omitted.
I added the tests for the metrics. One with remote-control enabled, resetting the stats, and basically just sanity checking that the metrics also reset. One with rc disabled just to make sure it works in both cases.
While writing the test, I ran into a segfault when rc was disabled, therefore the fix in f08dec9.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for Prometheus metrics, exposed using
libevent
's HTTP serverimplementation. It is feature gated with
--enable-prometheus-metrics
.The statistics export and socket code is based on code from
remote.c
.Here is an excerpt from the exported metrics: