carverauto/serviceradar

Fork 0

fixing dashboards #2686

Merged

mfreeman451 merged 1 commit from refs/pull/2686/head into staging

2026-01-18 00:27:02 +00:00

mfreeman451 commented

2026-01-18 00:25:47 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2327
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2327
Original created: 2026-01-18T00:25:47Z
Original updated: 2026-01-18T00:27:05Z
Original head: carverauto/serviceradar:fix/device-details-page-missing-sysmon
Original base: staging
Original merged: 2026-01-18T00:27:02Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Bug fix, Enhancement

Description

Refactor sysmon filter resolution to validate data existence before use
Replace identity-based filtering with filter token approach for metrics
Add table panel filtering to device details dashboard
Simplify sysmon presence detection logic

Diagram Walkthrough

flowchart LR
  A["Device Details Load"] --> B["Resolve Sysmon Filters"]
  B --> C["Validate Filter Data"]
  C --> D["Load Metric Sections"]
  D --> E["Build Panels"]
  E --> F["Drop Table Panels"]
  F --> G["Display Dashboard"]

File Walkthrough

Relevant files

Bug fix

show.ex

Refactor sysmon filtering with data validation

web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex

Introduced resolve_sysmon_filter_tokens/3 function to validate sysmon
filters against actual data before using them
Replaced load_sysmon_presence/3 with simpler boolean check based on
filter token existence
Changed load_metric_sections/3 to accept filter tokens instead of
identity map
Added drop_table_panels/1 function to filter out table plugin panels
from dashboard
Refactored sysmon_filter_tokens/1 to sysmon_filter_tokens/3 for better
modularity
Added sysmon_filter_has_data?/3 to query and verify filter data
availability

+59/-70

Formatting

index.ex

Fix whitespace in host count display

web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex

Fixed extra whitespace in host count display text

+1/-1

Imported from GitHub pull request. Original GitHub pull request: #2327 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2327 Original created: 2026-01-18T00:25:47Z Original updated: 2026-01-18T00:27:05Z Original head: carverauto/serviceradar:fix/device-details-page-missing-sysmon Original base: staging Original merged: 2026-01-18T00:27:02Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Refactor sysmon filter resolution to validate data existence before use - Replace identity-based filtering with filter token approach for metrics - Add table panel filtering to device details dashboard - Simplify sysmon presence detection logic ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Device Details Load"] --> B["Resolve Sysmon Filters"] B --> C["Validate Filter Data"] C --> D["Load Metric Sections"] D --> E["Build Panels"] E --> F["Drop Table Panels"] F --> G["Display Dashboard"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>show.ex</strong><dd><code>Refactor sysmon filtering with data validation</code>                      </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex <ul><li>Introduced <code>resolve_sysmon_filter_tokens/3</code> function to validate sysmon <br>filters against actual data before using them<br> <li> Replaced <code>load_sysmon_presence/3</code> with simpler boolean check based on <br>filter token existence<br> <li> Changed <code>load_metric_sections/3</code> to accept filter tokens instead of <br>identity map<br> <li> Added <code>drop_table_panels/1</code> function to filter out table plugin panels <br>from dashboard<br> <li> Refactored <code>sysmon_filter_tokens/1</code> to <code>sysmon_filter_tokens/3</code> for better <br>modularity<br> <li> Added <code>sysmon_filter_has_data?/3</code> to query and verify filter data <br>availability</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2327/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24">+59/-70</a>  </td> </tr> </table></td></tr><tr><td><strong>Formatting</strong></td><td><table> <tr> <td> <details> <summary><strong>index.ex</strong><dd><code>Fix whitespace in host count display</code>                                          </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex - Fixed extra whitespace in host count display text </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2327/files#diff-b2127e71582033bc6dfd2d7397f56bf43c1f7c613defffc504b3d8ee1e7406c4">+1/-1</a>      </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-18 00:26:12 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2327#issuecomment-3764524294
Original created: 2026-01-18T00:26:12Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Silent query failure: `sysmon_filter_has_data?/3` converts any SRQL query error into `false` without logging or surfacing context, which can silently mask underlying failures and hinder debugging. Referred Code `defp sysmon_filter_has_data?(srql_module, filter_tokens, scope) do query = [ "in:cpu_metrics", Enum.join(filter_tokens, " "), "time:last_24h", "sort:timestamp:desc", "limit:1" ] \|> Enum.join(" ") case srql_module.query(query, %{scope: scope}) do {:ok, %{"results" => rows}} when is_list(rows) -> rows != [] _ -> false end` Learn more about managing compliance generic rules or creating your own custom rules
⚪

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2327#issuecomment-3764524294 Original created: 2026-01-18T00:26:12Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=3>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=1>🔴</td> <td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2327/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1257-R1271'><strong>Silent query failure</strong></a>: <code>sysmon_filter_has_data?/3</code> converts any SRQL query error into <code>false</code> without logging or <br>surfacing context, which can silently mask underlying failures and hinder debugging.<br> <details open><summary>Referred Code</summary> ```elixir defp sysmon_filter_has_data?(srql_module, filter_tokens, scope) do query = [ "in:cpu_metrics", Enum.join(filter_tokens, " "), "time:last_24h", "sort:timestamp:desc", "limit:1" ] |> Enum.join(" ") case srql_module.query(query, %{scope: scope}) do {:ok, %{"results" => rows}} when is_list(rows) -> rows != [] _ -> false end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <tr><td align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-01-18 00:26:44 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2327#issuecomment-3764524710
Original created: 2026-01-18T00:26:44Z

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: build

