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

Alloy-Mixin: allow config overwrite #934

Merged
merged 9 commits into from
Jun 6, 2024
Merged

Conversation

gaantunes
Copy link
Contributor

@gaantunes gaantunes commented May 27, 2024

PR Description

Implementing the following changes:

  • Allow overwriting the config values for dashboards, instead of always pulling it from the local config file.
  • grafanaDashboards and prometheusAlerts hidden, according to the mixin schema.
  • allow filter selector for metrics to avoid Grafana Agent metrics query collision, and setting default to job="integrations/self"
  • allow filter selector for logs to avoid pulling logs from other platforms, and setting default to service_name="alloy"
  • fixing bugs reported by @rfratto in the comment below

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@gaantunes gaantunes requested a review from rfratto May 28, 2024 13:05
@rfratto
Copy link
Member

rfratto commented May 28, 2024

@gaantunes While we're working through fixes for #808, I found a few other issues that PR introduced:

  • Some panels are broken:
    • The "Nodes" and "Node table" panels in "Alloy / Cluster Overview" aren't using string substitution properly and so %(groupSelector)s is appearing literally in the panel query.
    • The "Component evaluation histogram" panel in "Alloy / Controller" has a stray ' at the end of the query breaking it
  • The "Alloy logs overview" dashboard is missing the alloy-mixin tag present on all other dashboards.
  • The "Alloy logs overview" dashboard doesn't align with the naming conventions of the other dashboards; Alloy / Logs Overview would be more in-line with the other dashboard names.

You can reproduce this locally if you navigate to the example directory and run

docker compose --profile=alloy up -d

then navigate to http://localhost:3000 for the Grafana instance it spawns. The Docker Compose environment also watches for changes to the mixin so you can test changes live.

After you're done, you can tear it down with

docker compose --profile=alloy down

@gaantunes
Copy link
Contributor Author

@rfratto should be ready for a re-review.

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

(sorry, I had reviewed this last week but never submitted my review 😞)

operations/alloy-mixin/dashboards.libsonnet Outdated Show resolved Hide resolved
operations/alloy-mixin/dashboards/prometheus.libsonnet Outdated Show resolved Hide resolved
operations/alloy-mixin/config.libsonnet Outdated Show resolved Hide resolved
operations/alloy-mixin/config.libsonnet Outdated Show resolved Hide resolved
operations/alloy-mixin/dashboards/utils/dashboard.jsonnet Outdated Show resolved Hide resolved
@gaantunes gaantunes requested a review from rfratto June 6, 2024 10:44
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Thanks, this is coming along well!

@gaantunes gaantunes requested a review from rfratto June 6, 2024 19:26
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

I skimmed through the diffs in our internal deployment of the mixin, looks good. Thanks!

@rfratto rfratto merged commit 10da36a into main Jun 6, 2024
18 checks passed
@rfratto rfratto deleted the gaantunes/mixin-bug-fix branch June 6, 2024 19:43
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants