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

Add Jsonnet mixin for deploying alongside kube-prometheus #283

Merged

Conversation

jrobersonaquent
Copy link
Contributor

Description

This is a Jsonnet mixin designed to be used with kube-prometheus that sets up the remote write configs needed to send metrics to fluentd.

This also includes a quick doc on how to configure and use it with kube-prometheus.

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in (tested and working in my company's dev k8s cluster)

Copy link
Contributor

@samjsong samjsong 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 the contribution! LGTM, but I've tagged members of the team so we can take a look

Copy link
Contributor

@frankreno frankreno left a comment

Choose a reason for hiding this comment

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

LGTM to me, thanks for the PR!

Copy link
Contributor

@rvmiller89 rvmiller89 left a comment

Choose a reason for hiding this comment

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

I'm fine with merging the doc about kube-prometheus and how to write the mixin.

However I'm nervous about merging the libsonnet file itself, since we currently have one source of truth for remote write metric configuration, and adding this file introduces the possibility for configuration drift. It is also non-trivial to verify the libsonnet file currently matches our remote write configuration.

For that reason, I'd ask that the libsonnet file either be omitted from this PR, or renamed so something like kube-prometheus-sumo-logic-mixin.libsonnet.example with the doc clarifying this file is only an example and may not be up-to-date with the remote write metric configuration. Ideally, this file could be generated as part of our CI based on our remote write configuration, so that it is always up-to-date.

@jrobersonaquent
Copy link
Contributor Author

@rvmiller89 I renamed it like you suggested and I'm working on another PR to update the Jsonnet file automatically.

Copy link
Contributor

@rvmiller89 rvmiller89 left a comment

Choose a reason for hiding this comment

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

Thanks for updating!

@rvmiller89 rvmiller89 merged commit 5d4889d into SumoLogic:master Nov 21, 2019
@jrobersonaquent jrobersonaquent deleted the add-kube-promethus-support branch November 21, 2019 21:24
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.

4 participants