-
Notifications
You must be signed in to change notification settings - Fork 40
Improve Prometheus setup, add injection scripts #57
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
Conversation
Signed-off-by: Fabian Reinartz <freinartz@google.com>
jkohen
left a comment
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.
Nice!
| @@ -0,0 +1,23 @@ | |||
| #!/bin/sh | |||
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.
Can you document how this should be invoked? What is the first argument ($1)?
Same for patch.sh
kube/README.md
Outdated
| * `KUBE_CLUSTER`: cluster name parameter for the sidecar | ||
| * `GCP_REGION`: GCP region parameter for the sidecar | ||
| * `GCP_PROJECT`: GCP project parameter for the sidecar | ||
| * `DATA_DIR`: data directory for the sidecar (`patch.sh` only) |
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.
Doesn't patch.sh require reconfiguring Prometheus to use the /data directory and mount a named volume? If so, how about splitting this into its own section, and explaining how to set up Prometheus correctly?
If I understood correctly, Prometheus Operator already sets the data volume, right?
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.
In general, a Prometheus server would already have a mounted volume. So the DATA_DIR and DATA_VOLUME are just about attaching that volume to the sidecar as well and using the same data dir as Prometheus already does.
Signed-off-by: Fabian Reinartz <freinartz@google.com>
jkohen
left a comment
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.
Thanks so much!
@jkohen required some fiddling but this is working well AFAICS.
I removed the setup with an operator since that repo has very extensive documentation and customization options for setting up full k8s monitoring.
The full deployment now has a proper configuration rather than just trying to scrape all ports it can find. It can also scrape the cadvisor metrics now, which are the most important ones probably.
Maybe we can first point out the injection option in the docs and then also the full deployment with the noted caveat that this is just very basic.