-
Notifications
You must be signed in to change notification settings - Fork 690
fix: dashboard fix #3040
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
fix: dashboard fix #3040
Conversation
WalkthroughThis PR updates Loki query operators in two Grafana resources from case-insensitive regex (|~ "(?i)...") to substring match (|= "..."). It also adds allValue: ".+" to three dashboard template variables (Namespace, DynamoGraph Deployment, Component) to enable an All option pattern. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
deploy/logging/grafana/logging-dashboard.yaml (2)
69-69: Substring match is case-sensitive; confirm behavior change from previous case-insensitive regex.Switching from
|~ "(?i)$var"to|= "$var"drops regex capability and case-insensitive matching. If users expect case-insensitive search for$search/$trace_id, consider retaining|~ "(?i)${search:regex}"/|~ "(?i)${trace_id:regex}", or document the new behavior near the inputs.
128-128: Same as above: solid fix for “All” on deployment selector.Consistency with namespace is good. Optional: align the variable’s
"regex"filter to".+"as well for symmetry with the Namespace var (UX nit).deploy/logging/grafana/dashboard.json (2)
59-59: Substring match is case-sensitive; confirm behavior change from previous case-insensitive regex.Changing to
|= "$search" |= "$trace_id"removes regex and case-insensitive matching. If that’s unintended, revert to|~ "(?i)${search:regex}" |~ "(?i)${trace_id:regex}", or clearly indicate “case-sensitive substring” in the input labels.
118-118: Good:allValue: ".+"for Deployment.Optional: align this variable’s
"regex"field to".+"like Namespace to keep UX consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deploy/logging/grafana/dashboard.json(4 hunks)deploy/logging/grafana/logging-dashboard.yaml(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (4)
deploy/logging/grafana/logging-dashboard.yaml (2)
103-103: Good:allValue: ".+"prevents empty/invalid matchers.This ensures the label regex is non-empty when “All” is selected, satisfying Loki’s requirement for a non-empty selector. Verify that no label values can be empty;
. +excludes empty strings.
153-153: Same as above: solid fix for “All” on component selector.Prevents empty regex and keeps queries valid.
deploy/logging/grafana/dashboard.json (2)
93-93: Good:allValue: ".+"for Namespace avoids empty selector issues.Matches the fix intent and keeps the stream selector valid even with “All.”
143-143: Good:allValue: ".+"for Component.Prevents empty regex; keeps queries valid with “All.”
0ac9a92 to
9c986ad
Compare
Signed-off-by: furkhan324@berkeley.edu <furkhan324@berkeley.edu>
Signed-off-by: furkhan324@berkeley.edu <furkhan324@berkeley.edu> Signed-off-by: zhongdaor <zhongdaor@nvidia.com>
Signed-off-by: furkhan324@berkeley.edu <furkhan324@berkeley.edu> Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
Signed-off-by: furkhan324@berkeley.edu <furkhan324@berkeley.edu> Signed-off-by: Kristen Kelleher <kkelleher@nvidia.com>
Overview:
Dashboard fixes:
Summary by CodeRabbit