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

agent-static-mixin: include custom 'all' value #6589

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

tpaschalis
Copy link
Member

This makes it for easier querying on the backend of your choice. I've verified by comparing the resulting dashboards in main and the fix branch

mixtool generate dashboards mixin.libsonnet --directory fix1

I'm adding the new parameter on the existing helper instead of refactoring to use addMultiTemplateWithAll, should be enough for now.

Copy link

@pracucci pracucci 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 working on it. I left a nit.

@@ -286,6 +286,7 @@ local template = grafana.template;
value: '$__all',
},
includeAll=true,
allValues='.*',
Copy link

Choose a reason for hiding this comment

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

I know I suggested you .* but .+ may actually be slightly more accurate. That's also what jsonnet-libs does by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, let's change it. I had both addMultiTemplate and addMultiTemplateWithAll helpers with a different default, changing it now.

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
@tpaschalis tpaschalis force-pushed the fix-static-mixin-all-selector branch from 88c0327 to 4c04185 Compare March 4, 2024 13:02
@tpaschalis tpaschalis enabled auto-merge (squash) March 4, 2024 13:03
Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
Copy link

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks. I think what you're doing in this PR is consistent with jsonnet-libs.

@tpaschalis tpaschalis merged commit a0f5d8a into grafana:main Mar 4, 2024
9 of 10 checks passed
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Apr 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants