Edit pass over telemetry docs #268
Merged
Conversation
This file contains 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
This mainly updates the documentation to cover the new Telemetry framework more
fully, and remove references to prior functionality that are no longer correct.
It also moves up coverage of how to export telemetry, since that's arguably what
regular users will care about more than how to build out your own additional
telemetry. I left those examples mostly unchanged though, they're really great.
I did switch their port to 9090 since it's Prometheus's suggested default, and I
simplified the sample outputs because it has in fact changed -- we no longer
include timestamps in the exposition, and trailing fractional zeroes are now
truncated as well ("1.0000" -> "1").
ckreibich
force-pushed
the
topic/christian/telemetry-editpass
branch
from
471a1d5 to
e32484a
Compare
Interesting. I guess we must have output the trailing zeroes and the timestamp as part of the Broker output, and prometheus-cpp doesn't do that. |
timwoj
approved these changes
Jul 29, 2024
| Native Prometheus Export | ||
| ------------------------ | ||
|
|
||
| Every Zeek process, regardless of whether it's runnig long-term standalone or as |
There was a problem hiding this comment.
Suggested change
| Every Zeek process, regardless of whether it's runnig long-term standalone or as | |
| Every Zeek process, regardless of whether it's running long-term standalone or as |
| <https://github.com/prometheus/docs/blob/main/content/docs/instrumenting/exposition_formats.md#text-format-example>`_. | ||
|
|
||
| The :zeek:see:`Telemetry::metrics_port` variable controls this behavior. Its | ||
| default of ``0/unknown`` won't expose the port; setting it to another TCP port |
There was a problem hiding this comment.
Suggested change
| default of ``0/unknown`` won't expose the port; setting it to another TCP port | |
| default of ``0/unknown`` disables exposing the port; setting it to another TCP port |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 mainly expands the framework's documentation to give a bit more context on Prometheus, and moves the description of how to get telemetry out of Zeek up, before we give examples of the framework's use (which I think matters more for developers).
I noticed a few discrepancies while updating the code examples:
MetricOptssays theunitfield is optional, but omitting it actually causes errors in the metrics registration functions (just search for unguarded access to$unit—opts$unit). I think that fix can wait for the first point release, particularly since existing code will still set the unit.This was a nice reminder that we'd catch such things earlier if we had a setup that automatically runs the documentation's code examples as btests, or some such.
The PR also updates the management framework side, though the changes here are much smaller.