carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

wip netflow layout #2832

Merged
mfreeman451 merged 1 commit from refs/pull/2832/head into staging 2026-02-02 05:08:06 +00:00
Conversation 3 Commits 1 Files changed 5 +1116 -461
mfreeman451 commented 2026-02-02 04:59:22 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #2668
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2668
Original created: 2026-02-02T04:59:22Z
Original updated: 2026-02-02T05:08:08Z
Original head: carverauto/serviceradar:2655-chore-sidebar-nav-cleanup---observability
Original base: staging
Original merged: 2026-02-02T05:08:06Z 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

Enhancement

Description

  • Consolidate NetFlow, Events, and Alerts into Observability unified view

  • Remove standalone NetFlow page and integrate as observability tab

  • Add Events and Alerts tabs to observability dashboard

  • Implement summary panels and tables for events, alerts, and netflows

  • Update sidebar navigation to group observability features

Diagram Walkthrough

flowchart LR
  A["Sidebar Navigation"] -->|consolidate| B["Observability Hub"]
  C["NetFlow Page"] -->|migrate| B
  D["Events"] -->|add tab| B
  E["Alerts"] -->|add tab| B
  B -->|display| F["Logs Tab"]
  B -->|display| G["Traces Tab"]
  B -->|display| H["Metrics Tab"]
  B -->|display| I["Events Tab"]
  B -->|display| J["Alerts Tab"]
  B -->|display| K["NetFlow Tab"]

File Walkthrough
Relevant files
Enhancement
layouts.ex
Refactor sidebar navigation for observability consolidation

web-ng/lib/serviceradar_web_ng_web/components/layouts.ex

  • Convert embed_templates and attr calls to use parentheses syntax
  • Remove standalone NetFlow, Events, and Alerts sidebar links
  • Consolidate observability features under single Observability sidebar
    item
  • Update active state logic to include all observability sub-features
 +23/-38 
index.ex
Add Events, Alerts, and NetFlow tabs to Observability       

web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex

  • Add default limit constants for events, alerts, and netflows
  • Initialize events, alerts, and netflows data structures in mount
  • Add events, alerts, and netflows tabs to observability interface
  • Implement event_summary, alert_summary, and netflow_summary components
  • Add events_table, alerts_table, and netflows_table rendering
    components
  • Implement helper functions for event/alert/netflow data formatting and
    filtering
  • Add netflow_presets component for quick filtering
  • Update tab routing logic to support new entity types with dynamic
    limits
  • Add compute functions for event, alert, and netflow summaries
 +1073/-56
index.ex
Remove standalone NetFlow page                                                     

web-ng/lib/serviceradar_web_ng_web/live/netflow_live/index.ex

  • Delete entire standalone NetFlow live view module
  • Functionality migrated to LogLive.Index observability dashboard
 +0/-366 
catalog.ex
Add NetFlow entity to SRQL catalog                                             

web-ng/lib/serviceradar_web_ng_web/srql/catalog.ex

  • Add new flows entity to SRQL catalog for NetFlow queries
  • Configure NetFlow entity with appropriate default time, sort fields,
    and filter fields
  • Enable NetFlow data to be queryable through SRQL interface
 +19/-0   
Configuration changes
router.ex
Route NetFlow to observability dashboard                                 

web-ng/lib/serviceradar_web_ng_web/router.ex

  • Redirect /netflows route to LogLive.Index instead of NetflowLive.Index
  • Consolidate NetFlow viewing into observability dashboard
 +1/-1     
