-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Automate Metric Documentation #890
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
Conversation
…istered metrics Signed-off-by: Stephanie Hingtgen <[email protected]>
Signed-off-by: Stephanie Hingtgen <[email protected]>
The test failing on CI is |
Thanks for this and your work! I love this and its use case - it is definitely important. However, I believe there are already tools for it. It was written by our friend @yeya24 https://github.com/yeya24/promlinter We might need to just document better how to use this, and popularize it all. We can even make a small integration test! If there is something missing we could contribute there. What do you think? What I don't like specifically in this PR is the addition of markdown map in heavily used and optimized registry structure. I would optimize here for efficiency of struct than some extra CPU to calculate all with existing information if possible. Having external tool is even better IMO (: |
I might think we can add some extra opts to render nice markdown for https://github.com/yeya24/promlinter 🤗 |
Actually, I love this idea. Having linting/markdown support in the instrumentation library solves the limitation of the static linter (no runtime support so dynamic instrumentation is not supported). But having For the extra performance cost, I feel it is okay as one Prometheus registry usually doesn't have too many metrics. @stephaniehingtgen Do you have any benchmarks showing how much extra memory does it cost? |
What do you propose? (: |
Open questions right now:
|
By adding this after the
I get this as the result: (after): This is a registry with 70 metrics registered.
Totally open to changing the function name & parameters - let me know what you think would be better :)
From what I can tell, it seems like there are two things that the outside tool cannot do:
Please correct me if I'm wrong on either of those fronts.
Could you clarify this a bit? I'm not exactly sure what this is asking. |
Thanks for checking on those! Indeed performance is fine.
I don't think documentation automation with separate CLI is a strong argument, it should be easy to set up CI for all of this as we do with any other linting. Also not sure how that helps with automation. Can you explain what would be the process of exporting the list of metrics? When we would run this
This is something that https://github.com/yeya24/promlinter can't do: If project wraps are registered with custom label names or prefix to metric names (see client_golang/prometheus/wrap.go Line 74 in b54b73c
I still have high hopes and think that static CLI tool can traverse code and find even usages of those wraps. There might be alternative too:
|
Sounds good! I'll go ahead and close this PR. Thanks for talking through it with me! 😄 |
@bwplotka @kakkoyun
Overview
This PR implements a new function,
WriteMetricsMarkdown
, which creates a markdown file containing all of the registered metrics with their corresponding help text and labels.Explanation
The goal for this function to enable developers to automatically generate and keep an up to date METRICS.md file.
Previously to generate this type of file, a developer would have to hit the metrics endpoint for the help text and then manually use that to generate a markdown file. This also wouldn't guarantee that all of the metrics were covered as the vectors may not show up in the metrics endpoint if it has not yet been used.
With the addition of this function, this could be automated for developers, and would ensure completeness as this function returns all of the metrics (including the vectors that have not yet been used). It would also ensure that documentation stays up to date as metrics are added.
Example
This code would be added to a user's code base:
The result would be a markdown file like this:
Notes
If there is concern over the additional map and memory, I can modify this to not include uninitialized vectors and make it a requirement that those are initialized before running this function. Then I could use the Gather function and loop through that.