Preserve underscores in resource group metric tags - #6553
efegokdemir wants to merge 4 commits into
Conversation
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
|
Removing the |
|
Updated the PR description to match the current implementation. Micrometer-compatible formatting is retained; monitor-side processing now resolves the formatted tag against configured resource-group names and preserves ambiguous/non-matching values. The focused MetricsUtilTest and reactor test command pass, and git diff --check passes. Please re-review the current HEAD when convenient. |
| } | ||
| match = validName; | ||
| } | ||
| } |
There was a problem hiding this comment.
It might make sense to collect all valid matches in Set. After checking all, if the Set size is one, then we have a match. If the Set size is greater than one, then we can't determine the actual match. This covers the case where resource-group and resource_group both get formatted to resource.group. Ideally the user would not do this, but it's possible. If there is no match, or multiple matches, then I would throw an IllegalStateException with an appropriate message.
| String queueName = MetricsUtil.resolveResourceGroupName(t.value(), | ||
| configuredCompactionResourceGroups); |
There was a problem hiding this comment.
This should catch the suggested IllegalStateException, log it, and continue to the next tag.
| queueName = MetricsUtil.resolveResourceGroupName(tag.value(), | ||
| configuredCompactionResourceGroups); | ||
| break; |
There was a problem hiding this comment.
This should catch the suggested IllegalStateException and log it
| assertEquals("user.small", | ||
| MetricsUtil.resolveResourceGroupName("user.small", Set.of("user_small", "user-small"))); |
There was a problem hiding this comment.
I don't think this behavior is correct. It's returning a resource group name that is not valid.
dlmarion
left a comment
There was a problem hiding this comment.
I don't think this behavior is correct. It's returning a resource group name that is not valid.
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
|
Addressed the current-head feedback in |
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
|
Updated eae6317 to follow the requested zero/ambiguous-match semantics: MetricsUtil now throws an informative IllegalStateException unless exactly one configured resource group matches. The monitor callers catch it, log the invalid tag, and skip that metric instead of retaining an invalid name. Added/updated focused resolution tests. Validation: |
Summary
Fixes the monitor's resource-group lookup after Micrometer formats metric tag names.
Changes
Testing
Notes
The implementation follows the requested matching approach: it keeps canonical formatting and resolves back to a configured resource-group name only when the formatted value has one unique match.