Imported from GitHub pull request. Original GitHub pull request: #2668 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2668 Original created: 2026-02-02T04:59:22Z Original updated: 2026-02-02T05:08:08Z Original head: carverauto/serviceradar:2655-chore-sidebar-nav-cleanup---observability Original base: staging Original merged: 2026-02-02T05:08:06Z 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** Enhancement ___ ### **Description** - Consolidate NetFlow, Events, and Alerts into Observability unified view - Remove standalone NetFlow page and integrate as observability tab - Add Events and Alerts tabs to observability dashboard - Implement summary panels and tables for events, alerts, and netflows - Update sidebar navigation to group observability features ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Sidebar Navigation"] -->|consolidate| B["Observability Hub"] C["NetFlow Page"] -->|migrate| B D["Events"] -->|add tab| B E["Alerts"] -->|add tab| B B -->|display| F["Logs Tab"] B -->|display| G["Traces Tab"] B -->|display| H["Metrics Tab"] B -->|display| I["Events Tab"] B -->|display| J["Alerts Tab"] B -->|display| K["NetFlow Tab"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>layouts.ex</strong><dd><code>Refactor sidebar navigation for observability consolidation</code></dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/components/layouts.ex <ul><li>Convert <code>embed_templates</code> and <code>attr</code> calls to use parentheses syntax<br> <li> Remove standalone NetFlow, Events, and Alerts sidebar links<br> <li> Consolidate observability features under single Observability sidebar <br>item<br> <li> Update active state logic to include all observability sub-features</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2668/files#diff-75eaced063e3e7325588594e742492bab789a979319a34f2f623f6d161cfffd2">+23/-38</a>&nbsp; </td> </tr> <tr> <td> <details> <summary><strong>index.ex</strong><dd><code>Add Events, Alerts, and NetFlow tabs to Observability</code>&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex <ul><li>Add default limit constants for events, alerts, and netflows<br> <li> Initialize events, alerts, and netflows data structures in mount<br> <li> Add events, alerts, and netflows tabs to observability interface<br> <li> Implement event_summary, alert_summary, and netflow_summary components<br> <li> Add events_table, alerts_table, and netflows_table rendering <br>components<br> <li> Implement helper functions for event/alert/netflow data formatting and <br>filtering<br> <li> Add netflow_presets component for quick filtering<br> <li> Update tab routing logic to support new entity types with dynamic <br>limits<br> <li> Add compute functions for event, alert, and netflow summaries</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321">+1073/-56</a></td> </tr> <tr> <td> <details> <summary><strong>index.ex</strong><dd><code>Remove standalone NetFlow page</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/live/netflow_live/index.ex <ul><li>Delete entire standalone NetFlow live view module<br> <li> Functionality migrated to LogLive.Index observability dashboard</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2668/files#diff-7ec566a3a20e36d101f72bb9ee19620be76b397c1d88a1edc6d9a88e8da6909e">+0/-366</a>&nbsp; </td> </tr> <tr> <td> <details> <summary><strong>catalog.ex</strong><dd><code>Add NetFlow entity to SRQL catalog</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/srql/catalog.ex <ul><li>Add new <code>flows</code> entity to SRQL catalog for NetFlow queries<br> <li> Configure NetFlow entity with appropriate default time, sort fields, <br>and filter fields<br> <li> Enable NetFlow data to be queryable through SRQL interface</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2668/files#diff-80c07c25c17e48bf86860ec91db10256b7700a863d5371798e1893e741dc0e15">+19/-0</a>&nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>router.ex</strong><dd><code>Route NetFlow to observability dashboard</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> web-ng/lib/serviceradar_web_ng_web/router.ex <ul><li>Redirect <code>/netflows</code> route to <code>LogLive.Index</code> instead of <code>NetflowLive.Index</code><br> <li> Consolidate NetFlow viewing into observability dashboard</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2668/files#diff-df516cd33165cd85914c1ccb3ff6511d3fe688d4a66498b99807958998c5d07c">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2026-02-02 05:00:05 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2668#issuecomment-3832918620
Original created: 2026-02-02T05:00:05Z

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
🟡
🎫 #2655
🟢 Move Netflow off the main sidebar navigation and into the Observability page as a new tab.
Move Alerts off the main sidebar navigation and into the Observability page as a new tab.
Move Events off the main sidebar navigation and into the Observability page as a new tab.
⚪ Confirm in the running UI that the sidebar no longer shows standalone Netflow, Alerts, and
Events items, and that they appear and behave correctly as tabs under Observability
(including active-state highlighting and navigation behavior).
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

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: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

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:
Invalid row navigation: The new events/alerts table rows can navigate to /events/unknown or /alerts/unknown when
an item lacks an id, instead of gracefully disabling navigation or handling the missing
identifier.

