Skip to content

Conversation

@heliapb
Copy link
Member

@heliapb heliapb commented Dec 31, 2024

From issue #47

@heliapb heliapb changed the title [feat] - Add MonitorsOverlapping [feat] - Add MonitoringOverlapping Dec 31, 2024
@heliapb heliapb force-pushed the feat/analyzeselectors branch from b71de06 to 07b44c0 Compare January 26, 2025 00:05
Signed-off-by: Hélia Barroso <[email protected]>
@heliapb heliapb force-pushed the feat/analyzeselectors branch from 21f0624 to 1778e9c Compare January 26, 2025 01:10
@heliapb heliapb marked this pull request as ready for review January 26, 2025 01:10
@heliapb heliapb requested a review from a team as a code owner January 26, 2025 01:10
Signed-off-by: Hélia Barroso <[email protected]>
podOverlaps := make(map[string][]string)
var overlapErrs []string

for _, servicemonitor := range serviceMonitors.Items {
Copy link
Collaborator

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]>
}
}

for key, pdMonitors := range podOverlaps {
Copy link
Collaborator

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?

Copy link
Member Author

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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants