-
Notifications
You must be signed in to change notification settings - Fork 5
[feat] - Add MonitoringOverlapping #50
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
base: main
Are you sure you want to change the base?
Conversation
b71de06 to
07b44c0
Compare
Signed-off-by: Hélia Barroso <[email protected]>
21f0624 to
1778e9c
Compare
Signed-off-by: Hélia Barroso <[email protected]>
| podOverlaps := make(map[string][]string) | ||
| var overlapErrs []string | ||
|
|
||
| for _, servicemonitor := range serviceMonitors.Items { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's required for a first version, but would be nice if we can also compare pod monitors with service monitors.
Imagine a use case where you deploy one pod monitor and one service monitor to collect metrics from the same service.
Signed-off-by: Hélia Barroso <[email protected]>
Signed-off-by: Hélia Barroso <[email protected]>
internal/analyzers/overlapping.go
Outdated
| } | ||
| } | ||
|
|
||
| for key, pdMonitors := range podOverlaps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we properly build the error message inside each for we don't nee to iterate it again to rebuild the error mesage, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, mv the error inside the overlapping function, wdyt?
Signed-off-by: Hélia Barroso <[email protected]>
From issue #47