Referred Code 
            phx-click={JS.navigate(~p"/events/#{event_id(event)}")}
          >
            <td class="whitespace-nowrap text-xs font-mono">
              {format_event_timestamp(event)}
            </td>
            <td class="whitespace-nowrap text-xs">
              <.event_severity_badge value={Map.get(event, "severity")} />
            </td>
            <td class="whitespace-nowrap text-xs truncate max-w-[12rem]" title={event_source(event)}>
              {event_source(event)}
            </td>
            <td class="text-xs truncate max-w-[32rem]" title={event_message(event)}>
              {event_message(event)}
            </td>
          </tr>
        <% end %>
      </tbody>
    </table>
  </div>
  """
end


 ... (clipped 198 lines)

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/2668#issuecomment-3832918620 Original created: 2026-02-02T05:00:05Z --- ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/337ea5380d245bc529070171813165371e779c23 --> 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>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2655>#2655</a></summary> <table width='100%'><tbody> <tr><td rowspan=3>🟢</td> <td>Move <code>Netflow</code> off the main sidebar navigation and into the <code>Observability</code> page as a new tab.</td></tr> <tr><td>Move <code>Alerts</code> off the main sidebar navigation and into the <code>Observability</code> page as a new tab.</td></tr> <tr><td>Move <code>Events</code> off the main sidebar navigation and into the <code>Observability</code> page as a new tab.</td></tr> <tr><td rowspan=1>⚪</td> <td>Confirm in the running UI that the sidebar no longer shows standalone <code>Netflow</code>, <code>Alerts</code>, and <br><code>Events</code> items, and that they appear and behave correctly as tabs under <code>Observability</code> <br>(including active-state highlighting and navigation behavior).</td></tr> </tbody></table> </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=5>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<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: 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: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<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/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321R1331-R1549'><strong>Invalid row navigation</strong></a>: The new events/alerts table rows can navigate to <code>/events/unknown</code> or <code>/alerts/unknown</code> when <br>an item lacks an id, instead of gracefully disabling navigation or handling the missing <br>identifier.<br> <details open><summary>Referred Code</summary> ```elixir phx-click={JS.navigate(~p"/events/#{event_id(event)}")} > <td class="whitespace-nowrap text-xs font-mono"> {format_event_timestamp(event)} </td> <td class="whitespace-nowrap text-xs"> <.event_severity_badge value={Map.get(event, "severity")} /> </td> <td class="whitespace-nowrap text-xs truncate max-w-[12rem]" title={event_source(event)}> {event_source(event)} </td> <td class="text-xs truncate max-w-[32rem]" title={event_message(event)}> {event_message(event)} </td> </tr> <% end %> </tbody> </table> </div> """ end ... (clipped 198 lines) ``` </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 align="center" colspan="2"> <!-- placeholder --> <!-- /compliance --update_compliance=true --> </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-02-02 05:00:19 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2668#issuecomment-3832919166
Original created: 2026-02-02T05:00:19Z

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/secret validation because the SRQL_TEST_DATABASE_CA_CERT value
was empty/unset.
The workflow requires SRQL_TEST_DATABASE_CA_CERT to be configured in order to
verify TLS for the SRQL test fixture, and it exited with code 1 after printing:
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.

Relevant error logs: 
1:  Runner name: 'arc-runner-set-hk6mk-runner-hssgd'
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}
...

356:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
357:  env:
358:  BUILDBUDDY_ORG_API_KEY: ***
359:  SRQL_TEST_DATABASE_URL: ***
360:  SRQL_TEST_ADMIN_URL: ***
361:  SRQL_TEST_DATABASE_CA_CERT: 
362:  DOCKERHUB_USERNAME: ***
363:  DOCKERHUB_TOKEN: ***
364:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
365:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
366:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
367:  ##[endgroup]
368:  ##[group]Run : install rustup if needed
369:  ^[[36;1m: install rustup if needed^[[0m
370:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
371:  ^[[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
372:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

512:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
513:  env:
514:  BUILDBUDDY_ORG_API_KEY: ***
515:  SRQL_TEST_DATABASE_URL: ***
516:  SRQL_TEST_ADMIN_URL: ***
517:  SRQL_TEST_DATABASE_CA_CERT: 
518:  DOCKERHUB_USERNAME: ***
519:  DOCKERHUB_TOKEN: ***
520:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
521:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
522:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
523:  CARGO_HOME: /home/runner/.cargo
524:  CARGO_INCREMENTAL: 0
525:  CARGO_TERM_COLOR: always
526:  ##[endgroup]
527:  ##[group]Run : work around spurious network errors in curl 8.0
528:  ^[[36;1m: work around spurious network errors in curl 8.0^[[0m
529:  ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m
...

580:  SRQL_TEST_DATABASE_CA_CERT: 
581:  DOCKERHUB_USERNAME: ***
582:  DOCKERHUB_TOKEN: ***
583:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
584:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
585:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
586:  CARGO_HOME: /home/runner/.cargo
587:  CARGO_INCREMENTAL: 0
588:  CARGO_TERM_COLOR: always
589:  ##[endgroup]
590:  Attempting to download 1.x...
591:  Acquiring v1.28.1 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.1/bazelisk-linux-amd64
592:  Adding to the cache ...
593:  Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.1/x64
594:  Added bazelisk to the path
595:  ##[warning]Failed to restore: Cache service responded with 400
596:  Restored bazelisk cache dir @ /home/runner/.cache/bazelisk
...

662:  env:
663:  BUILDBUDDY_ORG_API_KEY: ***
664:  SRQL_TEST_DATABASE_URL: ***
665:  SRQL_TEST_ADMIN_URL: ***
666:  SRQL_TEST_DATABASE_CA_CERT: 
667:  DOCKERHUB_USERNAME: ***
668:  DOCKERHUB_TOKEN: ***
669:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
670:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
671:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
672:  CARGO_HOME: /home/runner/.cargo
673:  CARGO_INCREMENTAL: 0
674:  CARGO_TERM_COLOR: always
675:  ##[endgroup]
676:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
677:  ##[error]Process completed with exit code 1.
678:  Post job cleanup.
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2668#issuecomment-3832919166 Original created: 2026-02-02T05:00:19Z --- ## 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/21577970170/job/62169260511) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed during environment/secret validation because the <code>SRQL_TEST_DATABASE_CA_CERT</code> value <br>was empty/unset.<br> The workflow requires <code>SRQL_TEST_DATABASE_CA_CERT</code> to be configured in order to <br>verify TLS for the SRQL test fixture, and it exited with code 1 after printing: <br><code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.</code><br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-hssgd' 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} ... 356: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 357: env: 358: BUILDBUDDY_ORG_API_KEY: *** 359: SRQL_TEST_DATABASE_URL: *** 360: SRQL_TEST_ADMIN_URL: *** 361: SRQL_TEST_DATABASE_CA_CERT: 362: DOCKERHUB_USERNAME: *** 363: DOCKERHUB_TOKEN: *** 364: TEST_CNPG_DATABASE: serviceradar_web_ng_test 365: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 366: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 367: ##[endgroup] 368: ##[group]Run : install rustup if needed 369: ^[[36;1m: install rustup if needed^[[0m 370: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 371: ^[[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 372: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 512: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 513: env: 514: BUILDBUDDY_ORG_API_KEY: *** 515: SRQL_TEST_DATABASE_URL: *** 516: SRQL_TEST_ADMIN_URL: *** 517: SRQL_TEST_DATABASE_CA_CERT: 518: DOCKERHUB_USERNAME: *** 519: DOCKERHUB_TOKEN: *** 520: TEST_CNPG_DATABASE: serviceradar_web_ng_test 521: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 522: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 523: CARGO_HOME: /home/runner/.cargo 524: CARGO_INCREMENTAL: 0 525: CARGO_TERM_COLOR: always 526: ##[endgroup] 527: ##[group]Run : work around spurious network errors in curl 8.0 528: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 529: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 580: SRQL_TEST_DATABASE_CA_CERT: 581: DOCKERHUB_USERNAME: *** 582: DOCKERHUB_TOKEN: *** 583: TEST_CNPG_DATABASE: serviceradar_web_ng_test 584: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 585: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 586: CARGO_HOME: /home/runner/.cargo 587: CARGO_INCREMENTAL: 0 588: CARGO_TERM_COLOR: always 589: ##[endgroup] 590: Attempting to download 1.x... 591: Acquiring v1.28.1 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.1/bazelisk-linux-amd64 592: Adding to the cache ... 593: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.1/x64 594: Added bazelisk to the path 595: ##[warning]Failed to restore: Cache service responded with 400 596: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 662: env: 663: BUILDBUDDY_ORG_API_KEY: *** 664: SRQL_TEST_DATABASE_URL: *** 665: SRQL_TEST_ADMIN_URL: *** 666: SRQL_TEST_DATABASE_CA_CERT: 667: DOCKERHUB_USERNAME: *** 668: DOCKERHUB_TOKEN: *** 669: TEST_CNPG_DATABASE: serviceradar_web_ng_test 670: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 671: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 672: CARGO_HOME: /home/runner/.cargo 673: CARGO_INCREMENTAL: 0 674: CARGO_TERM_COLOR: always 675: ##[endgroup] 676: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 677: ##[error]Process completed with exit code 1. 678: Post job cleanup. ``` </details></td></tr></table>
qodo-code-review[bot] commented 2026-02-02 05:01:46 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2668#issuecomment-3832922323
Original created: 2026-02-02T05:01:46Z

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consolidate observability tabs into separate components

