promtool: fix recording inconsistency due to duplicate labels #6026
Conversation
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
|
@brian-brazil please have a look |
cmd/promtool/main.go
Outdated
| @@ -303,9 +304,30 @@ func checkRules(filename string) (int, []error) { | |||
| numRules += len(rg.Rules) | |||
| } | |||
|
|
|||
| duplicateLabels := checkDuplicateLabels(rgs.Groups) | |||
| if duplicateLabels != 0 { | |||
| fmt.Printf("%d duplicate label(s) found. Might cause inconsistency while recording expressions.\n", duplicateLabels) | |||
There was a problem hiding this comment.
It'd be more useful to print the relevant labels. You should also check the metric name.
There was a problem hiding this comment.
Sure. I will implement it in the following commit.
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
Harkishen-Singh
force-pushed
the
fix-issue-5529
branch
from
11f074e
to
3507f61
Compare
|
@brian-brazil implemented the suggestions! please have a look. |
cmd/promtool/main.go
Outdated
| var stackLabels []map[string]string | ||
| var stackMetrics []string | ||
| for _, props := range rule.Rules { | ||
| for _, labels := range stackLabels { |
There was a problem hiding this comment.
A map would be more efficient, and easier to understant
cmd/promtool/main.go
Outdated
| } | ||
| stackLabels = append(stackLabels, props.Labels) | ||
|
|
||
| for _, mname := range stackMetrics { |
There was a problem hiding this comment.
You need to check the metric names and labels at the same time, not independently.
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
|
@brian-brazil done! please have a look. |
cmd/promtool/main.go
Outdated
| stackDuplicates = append(stackDuplicates, duplicateRules{metric: props.Record, label: props.Labels}) | ||
| } | ||
| } | ||
| stack = append(stack, duplicateRules{metric: props.Record, label: props.Labels}) |
There was a problem hiding this comment.
Building this up is odd, I'd suggest using an index variable in the for.
There was a problem hiding this comment.
More exactly, get rid of the stack - you don't need it. Nested iteration over the rules will do it.
There was a problem hiding this comment.
@brian-brazil, done! please have a look.
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
|
@brian-brazil, implemented index values in the necessary for loops. Please have a look. |
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
|
@brian-brazil , as suggested, I have removed the |
|
Thanks! |
krasi-georgiev
changed the title
Signed-off-by: Harkishen-Singh harkishensingh@hotmail.com
Refers #5529. It warns the user from inconsistent data which might occur if more than one labels in the same group are found.