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 overrides exporter #198

Merged
merged 1 commit into from
Oct 12, 2020
Merged

Conversation

simonswine
Copy link
Contributor

What this PR does:
Add overrides exporter deployment (which is part of grafana/cortex-tools) that exposes runtime
overrides and related presets.

Checklist

  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@simonswine simonswine requested a review from a team as a code owner October 9, 2020 14:53
@simonswine simonswine force-pushed the add-overrides-exporter branch from 7ff1366 to bf06d1d Compare October 9, 2020 14:53
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

LGTM, one typo, and a proposal to refactor as a mixin instead of using a boolean to enable.

cortex/config.libsonnet Outdated Show resolved Hide resolved
// this enables overrides exporter, which will expose the configured
// overrides and presets (if configured). Those metrics can be potentially
// high cardinatilty.
overrides_exporter_enabled:: false,
Copy link
Member

Choose a reason for hiding this comment

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

I think we probably want to avoid this bool for enabling the exporter and instead document how users can import it if they want it. It saves the repeated if enabled then logic that you have to do for each object which is nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can take a look at this updated version, I basically followed the test exporter with the setup before.

Not too sure how and where to document it, I think right now there is not much docs or am I missing something?

@simonswine simonswine force-pushed the add-overrides-exporter branch 2 times, most recently from d19c703 to fad3308 Compare October 9, 2020 17:09
Copy link
Contributor

@jtlisi jtlisi left a comment

Choose a reason for hiding this comment

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

LGTM

local name = 'overrides-exporter',

_config+: {
// overrides exporter can also make the previously defined presets available, this list references the
Copy link
Contributor

Choose a reason for hiding this comment

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

Did this sentence get cut off?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, fixed it

Overrides exporter part of grafana/cortex-tools and exposes runtime
overrides and related presets of Cortex as metrics.

Signed-off-by: Christian Simon <[email protected]>
@simonswine simonswine force-pushed the add-overrides-exporter branch from fad3308 to 3b391b6 Compare October 12, 2020 08:44
@simonswine simonswine merged commit eed86d2 into grafana:master Oct 12, 2020
simonswine added a commit to grafana/mimir that referenced this pull request Dec 20, 2021
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