Refactor the LogLive.Index LiveView by extracting the logic for each
observability tab (Events, Alerts, NetFlows) into its own dedicated
LiveComponent to improve modularity and maintainability.

Examples:

web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [1-2831] 
defmodule ServiceRadarWebNGWeb.LogLive.Index do
  use ServiceRadarWebNGWeb, :live_view

  import Ecto.Query
  import ServiceRadarWebNGWeb.UIComponents

  alias Phoenix.LiveView.JS
  alias ServiceRadarWebNG.Repo
  alias ServiceRadarWebNGWeb.SRQL.Page, as: SRQLPage
  alias ServiceRadarWebNGWeb.Stats

 ... (clipped 2821 lines)

Solution Walkthrough:

Before:

# In web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex
defmodule ServiceRadarWebNGWeb.LogLive.Index do
  # ...
  def mount(..., socket) do
    # Assigns state for logs, traces, metrics, events, alerts, netflows
    socket
    |> assign(:logs, [])
    |> assign(:events, [])
    |> assign(:alerts, [])
    |> assign(:netflows, [])
    |> assign(:event_summary, ...)
    # ... and so on for all tabs
  end

  def render(assigns) do
    ~H"""
    ...
    <.event_summary :if={@active_tab == "events"} ... />
    <.alert_summary :if={@active_tab == "alerts"} ... />
    <.netflow_summary :if={@active_tab == "netflows"} ... />
    ...
    <.events_table :if={@active_tab == "events"} ... />
    <.alerts_table :if={@active_tab == "alerts"} ... />
    <.netflows_table :if={@active_tab == "netflows"} ... />
    ...
    """
  end

  # All helper, rendering, and business logic functions for all tabs
  # are defined within this single, large module.
  defp compute_event_summary(...), do: ...
  defp events_table(...), do: ...
  defp compute_alert_summary(...), do: ...
  defp alerts_table(...), do: ...