Failed stage: Configure SRQL fixture database for tests [❌]

Failed test name: ""

Failure summary:

The action failed during environment/setup because a required secret was missing:
- The workflow
checks for the SRQL_TEST_DATABASE_CA_CERT secret and aborts when it is empty
(SRQL_TEST_DATABASE_CA_CERT: shows no value in the environment).
- The job explicitly reports:
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. and then exits with
code 1 (line 636–637).

Relevant error logs:

1:  Runner name: 'arc-runner-set-hk6mk-runner-nz4gg'
2:  Runner group name: 'Default'
...

139:  ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m
140:  ^[[36;1m  sudo apt-get update^[[0m
141:  ^[[36;1m  sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m
142:  ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m
143:  ^[[36;1m  sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
144:  ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m
145:  ^[[36;1m  sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
146:  ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m
147:  ^[[36;1m  sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m
148:  ^[[36;1melse^[[0m
149:  ^[[36;1m  echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m
150:  ^[[36;1m  exit 1^[[0m
151:  ^[[36;1mfi^[[0m
152:  ^[[36;1m^[[0m
153:  ^[[36;1mensure_pkg_config^[[0m
154:  ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m
155:  shell: /usr/bin/bash -e {0}
...

316:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
317:  env:
318:  BUILDBUDDY_ORG_API_KEY: ***
319:  SRQL_TEST_DATABASE_URL: ***
320:  SRQL_TEST_ADMIN_URL: ***
321:  SRQL_TEST_DATABASE_CA_CERT: 
322:  DOCKERHUB_USERNAME: ***
323:  DOCKERHUB_TOKEN: ***
324:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
325:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
326:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
327:  ##[endgroup]
328:  ##[group]Run : install rustup if needed
329:  ^[[36;1m: install rustup if needed^[[0m
330:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
331:  ^[[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m
332:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

472:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
473:  env:
474:  BUILDBUDDY_ORG_API_KEY: ***
475:  SRQL_TEST_DATABASE_URL: ***
476:  SRQL_TEST_ADMIN_URL: ***
477:  SRQL_TEST_DATABASE_CA_CERT: 
478:  DOCKERHUB_USERNAME: ***
479:  DOCKERHUB_TOKEN: ***
480:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
481:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
482:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
483:  CARGO_HOME: /home/runner/.cargo
484:  CARGO_INCREMENTAL: 0
485:  CARGO_TERM_COLOR: always
486:  ##[endgroup]
487:  ##[group]Run : work around spurious network errors in curl 8.0
488:  ^[[36;1m: work around spurious network errors in curl 8.0^[[0m
489:  ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m
...

540:  SRQL_TEST_DATABASE_CA_CERT: 
541:  DOCKERHUB_USERNAME: ***
542:  DOCKERHUB_TOKEN: ***
543:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
544:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
545:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
546:  CARGO_HOME: /home/runner/.cargo
547:  CARGO_INCREMENTAL: 0
548:  CARGO_TERM_COLOR: always
549:  ##[endgroup]
550:  Attempting to download 1.x...
551:  Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64
552:  Adding to the cache ...
553:  Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64
554:  Added bazelisk to the path
555:  ##[warning]Failed to restore: Cache service responded with 400
556:  Restored bazelisk cache dir @ /home/runner/.cache/bazelisk
...

622:  env:
623:  BUILDBUDDY_ORG_API_KEY: ***
624:  SRQL_TEST_DATABASE_URL: ***
625:  SRQL_TEST_ADMIN_URL: ***
626:  SRQL_TEST_DATABASE_CA_CERT: 
627:  DOCKERHUB_USERNAME: ***
628:  DOCKERHUB_TOKEN: ***
629:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
630:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
631:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
632:  CARGO_HOME: /home/runner/.cargo
633:  CARGO_INCREMENTAL: 0
634:  CARGO_TERM_COLOR: always
635:  ##[endgroup]
636:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
637:  ##[error]Process completed with exit code 1.
638:  Post job cleanup.

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2327#issuecomment-3764524710 Original created: 2026-01-18T00:26:44Z --- ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** build</td></tr> <tr><td> **Failed stage:** [Configure SRQL fixture database for tests](https://github.com/carverauto/serviceradar/actions/runs/21103180010/job/60690284545) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed during environment/setup because a required secret was missing:<br> - The workflow <br>checks for the <code>SRQL_TEST_DATABASE_CA_CERT</code> secret and aborts when it is empty <br>(<code>SRQL_TEST_DATABASE_CA_CERT:</code> shows no value in the environment).<br> - The job explicitly reports: <br><code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.</code> and then exits with <br>code <code>1</code> (line 636–637).<br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-nz4gg' 2: Runner group name: 'Default' ... 139: ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m 140: ^[[36;1m sudo apt-get update^[[0m 141: ^[[36;1m sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m 142: ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m 143: ^[[36;1m sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 144: ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m 145: ^[[36;1m sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 146: ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m 147: ^[[36;1m sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 148: ^[[36;1melse^[[0m 149: ^[[36;1m echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m 150: ^[[36;1m exit 1^[[0m 151: ^[[36;1mfi^[[0m 152: ^[[36;1m^[[0m 153: ^[[36;1mensure_pkg_config^[[0m 154: ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m 155: shell: /usr/bin/bash -e {0} ... 316: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 317: env: 318: BUILDBUDDY_ORG_API_KEY: *** 319: SRQL_TEST_DATABASE_URL: *** 320: SRQL_TEST_ADMIN_URL: *** 321: SRQL_TEST_DATABASE_CA_CERT: 322: DOCKERHUB_USERNAME: *** 323: DOCKERHUB_TOKEN: *** 324: TEST_CNPG_DATABASE: serviceradar_web_ng_test 325: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 326: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 327: ##[endgroup] 328: ##[group]Run : install rustup if needed 329: ^[[36;1m: install rustup if needed^[[0m 330: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 331: ^[[36;1m curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m 332: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 472: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 473: env: 474: BUILDBUDDY_ORG_API_KEY: *** 475: SRQL_TEST_DATABASE_URL: *** 476: SRQL_TEST_ADMIN_URL: *** 477: SRQL_TEST_DATABASE_CA_CERT: 478: DOCKERHUB_USERNAME: *** 479: DOCKERHUB_TOKEN: *** 480: TEST_CNPG_DATABASE: serviceradar_web_ng_test 481: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 482: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 483: CARGO_HOME: /home/runner/.cargo 484: CARGO_INCREMENTAL: 0 485: CARGO_TERM_COLOR: always 486: ##[endgroup] 487: ##[group]Run : work around spurious network errors in curl 8.0 488: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 489: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 540: SRQL_TEST_DATABASE_CA_CERT: 541: DOCKERHUB_USERNAME: *** 542: DOCKERHUB_TOKEN: *** 543: TEST_CNPG_DATABASE: serviceradar_web_ng_test 544: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 545: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 546: CARGO_HOME: /home/runner/.cargo 547: CARGO_INCREMENTAL: 0 548: CARGO_TERM_COLOR: always 549: ##[endgroup] 550: Attempting to download 1.x... 551: Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64 552: Adding to the cache ... 553: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64 554: Added bazelisk to the path 555: ##[warning]Failed to restore: Cache service responded with 400 556: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 622: env: 623: BUILDBUDDY_ORG_API_KEY: *** 624: SRQL_TEST_DATABASE_URL: *** 625: SRQL_TEST_ADMIN_URL: *** 626: SRQL_TEST_DATABASE_CA_CERT: 627: DOCKERHUB_USERNAME: *** 628: DOCKERHUB_TOKEN: *** 629: TEST_CNPG_DATABASE: serviceradar_web_ng_test 630: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 631: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 632: CARGO_HOME: /home/runner/.cargo 633: CARGO_INCREMENTAL: 0 634: CARGO_TERM_COLOR: always 635: ##[endgroup] 636: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 637: ##[error]Process completed with exit code 1. 638: Post job cleanup. ``` </details></td></tr></table>

qodo-code-review[bot] commented

2026-01-18 00:27:05 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2327#issuecomment-3764524969
Original created: 2026-01-18T00:27:05Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate pre-flight checks into one The `resolve_sysmon_filter_tokens` function makes up to two database queries to find valid metric data. This can be optimized by using a single, combined query to check for data using either `device_id` or `agent_id`, thus reducing database round-trips. Examples: web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [1241-1255] `defp resolve_sysmon_filter_tokens(srql_module, identity, scope) do device_tokens = sysmon_filter_tokens(identity, :device_uid, "device_id") agent_tokens = sysmon_filter_tokens(identity, :agent_id, "agent_id") cond do device_tokens != [] and sysmon_filter_has_data?(srql_module, device_tokens, scope) -> device_tokens agent_tokens != [] and sysmon_filter_has_data?(srql_module, agent_tokens, scope) -> agent_tokens ... (clipped 5 lines)` Solution Walkthrough: Before: `defp resolve_sysmon_filter_tokens(srql_module, identity, scope) do device_tokens = sysmon_filter_tokens(identity, :device_uid, "device_id") agent_tokens = sysmon_filter_tokens(identity, :agent_id, "agent_id") cond do # This makes one DB query device_tokens != [] and sysmon_filter_has_data?(srql_module, device_tokens, scope) -> device_tokens # This can make a second DB query agent_tokens != [] and sysmon_filter_has_data?(srql_module, agent_tokens, scope) -> agent_tokens true -> [] end end` After: defp resolve_sysmon_filter_tokens(srql_module, identity, scope) do # Build tokens for both identifiers device_tokens = sysmon_filter_tokens(identity, :device_uid, "device_id") agent_tokens = sysmon_filter_tokens(identity, :agent_id, "agent_id") # Build a single query with an OR condition # e.g., "in:cpu_metrics (device_id:\"...\" or agent_id:\"...\") limit:1 fields:device_id,agent_id" combined_query = build_combined_check_query(device_tokens, agent_tokens) # Make only one DB query case srql_module.query(combined_query, %{scope: scope}) do {:ok, %{"results" => [row \| _]}} -> # Check which identifier matched and return the corresponding tokens if row["device_id"] != nil, do: device_tokens, else: agent_tokens _ -> [] end end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a performance bottleneck in `resolve_sysmon_filter_tokens` where up to two separate database queries can be executed, and proposes a valid optimization to consolidate them into a single query, improving efficiency.	Medium
General	Handle nil panels safely Refactor `drop_table_panels/1` to explicitly handle `nil` and other non-list inputs by returning an empty list, ensuring it always returns a list. web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [1290-1294] `+defp drop_table_panels(nil), do: [] + defp drop_table_panels(panels) when is_list(panels) do Enum.reject(panels, &(&1.plugin == TablePlugin)) end -defp drop_table_panels(panels), do: panels +defp drop_table_panels(_), do: []` Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion improves code robustness and consistency by ensuring `drop_table_panels/1` always returns a list, aligning its behavior with the similar `drop_low_value_categories/1` function and preventing potential errors in the processing pipeline.	Low
General	Remove double whitespace Remove the extra whitespace in the `span` element to fix a display typo. web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex [1256] `-<span> of {@hosts_processed} hosts</span> +<span> of {@hosts_processed} hosts</span>` Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion correctly identifies and fixes a minor typo (an extra space) introduced in the PR, which improves the UI text formatting.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2327#issuecomment-3764524969 Original created: 2026-01-18T00:27:05Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Consolidate pre-flight checks into one</summary> ___ **The <code>resolve_sysmon_filter_tokens</code> function makes up to two database queries to <br>find valid metric data. This can be optimized by using a single, combined query <br>to check for data using either <code>device_id</code> or <code>agent_id</code>, thus reducing database <br>round-trips.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2327/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1241-R1255">web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [1241-1255]</a> </summary> ```elixir defp resolve_sysmon_filter_tokens(srql_module, identity, scope) do device_tokens = sysmon_filter_tokens(identity, :device_uid, "device_id") agent_tokens = sysmon_filter_tokens(identity, :agent_id, "agent_id") cond do device_tokens != [] and sysmon_filter_has_data?(srql_module, device_tokens, scope) -> device_tokens agent_tokens != [] and sysmon_filter_has_data?(srql_module, agent_tokens, scope) -> agent_tokens ... (clipped 5 lines) ``` </details> ### Solution Walkthrough: #### Before: ```elixir defp resolve_sysmon_filter_tokens(srql_module, identity, scope) do device_tokens = sysmon_filter_tokens(identity, :device_uid, "device_id") agent_tokens = sysmon_filter_tokens(identity, :agent_id, "agent_id") cond do # This makes one DB query device_tokens != [] and sysmon_filter_has_data?(srql_module, device_tokens, scope) -> device_tokens # This can make a second DB query agent_tokens != [] and sysmon_filter_has_data?(srql_module, agent_tokens, scope) -> agent_tokens true -> [] end end ``` #### After: ```elixir defp resolve_sysmon_filter_tokens(srql_module, identity, scope) do # Build tokens for both identifiers device_tokens = sysmon_filter_tokens(identity, :device_uid, "device_id") agent_tokens = sysmon_filter_tokens(identity, :agent_id, "agent_id") # Build a single query with an OR condition # e.g., "in:cpu_metrics (device_id:\"...\" or agent_id:\"...\") limit:1 fields:device_id,agent_id" combined_query = build_combined_check_query(device_tokens, agent_tokens) # Make only one DB query case srql_module.query(combined_query, %{scope: scope}) do {:ok, %{"results" => [row | _]}} -> # Check which identifier matched and return the corresponding tokens if row["device_id"] != nil, do: device_tokens, else: agent_tokens _ -> [] end end ``` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a performance bottleneck in `resolve_sysmon_filter_tokens` where up to two separate database queries can be executed, and proposes a valid optimization to consolidate them into a single query, improving efficiency. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Handle nil panels safely</summary> ___ **Refactor <code>drop_table_panels/1</code> to explicitly handle <code>nil</code> and other non-list inputs <br>by returning an empty list, ensuring it always returns a list.** [web-ng/lib/serviceradar_web_ng_web/live/device_live/show.ex [1290-1294]](https://github.com/carverauto/serviceradar/pull/2327/files#diff-92d8e0af57d2f65dfb8562e896dbea17ef9f47074ffd14f58261decc76f80c24R1290-R1294) ```diff +defp drop_table_panels(nil), do: [] + defp drop_table_panels(panels) when is_list(panels) do Enum.reject(panels, &(&1.plugin == TablePlugin)) end -defp drop_table_panels(panels), do: panels +defp drop_table_panels(_), do: [] ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion improves code robustness and consistency by ensuring `drop_table_panels/1` always returns a list, aligning its behavior with the similar `drop_low_value_categories/1` function and preventing potential errors in the processing pipeline. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Remove double whitespace</summary> ___ **Remove the extra whitespace in the `span` element to fix a display typo.** [web-ng/lib/serviceradar_web_ng_web/live/settings/networks_live/index.ex [1256]](https://github.com/carverauto/serviceradar/pull/2327/files#diff-b2127e71582033bc6dfd2d7397f56bf43c1f7c613defffc504b3d8ee1e7406c4R1256-R1256) ```diff -<span> of {@hosts_processed} hosts</span> +<span> of {@hosts_processed} hosts</span> ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 2</summary> __ Why: The suggestion correctly identifies and fixes a minor typo (an extra space) introduced in the PR, which improves the UI text formatting. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>