misc(metrics) space as metadata separator in metric name #618
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #618 +/- ##
===================================================
- Coverage 90.84256% 90.82880% -0.01377%
===================================================
Files 52 52
Lines 11204 11209 +5
===================================================
+ Hits 10178 10181 +3
- Misses 1026 1028 +2
... and 2 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. |
WasmX metrics defined in Proxy-Wasm filters are prefixed with `pw` and the name of the filter in which they were defined, e.g. `pw a_filter a_metric`. Guaranteeing that WASM module names don't contain tabs or spaces makes extraction of filter name from metric names simpler.
casimiro
force-pushed
the
misc/metrics-name-separator
branch
from
d821308 to
b079790
Compare
thibaultcha
reviewed
Oct 29, 2024
| always prefixed with: `pw.{filter_name}.{metric_name}`. This means that a metric | ||
| named `a_counter` inserted by `a_filter` will have its name stored as: | ||
| `pw.a_filter.a_counter`. | ||
| always prefixed with space-separated metadata: `pw {filter_name} `. This means |
There was a problem hiding this comment.
Reading this I realized space might not be the ideal character; it's easy to miss the trailing space in docs describing pw {filter_name} . Would it be fine if we used colons like pw:{filter_name}:? I opened #619 for merge if that's alright.
There was a problem hiding this comment.
Indeed, colons are more easily spotted than spaces.
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.
WasmX metrics defined in Proxy-Wasm filters have their names prefixed by
pw.{a_filter}where{a_filter}is the name of the filter where the metric was defined, e.g.pw.a_filter.a_counter.When the Gateway serializes a WasmX metric defined in a Proxy-Wasm filter it needs to extract the filter name from the metric name. The filter name is used to load potential label patterns associated with the filter which are then used to extract potential labels from the metric name.
This PR proposes changing the separator in metric names from the dot to the space (e.g.
pw a_filter a_counter) and forbidding space-like characters in WASM modules names; which simplifies the task of extracting the filter name from a metric name.