end

After:

# In web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex
defmodule ServiceRadarWebNGWeb.LogLive.Index do
  use ServiceRadarWebNGWeb, :live_view

  def render(assigns) do
    ~H"""
    <.observability_tabs active={@active_tab} />

    <div :if={@active_tab == "events"}>
      <.live_component module={EventsTabComponent} id="events-tab" srql={@srql} />
    </div>
    <div :if={@active_tab == "alerts"}>
      <.live_component module={AlertsTabComponent} id="alerts-tab" srql={@srql} />
    </div>
    <div :if={@active_tab == "netflows"}>
      <.live_component module={NetflowTabComponent} id="netflow-tab" srql={@srql} />
    </div>
    ...
    """
  end
end

# In web-ng/lib/serviceradar_web_ng_web/live/log_live/events_tab_component.ex (New File)
defmodule ServiceRadarWebNGWeb.LogLive.EventsTabComponent do
  use ServiceRadarWebNGWeb, :live_component

  def mount(socket) do
    # Mount and assign state specific to the Events tab
    {:ok, assign(socket, :events, [], :summary, ...)}
  end

  def render(assigns) do
    # Rendering logic for the Events tab is self-contained
    ~H"""
    <.event_summary summary={@summary} />
    <.events_table events={@events} />
    """
  end
end
Suggestion importance[1-10]: 9

__

Why: This is a critical architectural suggestion that addresses the significant complexity introduced in LogLive.Index, which has become a "god module" and will be hard to maintain.

High
Possible issue
Prevent nil values from causing errors

In compute_netflow_summary, provide a default value of 0 when calling to_int on
the results of netflow_protocol_num and netflow_bytes to prevent crashes from
nil values.

web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [2605-2634]

 defp compute_netflow_summary(flows) when is_list(flows) do
   Enum.reduce(
     flows,
     %{total: 0, tcp: 0, udp: 0, other: 0, total_bytes: 0, v5: 0, v9: 0, ipfix: 0},
     fn flow, acc ->
-      protocol = flow |> netflow_protocol_num() |> to_int()
-      bytes = flow |> netflow_bytes() |> to_int()
+      protocol = flow |> netflow_protocol_num() |> to_int(0)
+      bytes = flow |> netflow_bytes() |> to_int(0)
       flow_type = netflow_flow_type(flow)
 
       updated =
         case protocol do
           6 -> Map.update!(acc, :tcp, &(&1 + 1))
           17 -> Map.update!(acc, :udp, &(&1 + 1))
           _ -> Map.update!(acc, :other, &(&1 + 1))
         end
 
       updated =
         case flow_type do
           "NETFLOW_V5" -> Map.update!(updated, :v5, &(&1 + 1))
           "NETFLOW_V9" -> Map.update!(updated, :v9, &(&1 + 1))
           "IPFIX" -> Map.update!(updated, :ipfix, &(&1 + 1))
           _ -> updated
         end
 
       updated
       |> Map.update!(:total, &(&1 + 1))
       |> Map.update!(:total_bytes, &(&1 + bytes))
     end
   )
 end
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that netflow_protocol_num/1 and netflow_bytes/1 can return nil, which would cause a crash when passed to to_int/1. Providing a default value makes the summary computation more robust and prevents potential runtime errors, which is a significant improvement.

Medium
Apply max_limit in SRQL init

Pass max_limit to ensure_srql_entity and then to SRQLPage.init to ensure
consistent limit handling when initializing the SRQL page state for a new
entity.

web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [68-2831]

-# in handle_params/3
+# in handle_params/3, pass max_limit as well
 socket =
   socket
   |> assign(:active_tab, tab)
   |> assign(:logs, [])
   |> assign(:traces, [])
   |> assign(:metrics, [])
   |> assign(:events, [])
   |> assign(:alerts, [])
   |> assign(:netflows, [])
-  |> ensure_srql_entity(entity, default_limit)
+  |> ensure_srql_entity(entity, default_limit, max_limit)
   |> SRQLPage.load_list(params, uri, list_key,
     default_limit: default_limit,
     max_limit: max_limit
   )
 
-# later in the same module
-defp ensure_srql_entity(socket, entity, default_limit \\ @default_limit)
+# updated ensure_srql_entity/4
+defp ensure_srql_entity(socket, entity, default_limit, max_limit)
      when is_binary(entity) do
   current = socket.assigns |> Map.get(:srql, %{}) |> Map.get(:entity)
 
   if current == entity do
     socket
   else
-    SRQLPage.init(socket, entity, default_limit: default_limit)
+    SRQLPage.init(socket, entity,
+      default_limit: default_limit,
+      max_limit: max_limit
+    )
   end
 end

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that max_limit is not being passed to SRQLPage.init, leading to inconsistent limit handling for new entities. Applying this change ensures that both default_limit and max_limit are correctly initialized, improving the correctness of the pagination logic.

