[collectd 6] Write Prometheus plugin: Ensure metric and label names are properly formatted. #4206
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
eero-t
requested changes
Dec 21, 2023
octo
force-pushed
the
6/write_prometheus
branch
from
5c4dc1c
to
c47deae
Compare
|
Created a separate PR for Yes, test cases for metric/label names with invalid characters was still in the works, hence the draft status. This is ready for review now. |
This new function is similar to `strbuf_print_escaped` but differs in two
important aspects:
* `strbuf_print_restricted` expects a list of acceptable characters,
i.e. an allow list. `strbuf_print_escaped` expects a deny list.
* `strbuf_print_restricted` *replaces* characters not in the allow list.
`strbuf_print_escaped` adds an escape character in front of the
denied character.
octo
force-pushed
the
6/write_prometheus
branch
from
d197258
to
1a56a90
Compare
eero-t
reviewed
Dec 21, 2023
Instead of building the buffer piece by piece, copy the entire string into the buffer and do the replacements there. I think the code is quite efficient, but I haven't profiled either version so can't say for sure that there is a speedup. The new code *may* be easier to reason about, since the "copy and replace" approach has a much simpler loop body than the previous approach.
eero-t
requested changes
Dec 21, 2023
There was a problem hiding this comment.
There's some leftover code, but otherwise looks good!
PS. I think this is the first time I came across C99 static array declarator use, so I learned something new! According to GCC but tracker, although support for them was added in v4.x, only v11.x gives warnings if the specified bounds are not respected: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50584
Co-authored-by: Eero Tamminen <eero.t.tamminen@intel.com>
eero-t
approved these changes
Dec 21, 2023
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.
ChangeLog: Write Prometheus plugin: Escaping for metric and label names has been added. Test coverage has been improved.
Fixes: #4204