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: initial Prometheus analyzers #855

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

pintohutch
Copy link
Contributor

@pintohutch pintohutch commented Jan 10, 2024

Closes #

📑 Description

Added a prometheus integration with two analyzers:

  1. PrometheusConfigValidate
  2. PrometheusConfigRelabelReport

The integration does not deploy any Prometheus stack in the cluster.
Instead, it searches the provided --namespace for a Prometheus
configuration, stored in a ConfigMap or Secret. If it finds one, it
unmarshals it into memory and runs the analyzers on it.

PrometheusConfigValidate checks if the actual Prometheus configuration is valid or has
any errors.

PrometheusConfigRelabelReport tries to distill the scrape config
relabeling rules to give a concise label set per job that targets need
to have to be scraped. This analyzer is unconventional, in that it does
not necessarily mean there are issues with the config. It merely tries
to give a human-readable explanation of the relabel rules it discovers,
leaning on the LLM and prompt.

Tested on both kube-prometheus and Google Managed Prometheus
stacks.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

@pintohutch pintohutch changed the title Initial Prometheus analyzers feat: initial Prometheus analyzers Jan 10, 2024
@pintohutch pintohutch force-pushed the prom-analyzer branch 4 times, most recently from 8ea968e to 7a65e21 Compare January 10, 2024 20:23
@pintohutch
Copy link
Contributor Author

Quick demo:

k8sgpt-prom-demo.mov

The output isn't perfect. I'm betting we can improve with a better prompt...

@pintohutch pintohutch marked this pull request as ready for review January 10, 2024 20:29
@pintohutch pintohutch requested review from a team as code owners January 10, 2024 20:29
@pintohutch
Copy link
Contributor Author

Also for the PrometheusConfigRelabelReport analyzer, the entire relabel config is inserted into the prompt. This may not be ideal and prints a verbose "error" message.

Maybe there's a better way to do this as well.

Copy link
Contributor

@matthisholleville matthisholleville left a comment

Choose a reason for hiding this comment

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

Thank you for this awesome contribution 🤩 . I'm really excited to add this integration. Left some comments

pkg/ai/prompts.go Outdated Show resolved Hide resolved
pkg/integration/prometheus/config_analyzer.go Show resolved Hide resolved
pkg/integration/prometheus/config_analyzer.go Show resolved Hide resolved
pkg/integration/prometheus/config_analyzer.go Outdated Show resolved Hide resolved
pkg/integration/prometheus/prometheus.go Show resolved Hide resolved
Copy link
Contributor Author

@pintohutch pintohutch left a comment

Choose a reason for hiding this comment

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

Thanks @matthisholleville - pushed some changes per your comments!

pkg/ai/prompts.go Outdated Show resolved Hide resolved
pkg/integration/prometheus/config_analyzer.go Show resolved Hide resolved
pkg/integration/prometheus/config_analyzer.go Show resolved Hide resolved
pkg/integration/prometheus/config_analyzer.go Outdated Show resolved Hide resolved
pkg/integration/prometheus/prometheus.go Show resolved Hide resolved
pkg/integration/prometheus/prometheus.go Show resolved Hide resolved
@AlexsJones
Copy link
Member

Overall comment - great contribution and thank you for helping us figure out what we want integrations to be. I am glad the pattern itself is holding up okay and kudos to figuring it out.

I do think there should be some guard rails if you go run main.go integration activate prometheus without having prometheus installed -perhaps use the Deploy() function to check if there is a prometheus pod in the namespace with a simple regex? At least have a warning message if we don't find something.

In future, ( maybe kubecon ) the contributors need to talk about that wider vision of how integrations will look - I am very excited to have a Prometheus component though.

@pintohutch
Copy link
Contributor Author

pintohutch commented Jan 11, 2024

Demo:
https://github.com/k8sgpt-ai/k8sgpt/assets/3731548/bafcec60-b772-4d5d-8288-a9511fdef3ac

Thanks @AlexsJones - added the functionality to Deploy() per suggestion and included some logging and comments with context.

Added a prometheus integration with two analyzers:
1. PrometheusConfigValidate
2. PrometheusConfigRelabelReport

The integration does not deploy any Prometheus stack in the cluster.
Instead, it searches the provided --namespace for a Prometheus
configuration, stored in a ConfigMap or Secret. If it finds one, it
unmarshals it into memory and runs the analyzers on it.

PrometheusConfigValidate checks if the actual Prometheus configuration is valid or has
any errors.

PrometheusConfigRelabelReport tries to distill the scrape config
relabeling rules to give a concise label set per job that targets need
to have to be scraped. This analyzer is unconventional, in that it does
not necessarily mean there are issues with the config. It merely tries
to give a human-readable explanation of the relabel rules it discovers,
leaning on the LLM and prompt.

Tested on both kube-prometheus and Google Managed Prometheus
stacks.

Signed-off-by: Daniel Clark <danielclark@google.com>
Simplify ConfigValidate prompt and add comments.

Signed-off-by: Daniel Clark <danielclark@google.com>
Add Prometheus configuration discovery to integration activate command.

Also improve logging to make this more clear to users.

Signed-off-by: Daniel Clark <danielclark@google.com>
Copy link
Contributor

@matthisholleville matthisholleville left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexsJones AlexsJones merged commit 45fa827 into k8sgpt-ai:main Jan 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants