Skip to content
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

feat(examples,metrics,kube-state-metrics): add configmap and promethe… #10919

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

sebastiangaiser
Copy link
Contributor

@sebastiangaiser sebastiangaiser commented Dec 6, 2024

…us-rules

In order to implement https://github.com/strimzi/proposals/blob/main/087-monitoring-of-custom-resources.md an initial version of the configmap and prometheus-rules are added for 'KafkaUser' and 'KafkaTopic' resources.

Part of #10276

Type of change

  • Enhancement / new feature

Description

See commit message

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

…us-rules

In order to implement https://github.com/strimzi/proposals/blob/main/087-monitoring-of-custom-resources.md an initial version of the configmap and prometheus-rules are added for 'KafkaUser' and 'KafkaTopic' resources.

Part of strimzi#10276

Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser
Copy link
Contributor Author

sebastiangaiser commented Dec 6, 2024

I'm unsure wehter KafkaTopic could also have a DeprecatedFields status field or not.
In addition a Deployment and PodMonitor should be added.

@scholzj
Copy link
Member

scholzj commented Dec 6, 2024

I'm unsure wheter KafkaTopic could also have a DeprecatedFields status field or not.

At least in theory, all resources might have those. But I think for example KafkaTopic does not have anything deprecated right now. But I would have it covered for all resources and in the worst case, the alert will never be raised. If you don't add it now, we will likely forget to add it if we one day deprecate something there.

Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser
Copy link
Contributor Author

sebastiangaiser commented Dec 10, 2024

But I would have it covered for all resources

@scholzj I added the deprecated field assuming it follows the same structure in the status object.

Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. But it would be great to probably:

@scholzj scholzj added this to the 0.46.0 milestone Dec 11, 2024
@ppatierno
Copy link
Member

I had a first pass and agree with Jakub on what's missing. I will have another pass when the upgrades are committed.

@sebastiangaiser
Copy link
Contributor Author

sebastiangaiser commented Dec 26, 2024

I've tried to address your points:

  • added a README in the example directory
  • added a section to the documentation. Tbh I've never worked with AsciiDoc before, so please guide me a bit here ;)

In addition I added a KSM deployment (similar as it already exists for Grafana) for clarifying how a Kubernetes deployment could look like. I've decided against the PodMonitor and for the Service + ServiceMonitor for simpler docs. Hope this is fine for you, too.

Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

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.

3 participants