Medium
General
Simplify complex nested case statements

Refactor the netflow_protocol_badge function to use a cond statement instead of
nested case statements for better readability, and handle potential nil values
from to_int by providing a default.

web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [1641-1675]

 defp netflow_protocol_badge(assigns) do
   protocol = assigns.protocol
   name = assigns.name
-  protocol_num = to_int(protocol)
+  protocol_num = to_int(protocol, nil)
 
   {label, variant} =
     case protocol_num do
       6 ->
         {"TCP", "success"}
 
       17 ->
         {"UDP", "info"}
 
       1 ->
         {"ICMP", "ghost"}
 
       _ ->
-        case name do
-          value when is_binary(value) and value != "" ->
-            {String.upcase(value), "ghost"}
+        cond do
+          is_binary(name) and name != "" ->
+            {String.upcase(name), "ghost"}
 
-          _ ->
-            case protocol do
-              value when is_binary(value) and value != "" -> {value, "ghost"}
-              _ -> {"Unknown", "ghost"}
-            end
+          is_binary(protocol) and protocol != "" ->
+            {protocol, "ghost"}
+
+          true ->
+            {"Unknown", "ghost"}
         end
     end
 
   assigns = assign(assigns, label: label, variant: variant)
 
   ~H"""
   <.ui_badge variant={@variant}>{@label}</.ui_badge>
   """
 end
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that nested case statements can be simplified using cond, which improves code readability. It also correctly identifies a potential nil value issue with to_int(protocol), and the proposed fix makes the function more robust.

Low
Simplify sidebar active check

Refactor the active check for the "Observability" sidebar link to use Enum.any?
with a list of paths, simplifying the logic and improving maintainability.

web-ng/lib/serviceradar_web_ng_web/components/layouts.ex [165-177]

 <.sidebar_link
   href={~p"/observability"}
   label="Observability"
   icon="hero-presentation-chart-line"
-  active={
-    @current_path &&
-      (String.starts_with?(@current_path, "/observability") ||
-         String.starts_with?(@current_path, "/logs") ||
-         String.starts_with?(@current_path, "/events") ||
-         String.starts_with?(@current_path, "/alerts") ||
-         String.starts_with?(@current_path, "/netflows"))
-  }
+  active={@current_path && Enum.any?(~w(
+    /observability /logs /events /alerts /netflows
+  ), &String.starts_with?(@current_path, &1))}
 />
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion improves code readability and maintainability by replacing multiple || conditions with a more scalable Enum.any? check. This makes the code cleaner and easier to modify in the future.

Low
Simplify nested data access logic

Refactor the netflow_flow_type function to use a more idiomatic pipeline with
get_in and case for cleaner and more concise nested data access.

web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [1714-1732]

 defp netflow_flow_type(flow) do
-  payload =
-    case Map.get(flow, "ocsf_payload") do
-      %{} = data -> data
-      _ -> nil
-    end
+  flow
+  |> get_in(["ocsf_payload"])
+  |> case do
+    %{} = payload ->
+      get_in(payload, ["unmapped", "flow_type"]) || get_in(payload, ["flow_type"])
 
-  flow_type =
-    if is_map(payload) do
-      get_in(payload, ["unmapped", "flow_type"]) || get_in(payload, ["flow_type"])
-    else
+    _ ->
       nil
-    end
-
-  case flow_type do
+  end
+  |> case do
     value when is_binary(value) and value != "" -> value
     _ -> nil
   end
 end
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion offers a more idiomatic and slightly cleaner way to access nested data using get_in and case. While the original code is correct, the proposed change improves readability and conciseness.

