-
Notifications
You must be signed in to change notification settings - Fork 0
Correct bugs in metric bars #19
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
Conversation
WalkthroughRefactors Flux queries in the Overview of Devices Grafana dashboard to consistently fetch the latest per-device point via grouping followed by sort(columns:["_time"], desc:true) |> limit(n:1), replaces several last()-based patterns, updates CPU idle→usage mapping, reorganizes AXIS OS targets/transformations, switches some targets to rawQuery, and bumps version to 30. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant G as Grafana Panel
participant Q as Panel Query (Flux)
participant I as InfluxDB
U->>G: Open/refresh Overview of Devices
G->>Q: Execute panel query (may be rawQuery)
Note over Q: group |> sort(_time, desc: true) |> limit(1)\nCPU panels: map(100.0 - _value)
Q->>I: Fetch time-series data
I-->>Q: Latest per-device points
Q-->>G: Return time_series results
Note over G: Apply transformations (timeSeriesTable, merge, organize, renameByName using ${unique_identifier})
G-->>U: Render updated panels and device links
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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: 1
🧹 Nitpick comments (2)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (2)
851-853: Storage path hard-coded; parameterize for portabilityFiltering path == "/usr/local" may not hold across all Axis devices or custom mounts. Consider a dashboard variable (e.g., storage_path) with sensible defaults, or match multiple known paths via regex.
Apply:
- |> filter(fn: (r) => r["path"] == "/usr/local") + |> filter(fn: (r) => r["path"] =~ /${storage_path:regex}/)And add a templating variable storage_path with default "/usr/local".
934-936: SD card path: make configurableSame concern as Storage. Suggest a variable (e.g., sd_path) defaulting to "/var/spool/storage/SD_DISK" to cover device differences or future firmware changes.
- |> filter(fn: (r) => r["path"] == "/var/spool/storage/SD_DISK") + |> filter(fn: (r) => r["path"] =~ /${sd_path:regex}/)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
dashboard-deployments/**
⚙️ CodeRabbit configuration file
This directory contains server-side dashboard examples for visualizing data from FixedIT Data Agent running on Axis devices. These are primarily for DevOps/IT professionals comfortable with deploying Grafana dashboards and InfluxDB servers. Some dashboards work directly with bundled agent configurations, others can be customized. Licensed under Elastic License 2.0. Review for deployment complexity, dashboard usability, and proper licensing compliance. We should make sure that dashboards are portable and does not check in specific host names, or refer to other specific data from a single deployment.
Files:
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
🔇 Additional comments (3)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (3)
590-606: Good fix: replace last() after group with sort+limit for UptimeThis avoids cross-shard stale reads and matches the PR goal. Raw query + time_series format is appropriate for Stat.
768-770: RAM: latest-point retrieval fixedsort(desc: true) |> limit(1) post-group is the robust pattern. Looks good.
1185-1186: Version bump OKDashboard version increment to 30 matches change scope.
...ts/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
Show resolved
Hide resolved
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: 4
♻️ Duplicate comments (1)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (1)
687-689: CPU idle→usage inversion: correct and consistent.Using sort+limit before map is solid. Good fix for metric bar correctness.
🧹 Nitpick comments (3)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (3)
183-183: Shard-safe latest-point retrieval: good; also normalize join key to "host".sort+limit after group is correct. To make the downstream joinByField(byField: "host") robust even if ${unique_identifier} changes, also rename it to "host" here.
- |> keep(columns: ["_time", "${unique_identifier}"]) - |> rename(columns: {_time: "seen"}) + |> keep(columns: ["_time", "${unique_identifier}"]) + |> rename(columns: {_time: "seen", "${unique_identifier}": "host"})
851-853: Parameterize disk path for portability across Axis devices.Hard-coding "/usr/local" may not exist on all Axis devices. Make it a dashboard variable with a sane default.
- |> filter(fn: (r) => r["path"] == "/usr/local") + |> filter(fn: (r) => r["path"] == "${disk_path_user_local}")Add a templating variable (outside this hunk):
{ "name": "disk_path_user_local", "label": "Disk Path (User)", "type": "constant", "query": "/usr/local", "hide": 2 }
934-936: Likewise, make SD card mount path configurable.Different firmware or models can mount SD differently. Use a variable with default "/var/spool/storage/SD_DISK".
- |> filter(fn: (r) => r["path"] == "/var/spool/storage/SD_DISK") + |> filter(fn: (r) => r["path"] == "${disk_path_sd}")Add a templating variable (outside this hunk):
{ "name": "disk_path_sd", "label": "Disk Path (SD)", "type": "constant", "query": "/var/spool/storage/SD_DISK", "hide": 2 }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
dashboard-deployments/**
⚙️ CodeRabbit configuration file
This directory contains server-side dashboard examples for visualizing data from FixedIT Data Agent running on Axis devices. These are primarily for DevOps/IT professionals comfortable with deploying Grafana dashboards and InfluxDB servers. Some dashboards work directly with bundled agent configurations, others can be customized. Licensed under Elastic License 2.0. Review for deployment complexity, dashboard usability, and proper licensing compliance. We should make sure that dashboards are portable and does not check in specific host names, or refer to other specific data from a single deployment.
Files:
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
🔇 Additional comments (4)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (4)
228-230: LGTM: consistent rename to host and firmware version.This aligns perfectly with the join and organize steps.
590-593: LGTM: shard-safe latest uptime sample; rawQuery on for clarity.Query pattern and panel config are appropriate for a stat of the latest value.
768-770: RAM latest-value retrieval: good.Pattern matches the CPU fix; looks correct.
1185-1186: Version bump noted.Dashboard version increment to 30 is appropriate.
...ts/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
Outdated
Show resolved
Hide resolved
...ts/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
Outdated
Show resolved
Hide resolved
...ts/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
Outdated
Show resolved
Hide resolved
...ts/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
Outdated
Show resolved
Hide resolved
6fd3ab2 to
067bbd5
Compare
- last() after group is not robust against multiple shards and could give an older value. This changes to sort+limit instead. - Change CPU to use inverted idle since this is what we use in other visualizations. - Remove unused subqueries
Fixes same issue as the previous commit did for the bars, even if we have not observed any practical problems in the last reports table.
9160df4 to
c73533a
Compare
|
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
♻️ Duplicate comments (1)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (1)
408-426: Device link: use raw value (same as earlier comment)Repeat of the link nit to prefer ${__value.raw} for robustness.
- "url": "/d/${device_details_uid:raw}/device-details?var-bucket=${bucket}&var-device=${__value.text}" + "url": "/d/${device_details_uid:raw}/device-details?var-bucket=${bucket}&var-device=${__value.raw}"
🧹 Nitpick comments (5)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (5)
84-98: Device link: prefer raw value to avoid URL issuesUse ${__value.raw} instead of ${__value.text} to prevent issues with spaces or special characters in device IDs.
- "url": "/d/${device_details_uid:raw}/device-details?var-bucket=${bucket}&var-device=${__value.text}" + "url": "/d/${device_details_uid:raw}/device-details?var-bucket=${bucket}&var-device=${__value.raw}"
183-185: Shard-safe latest-point pattern: LGTMReplacing last() after group with sort+limit is correct and shard-safe. Minor nit: consider “timestamp” as one word in refId for consistency.
- "refId": "Get last time stamp" + "refId": "Get last timestamp"
219-221: Rename refId to avoid “camera” wordingPer repo guidelines, prefer “device” over “camera” here.
- "refId": "Get camera model name" + "refId": "Get device model name"
237-239: Validate variable interpolation in joinByField.byFieldGrafana usually interpolates
$var and $ {var}, but transformations can be stricter. To be safe and consistent with other uses in this panel, switch to ${unique_identifier}. Please verify the join still works after this change.- "byField": "$unique_identifier", + "byField": "${unique_identifier}",
687-689: CPU bar: inversion is correct; consider clamping to [0,100]100 - idle is right. Optional: clamp to [0,100] to guard against noisy metrics producing <0 or >100.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json(17 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
dashboard-deployments/**
⚙️ CodeRabbit configuration file
This directory contains server-side dashboard examples for visualizing data from FixedIT Data Agent running on Axis devices. These are primarily for DevOps/IT professionals comfortable with deploying Grafana dashboards and InfluxDB servers. Some dashboards work directly with bundled agent configurations, others can be customized. Licensed under Elastic License 2.0. Review for deployment complexity, dashboard usability, and proper licensing compliance. We should make sure that dashboards are portable and does not check in specific host names, or refer to other specific data from a single deployment.
Files:
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json
🧠 Learnings (1)
📓 Common learnings
Learnt from: daniel-falk
PR: fixedit-ai/fixedit-data-agent-examples#18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.369Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
🔇 Additional comments (10)
dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json (10)
192-194: External IP latest-point query: LGTMCorrect use of group → sort(desc) → limit(1); keeps per-device field cleanly.
201-203: Local IPs latest-point query: LGTMConsistent with the shard-safe pattern.
210-212: Internet latency latest-point query: LGTMGood alignment with the unit override to seconds in field overrides.
228-230: Firmware version latest-point query: LGTMPattern and selected fields look correct.
249-258: Organize mappings to dynamic identifier: LGTMIndex and rename mappings correctly reference ${unique_identifier} → “Device”.
768-770: RAM bar latest-point query: LGTMConsistent and shard-safe.
851-853: Storage (/usr/local) latest-point query: LGTMFilter path and pattern look good.
934-936: SD Card latest-point query: LGTMCorrect path filter and latest-point retrieval.
590-593: Uptime stat: rawQuery with latest-point pattern: LGTMGroup → sort(desc) → limit(1) is correct; rawQuery/time_series setup is appropriate for a stat.
1198-1198: Version bump: LGTMDashboard version updated to 30.
This was introduced in PR #19 when removing some renaming transformations, but these columns are still renamed to Device meaning that we should not have changed the matcher.
This was introduced in PR #19 when removing some renaming transformations, but these columns are still renamed to Device meaning that we should not have changed the matcher.



Summary by CodeRabbit
New Features
Bug Fixes
Chores