Low
  • More
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2668#issuecomment-3832922323 Original created: 2026-02-02T05:01:46Z --- ## PR Code Suggestions ✨ <!-- 337ea53 --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Consolidate observability tabs into separate components</summary> ___ **Refactor the <code>LogLive.Index</code> LiveView by extracting the logic for each <br>observability tab (Events, Alerts, NetFlows) into its own dedicated <br>LiveComponent to improve modularity and maintainability.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321R1-R2831">web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [1-2831]</a> </summary> ```elixir defmodule ServiceRadarWebNGWeb.LogLive.Index do use ServiceRadarWebNGWeb, :live_view import Ecto.Query import ServiceRadarWebNGWeb.UIComponents alias Phoenix.LiveView.JS alias ServiceRadarWebNG.Repo alias ServiceRadarWebNGWeb.SRQL.Page, as: SRQLPage alias ServiceRadarWebNGWeb.Stats ... (clipped 2821 lines) ``` </details> ### Solution Walkthrough: #### Before: ```elixir # In web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex defmodule ServiceRadarWebNGWeb.LogLive.Index do # ... def mount(..., socket) do # Assigns state for logs, traces, metrics, events, alerts, netflows socket |> assign(:logs, []) |> assign(:events, []) |> assign(:alerts, []) |> assign(:netflows, []) |> assign(:event_summary, ...) # ... and so on for all tabs end def render(assigns) do ~H""" ... <.event_summary :if={@active_tab == "events"} ... /> <.alert_summary :if={@active_tab == "alerts"} ... /> <.netflow_summary :if={@active_tab == "netflows"} ... /> ... <.events_table :if={@active_tab == "events"} ... /> <.alerts_table :if={@active_tab == "alerts"} ... /> <.netflows_table :if={@active_tab == "netflows"} ... /> ... """ end # All helper, rendering, and business logic functions for all tabs # are defined within this single, large module. defp compute_event_summary(...), do: ... defp events_table(...), do: ... defp compute_alert_summary(...), do: ... defp alerts_table(...), do: ... end ``` #### After: ```elixir # In web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex defmodule ServiceRadarWebNGWeb.LogLive.Index do use ServiceRadarWebNGWeb, :live_view def render(assigns) do ~H""" <.observability_tabs active={@active_tab} /> <div :if={@active_tab == "events"}> <.live_component module={EventsTabComponent} id="events-tab" srql={@srql} /> </div> <div :if={@active_tab == "alerts"}> <.live_component module={AlertsTabComponent} id="alerts-tab" srql={@srql} /> </div> <div :if={@active_tab == "netflows"}> <.live_component module={NetflowTabComponent} id="netflow-tab" srql={@srql} /> </div> ... """ end end # In web-ng/lib/serviceradar_web_ng_web/live/log_live/events_tab_component.ex (New File) defmodule ServiceRadarWebNGWeb.LogLive.EventsTabComponent do use ServiceRadarWebNGWeb, :live_component def mount(socket) do # Mount and assign state specific to the Events tab {:ok, assign(socket, :events, [], :summary, ...)} end def render(assigns) do # Rendering logic for the Events tab is self-contained ~H""" <.event_summary summary={@summary} /> <.events_table events={@events} /> """ end end ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This is a critical architectural suggestion that addresses the significant complexity introduced in `LogLive.Index`, which has become a "god module" and will be hard to maintain. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>Possible issue</td> <td> <details><summary>Prevent nil values from causing errors</summary> ___ **In <code>compute_netflow_summary</code>, provide a default value of <code>0</code> when calling <code>to_int</code> on <br>the results of <code>netflow_protocol_num</code> and <code>netflow_bytes</code> to prevent crashes from <br><code>nil</code> values.** [web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [2605-2634]](https://github.com/carverauto/serviceradar/pull/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321R2605-R2634) ```diff defp compute_netflow_summary(flows) when is_list(flows) do Enum.reduce( flows, %{total: 0, tcp: 0, udp: 0, other: 0, total_bytes: 0, v5: 0, v9: 0, ipfix: 0}, fn flow, acc -> - protocol = flow |> netflow_protocol_num() |> to_int() - bytes = flow |> netflow_bytes() |> to_int() + protocol = flow |> netflow_protocol_num() |> to_int(0) + bytes = flow |> netflow_bytes() |> to_int(0) flow_type = netflow_flow_type(flow) updated = case protocol do 6 -> Map.update!(acc, :tcp, &(&1 + 1)) 17 -> Map.update!(acc, :udp, &(&1 + 1)) _ -> Map.update!(acc, :other, &(&1 + 1)) end updated = case flow_type do "NETFLOW_V5" -> Map.update!(updated, :v5, &(&1 + 1)) "NETFLOW_V9" -> Map.update!(updated, :v9, &(&1 + 1)) "IPFIX" -> Map.update!(updated, :ipfix, &(&1 + 1)) _ -> updated end updated |> Map.update!(:total, &(&1 + 1)) |> Map.update!(:total_bytes, &(&1 + bytes)) end ) end ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies that `netflow_protocol_num/1` and `netflow_bytes/1` can return `nil`, which would cause a crash when passed to `to_int/1`. Providing a default value makes the summary computation more robust and prevents potential runtime errors, which is a significant improvement. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Apply max_limit in SRQL init</summary> ___ **Pass <code>max_limit</code> to <code>ensure_srql_entity</code> and then to <code>SRQLPage.init</code> to ensure <br>consistent limit handling when initializing the SRQL page state for a new <br>entity.** [web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [68-2831]](https://github.com/carverauto/serviceradar/pull/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321R68-R2831) ```diff -# in handle_params/3 +# in handle_params/3, pass max_limit as well socket = socket |> assign(:active_tab, tab) |> assign(:logs, []) |> assign(:traces, []) |> assign(:metrics, []) |> assign(:events, []) |> assign(:alerts, []) |> assign(:netflows, []) - |> ensure_srql_entity(entity, default_limit) + |> ensure_srql_entity(entity, default_limit, max_limit) |> SRQLPage.load_list(params, uri, list_key, default_limit: default_limit, max_limit: max_limit ) -# later in the same module -defp ensure_srql_entity(socket, entity, default_limit \\ @default_limit) +# updated ensure_srql_entity/4 +defp ensure_srql_entity(socket, entity, default_limit, max_limit) when is_binary(entity) do current = socket.assigns |> Map.get(:srql, %{}) |> Map.get(:entity) if current == entity do socket else - SRQLPage.init(socket, entity, default_limit: default_limit) + SRQLPage.init(socket, entity, + default_limit: default_limit, + max_limit: max_limit + ) end end ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that `max_limit` is not being passed to `SRQLPage.init`, leading to inconsistent limit handling for new entities. Applying this change ensures that both `default_limit` and `max_limit` are correctly initialized, improving the correctness of the pagination logic. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=3>General</td> <td> <details><summary>Simplify complex nested case statements</summary> ___ **Refactor the <code>netflow_protocol_badge</code> function to use a <code>cond</code> statement instead of <br>nested <code>case</code> statements for better readability, and handle potential <code>nil</code> values <br>from <code>to_int</code> by providing a default.** [web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [1641-1675]](https://github.com/carverauto/serviceradar/pull/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321R1641-R1675) ```diff defp netflow_protocol_badge(assigns) do protocol = assigns.protocol name = assigns.name - protocol_num = to_int(protocol) + protocol_num = to_int(protocol, nil) {label, variant} = case protocol_num do 6 -> {"TCP", "success"} 17 -> {"UDP", "info"} 1 -> {"ICMP", "ghost"} _ -> - case name do - value when is_binary(value) and value != "" -> - {String.upcase(value), "ghost"} + cond do + is_binary(name) and name != "" -> + {String.upcase(name), "ghost"} - _ -> - case protocol do - value when is_binary(value) and value != "" -> {value, "ghost"} - _ -> {"Unknown", "ghost"} - end + is_binary(protocol) and protocol != "" -> + {protocol, "ghost"} + + true -> + {"Unknown", "ghost"} end end assigns = assign(assigns, label: label, variant: variant) ~H""" <.ui_badge variant={@variant}>{@label}</.ui_badge> """ end ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=3 --> <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion correctly points out that nested `case` statements can be simplified using `cond`, which improves code readability. It also correctly identifies a potential `nil` value issue with `to_int(protocol)`, and the proposed fix makes the function more robust. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Simplify sidebar active check</summary> ___ **Refactor the <code>active</code> check for the "Observability" sidebar link to use <code>Enum.any?</code> <br>with a list of paths, simplifying the logic and improving maintainability.** [web-ng/lib/serviceradar_web_ng_web/components/layouts.ex [165-177]](https://github.com/carverauto/serviceradar/pull/2668/files#diff-75eaced063e3e7325588594e742492bab789a979319a34f2f623f6d161cfffd2R165-R177) ```diff <.sidebar_link href={~p"/observability"} label="Observability" icon="hero-presentation-chart-line" - active={ - @current_path && - (String.starts_with?(@current_path, "/observability") || - String.starts_with?(@current_path, "/logs") || - String.starts_with?(@current_path, "/events") || - String.starts_with?(@current_path, "/alerts") || - String.starts_with?(@current_path, "/netflows")) - } + active={@current_path && Enum.any?(~w( + /observability /logs /events /alerts /netflows + ), &String.starts_with?(@current_path, &1))} /> ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=4 --> <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: The suggestion improves code readability and maintainability by replacing multiple `||` conditions with a more scalable `Enum.any?` check. This makes the code cleaner and easier to modify in the future. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Simplify nested data access logic</summary> ___ **Refactor the <code>netflow_flow_type</code> function to use a more idiomatic pipeline with <br><code>get_in</code> and <code>case</code> for cleaner and more concise nested data access.** [web-ng/lib/serviceradar_web_ng_web/live/log_live/index.ex [1714-1732]](https://github.com/carverauto/serviceradar/pull/2668/files#diff-5c469994d989d04955c9b8a31536ed518a182b3f46819ecee9c3c22f651a4321R1714-R1732) ```diff defp netflow_flow_type(flow) do - payload = - case Map.get(flow, "ocsf_payload") do - %{} = data -> data - _ -> nil - end + flow + |> get_in(["ocsf_payload"]) + |> case do + %{} = payload -> + get_in(payload, ["unmapped", "flow_type"]) || get_in(payload, ["flow_type"]) - flow_type = - if is_map(payload) do - get_in(payload, ["unmapped", "flow_type"]) || get_in(payload, ["flow_type"]) - else + _ -> nil - end - - case flow_type do + end + |> case do value when is_binary(value) and value != "" -> value _ -> nil end end ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=5 --> <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: The suggestion offers a more idiomatic and slightly cleaner way to access nested data using `get_in` and `case`. While the original code is correct, the proposed change improves readability and conciseness. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More <!-- /improve --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2832
No description provided.