carverauto/serviceradar

Fork 0

fix: Netflow tweaks to collector conversions and bump 0.8.4 version #2826

Closed

mikemiles-dev wants to merge 2 commits from refs/pull/2826/head into staging

mikemiles-dev commented

2026-02-01 18:34:33 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2658
Original author: @mikemiles-dev
Original URL: https://github.com/carverauto/serviceradar/pull/2658
Original created: 2026-02-01T18:34:33Z
Original updated: 2026-02-03T23:06:59Z
Original head: mikemiles-dev/serviceradar:fix/netflow_tweaks
Original base: staging

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, Bug fix

Description

Add protocol name field to NetFlow data collection and display
- Capture protocol_name from NetFlow packets in Rust collector
- Display protocol names in web UI with intelligent fallback logic
Improve field value conversion robustness in NetFlow parser
- Handle DataNumber and ProtocolType variants explicitly
- Add comprehensive conversion functions with proper type handling
Refactor cache statistics structure for simplified reporting
- Update CacheStatsVec type to use unified ParserCacheStats
- Simplify metrics logging to use consolidated stats object
Update dependencies and add comprehensive test coverage
- Bump netflow_parser from 0.8.1 to 0.8.4
- Add 30+ tests for data conversion and protocol name handling

Diagram Walkthrough

flowchart LR
  A["NetFlow Packet"] -->|"Extract protocol_name"| B["Rust Converter"]
  B -->|"Store in FlowMessage"| C["Protocol Field"]
  C -->|"Query from DB"| D["Web UI"]
  D -->|"Display with variant"| E["Protocol Badge"]
  F["Field Value"] -->|"Convert DataNumber/ProtocolType"| G["Robust Conversion"]
  G -->|"Handle edge cases"| H["u32/u64 Output"]

File Walkthrough

Relevant files

Enhancement

7 files

index.ex `Add protocol name display with intelligent fallback`	+29/-7
converter.rs `Implement protocol name extraction and robust field conversions`	+260/-5
listener.rs `Refactor cache statistics to unified structure`	+3/-19
metrics.rs `Update metrics logging for consolidated cache stats`	+20/-20
netflow_to_ocsf.json `Use protocol_name field with fallback to numeric mapping`	+1/-1
netflow_to_ocsf.json `Use protocol_name field with fallback to numeric mapping`	+1/-1
flow.proto `Add protocol_name string field to FlowMessage`	+3/-0

Bug fix

1 files

publisher.rs
Add missing nats_creds_file field to test config +1/-0

Dependencies

1 files

Cargo.toml
Bump netflow_parser dependency to 0.8.4 +1/-1

Imported from GitHub pull request. Original GitHub pull request: #2658 Original author: @mikemiles-dev Original URL: https://github.com/carverauto/serviceradar/pull/2658 Original created: 2026-02-01T18:34:33Z Original updated: 2026-02-03T23:06:59Z Original head: mikemiles-dev/serviceradar:fix/netflow_tweaks Original base: staging --- ### **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, Bug fix ___ ### **Description** - Add protocol name field to NetFlow data collection and display - Capture `protocol_name` from NetFlow packets in Rust collector - Display protocol names in web UI with intelligent fallback logic - Improve field value conversion robustness in NetFlow parser - Handle `DataNumber` and `ProtocolType` variants explicitly - Add comprehensive conversion functions with proper type handling - Refactor cache statistics structure for simplified reporting - Update `CacheStatsVec` type to use unified `ParserCacheStats` - Simplify metrics logging to use consolidated stats object - Update dependencies and add comprehensive test coverage - Bump `netflow_parser` from 0.8.1 to 0.8.4 - Add 30+ tests for data conversion and protocol name handling ___ ### Diagram Walkthrough ```mermaid flowchart LR A["NetFlow Packet"] -->|"Extract protocol_name"| B["Rust Converter"] B -->|"Store in FlowMessage"| C["Protocol Field"] C -->|"Query from DB"| D["Web UI"] D -->|"Display with variant"| E["Protocol Badge"] F["Field Value"] -->|"Convert DataNumber/ProtocolType"| G["Robust Conversion"] G -->|"Handle edge cases"| H["u32/u64 Output"] ``` <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><details><summary>7 files</summary><table> <tr> <td><strong>index.ex</strong><dd><code>Add protocol name display with intelligent fallback</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-7ec566a3a20e36d101f72bb9ee19620be76b397c1d88a1edc6d9a88e8da6909e">+29/-7</a>    </td> </tr> <tr> <td><strong>converter.rs</strong><dd><code>Implement protocol name extraction and robust field conversions</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-873d955c2362720f0772872c1dfaba79c512b09dc5c794009ea466ba73cbaae4">+260/-5</a>  </td> </tr> <tr> <td><strong>listener.rs</strong><dd><code>Refactor cache statistics to unified structure</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-3ec4d0175a1f407a9c9246268da3491d1fdfacd2e8c00d536fde36357e993999">+3/-19</a>    </td> </tr> <tr> <td><strong>metrics.rs</strong><dd><code>Update metrics logging for consolidated cache stats</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-d3fef810bfa2a5b985b71300423ec1d9c6eea321fee261710cd3257b521ad7e2">+20/-20</a>  </td> </tr> <tr> <td><strong>netflow_to_ocsf.json</strong><dd><code>Use protocol_name field with fallback to numeric mapping</code>  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-f1817cc1d98f34d2d27d99bcc8471124179fd5ed61860e45b87d397ff3e08d9a">+1/-1</a>      </td> </tr> <tr> <td><strong>netflow_to_ocsf.json</strong><dd><code>Use protocol_name field with fallback to numeric mapping</code>  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-1de8bfcc6c483d7eebbf49223ced7377d1647d5ac71069b4ef49c705caa164e3">+1/-1</a>      </td> </tr> <tr> <td><strong>flow.proto</strong><dd><code>Add protocol_name string field to FlowMessage</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-81ddfbe717075dc4000bbe1aca2ffdbf0b81b11b5f6103c235af6aba0ec9600e">+3/-0</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Bug fix</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>publisher.rs</strong><dd><code>Add missing nats_creds_file field to test config</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-10dec5866b4bc416c62590bdf2d3ded6c234b8d423ec4aebc70eac891ff0c07d">+1/-0</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Dependencies</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>Cargo.toml</strong><dd><code>Bump netflow_parser dependency to 0.8.4</code>                                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-03c9bd143b73f35c01bd56390e03d9bde76065f53473ed6a70aa49c6dd1d67b3">+1/-1</a>      </td> </tr> </table></details></td></tr></tbody></table> </details> ___

CLAassistant commented

2026-02-01 18:34:39 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @CLAassistant
Original URL: https://github.com/carverauto/serviceradar/pull/2658#issuecomment-3831727232
Original created: 2026-02-01T18:34:39Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

mikemiles-dev seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Imported GitHub PR comment. Original author: @CLAassistant Original URL: https://github.com/carverauto/serviceradar/pull/2658#issuecomment-3831727232 Original created: 2026-02-01T18:34:39Z --- [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/carverauto/serviceradar?pullRequest=2658) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/carverauto/serviceradar?pullRequest=2658) before we can accept your contribution.<br/><hr/>**mikemiles-dev** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/carverauto/serviceradar?pullRequest=2658) it.</sub>

qodo-code-review[bot] commented

2026-02-01 18:35:10 +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/2658#issuecomment-3831728508
Original created: 2026-02-01T18:35:10Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Log injection Description: Untrusted NetFlow source identifiers are interpolated directly into log lines via `source` (derived from parser keys) without sanitization, which could enable log forging/injection (e.g., embedded newlines/control characters) if an attacker can influence the parsed source key representation. metrics.rs [34-63] Referred Code for (source, stats) in &v9_stats { info!( "V9 Template Cache [{}] - V9: {}/{}, IPFIX: {}/{}, V9 Hits/Misses: {}/{}, IPFIX Hits/Misses: {}/{}", source, stats.v9.current_size, stats.v9.max_size, stats.ipfix.current_size, stats.ipfix.max_size, stats.v9.metrics.hits, stats.v9.metrics.misses, stats.ipfix.metrics.hits, stats.ipfix.metrics.misses ); } // Log IPFIX cache stats per source for (source, stats) in &ipfix_stats { info!( "IPFIX Template Cache [{}] - V9: {}/{}, IPFIX: {}/{}, V9 Hits/Misses: {}/{}, IPFIX Hits/Misses: {}/{}", source, stats.v9.current_size, ... (clipped 9 lines)
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: 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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation 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
Update

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/2658#issuecomment-3831728508 Original created: 2026-02-01T18:35:10Z --- ## 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 rowspan=1>⚪</td> <td><details><summary><strong>Log injection </strong></summary><br> <b>Description:</b> Untrusted NetFlow source identifiers are interpolated directly into log lines via <code>source</code> <br>(derived from parser keys) without sanitization, which could enable log forging/injection <br>(e.g., embedded newlines/control characters) if an attacker can influence the parsed <br>source key representation.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/2658/files#diff-d3fef810bfa2a5b985b71300423ec1d9c6eea321fee261710cd3257b521ad7e2R34-R63'>metrics.rs [34-63]</a></strong><br> <details open><summary>Referred Code</summary> ```rust for (source, stats) in &v9_stats { info!( "V9 Template Cache [{}] - V9: {}/{}, IPFIX: {}/{}, V9 Hits/Misses: {}/{}, IPFIX Hits/Misses: {}/{}", source, stats.v9.current_size, stats.v9.max_size, stats.ipfix.current_size, stats.ipfix.max_size, stats.v9.metrics.hits, stats.v9.metrics.misses, stats.ipfix.metrics.hits, stats.ipfix.metrics.misses ); } // Log IPFIX cache stats per source for (source, stats) in &ipfix_stats { info!( "IPFIX Template Cache [{}] - V9: {}/{}, IPFIX: {}/{}, V9 Hits/Misses: {}/{}, IPFIX Hits/Misses: {}/{}", source, stats.v9.current_size, ... (clipped 9 lines) ``` </details></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=6>🟢</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: 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:** 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 align="center" colspan="2"> - [ ] Update  </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-01 18:36:18 +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/2658#issuecomment-3831730820
Original created: 2026-02-01T18:36:18Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate protocol mapping logic To improve maintainability, centralize the protocol number-to-name mapping within the Rust collector. This involves removing the redundant fallback logic from the OCSF mapping file and the Elixir web UI. Examples: packaging/zen/rules/netflow_to_ocsf.json [69] `"value": "{ protocol_num: proto, protocol_name: protocol_name != '' ? protocol_name : (proto == 1 ? 'ICMP' : proto == 6 ? 'TCP' : proto == 17 ? 'UDP' : proto == 47 ? 'GRE' : proto == 50 ? 'ESP' : proto == 58 ? 'ICMPv6' : 'Unknown'), tcp_flags: tcp_flags, direction_id: 0, boundary_id: 0 }"` web-ng/lib/serviceradar_web_ng_web/live/netflow_live/index.ex [264-275] `{label, variant} = if is_binary(protocol_name) and protocol_name != "" do {String.upcase(protocol_name), protocol_variant(protocol_name)} else case protocol do 6 -> {"TCP", "success"} 17 -> {"UDP", "info"} 1 -> {"ICMP", "ghost"} nil -> {"Unknown", "ghost"} other -> {to_string(other), "ghost"} ... (clipped 2 lines)` Solution Walkthrough: Before: // packaging/zen/rules/netflow_to_ocsf.json "value": "{ ..., protocol_name: protocol_name != '' ? protocol_name : (proto == 1 ? 'ICMP' : proto == 6 ? 'TCP' : ...), ... }" // web-ng/.../index.ex defp protocol_badge(assigns) do protocol_name = Map.get(assigns, :protocol_name) protocol = assigns.protocol {label, variant} = if is_binary(protocol_name) and protocol_name != "" do {String.upcase(protocol_name), protocol_variant(protocol_name)} else case protocol do // Fallback logic 6 -> {"TCP", "success"} 17 -> {"UDP", "info"} ... end end ... end After: // packaging/zen/rules/netflow_to_ocsf.json "value": "{ ..., protocol_name: protocol_name, ... }" // web-ng/.../index.ex defp protocol_badge(assigns) do protocol_name = Map.get(assigns, :protocol_name, "UNKNOWN") label = String.upcase(protocol_name) variant = protocol_variant(protocol_name) ... end // rust/netflow-collector/src/converter.rs // Ensure protocol_name is always populated, e.g., with "UNKNOWN" for unmapped numbers. fn protocol_type_name(pt: ProtocolTypes) -> String { let name = format!("{pt:?}"); if name == "RESERVED" { "UNKNOWN".to_string() } else { name.to_uppercase() } } Suggestion importance[1-10]: 9 __ Why: This is a significant architectural suggestion that correctly identifies duplicated logic introduced in the PR across multiple components, proposing a cleaner, more maintainable single-source-of-truth design.	High
Possible issue	annotate async test properly Add the `#[tokio::test]` attribute to the `test_publisher_creation` function to ensure it is executed by the test runner. rust/netflow-collector/src/publisher.rs [219-243] +#[tokio::test] async fn test_publisher_creation() { let config = Arc::new(Config { listen_addr: "0.0.0.0:2055".to_string(), buffer_size: 65536, nats_url: "nats://localhost:4222".to_string(), stream_name: "flows".to_string(), subject: "flows.raw.netflow".to_string(), stream_subjects: None, stream_max_bytes: 10 * 1024 * 1024 * 1024, partition: "default".to_string(), max_templates: 2000, max_template_fields: 10_000, channel_size: 1000, batch_size: 100, publish_timeout_ms: 5000, drop_policy: crate::config::DropPolicy::DropOldest, security: None, metrics_addr: None, nats_creds_file: None, }); let (_tx, rx) = mpsc::channel(1000); let publisher = Publisher::new(config, rx); - assert!(publisher.config.subject == "flows.raw.netflow"); + assert_eq!(publisher.config.subject, "flows.raw.netflow"); } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the `test_publisher_creation` function is not a runnable test and proposes adding the `#[tokio::test]` attribute to fix it.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2658#issuecomment-3831730820 Original created: 2026-02-01T18:36:18Z --- ## 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 protocol mapping logic</summary> ___ **To improve maintainability, centralize the protocol number-to-name mapping <br>within the Rust collector. This involves removing the redundant fallback logic <br>from the OCSF mapping file and the Elixir web UI.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-f1817cc1d98f34d2d27d99bcc8471124179fd5ed61860e45b87d397ff3e08d9aR69-R69">packaging/zen/rules/netflow_to_ocsf.json [69]</a> </summary> ```json "value": "{ protocol_num: proto, protocol_name: protocol_name != '' ? protocol_name : (proto == 1 ? 'ICMP' : proto == 6 ? 'TCP' : proto == 17 ? 'UDP' : proto == 47 ? 'GRE' : proto == 50 ? 'ESP' : proto == 58 ? 'ICMPv6' : 'Unknown'), tcp_flags: tcp_flags, direction_id: 0, boundary_id: 0 }" ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2658/files#diff-7ec566a3a20e36d101f72bb9ee19620be76b397c1d88a1edc6d9a88e8da6909eR264-R275">web-ng/lib/serviceradar_web_ng_web/live/netflow_live/index.ex [264-275]</a> </summary> ```elixir {label, variant} = if is_binary(protocol_name) and protocol_name != "" do {String.upcase(protocol_name), protocol_variant(protocol_name)} else case protocol do 6 -> {"TCP", "success"} 17 -> {"UDP", "info"} 1 -> {"ICMP", "ghost"} nil -> {"Unknown", "ghost"} other -> {to_string(other), "ghost"} ... (clipped 2 lines) ``` </details> ### Solution Walkthrough: #### Before: ```elixir // packaging/zen/rules/netflow_to_ocsf.json "value": "{ ..., protocol_name: protocol_name != '' ? protocol_name : (proto == 1 ? 'ICMP' : proto == 6 ? 'TCP' : ...), ... }" // web-ng/.../index.ex defp protocol_badge(assigns) do protocol_name = Map.get(assigns, :protocol_name) protocol = assigns.protocol {label, variant} = if is_binary(protocol_name) and protocol_name != "" do {String.upcase(protocol_name), protocol_variant(protocol_name)} else case protocol do // Fallback logic 6 -> {"TCP", "success"} 17 -> {"UDP", "info"} ... end end ... end ``` #### After: ```elixir // packaging/zen/rules/netflow_to_ocsf.json "value": "{ ..., protocol_name: protocol_name, ... }" // web-ng/.../index.ex defp protocol_badge(assigns) do protocol_name = Map.get(assigns, :protocol_name, "UNKNOWN") label = String.upcase(protocol_name) variant = protocol_variant(protocol_name) ... end // rust/netflow-collector/src/converter.rs // Ensure protocol_name is always populated, e.g., with "UNKNOWN" for unmapped numbers. fn protocol_type_name(pt: ProtocolTypes) -> String { let name = format!("{pt:?}"); if name == "RESERVED" { "UNKNOWN".to_string() } else { name.to_uppercase() } } ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This is a significant architectural suggestion that correctly identifies duplicated logic introduced in the PR across multiple components, proposing a cleaner, more maintainable single-source-of-truth design. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>annotate async test properly</summary> ___ **Add the <code>#[tokio::test]</code> attribute to the <code>test_publisher_creation</code> function to <br>ensure it is executed by the test runner.** [rust/netflow-collector/src/publisher.rs [219-243]](https://github.com/carverauto/serviceradar/pull/2658/files#diff-10dec5866b4bc416c62590bdf2d3ded6c234b8d423ec4aebc70eac891ff0c07dR219-R243) ```diff +#[tokio::test] async fn test_publisher_creation() { let config = Arc::new(Config { listen_addr: "0.0.0.0:2055".to_string(), buffer_size: 65536, nats_url: "nats://localhost:4222".to_string(), stream_name: "flows".to_string(), subject: "flows.raw.netflow".to_string(), stream_subjects: None, stream_max_bytes: 10 * 1024 * 1024 * 1024, partition: "default".to_string(), max_templates: 2000, max_template_fields: 10_000, channel_size: 1000, batch_size: 100, publish_timeout_ms: 5000, drop_policy: crate::config::DropPolicy::DropOldest, security: None, metrics_addr: None, nats_creds_file: None, }); let (_tx, rx) = mpsc::channel(1000); let publisher = Publisher::new(config, rx); - assert!(publisher.config.subject == "flows.raw.netflow"); + assert_eq!(publisher.config.subject, "flows.raw.netflow"); } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that the `test_publisher_creation` function is not a runnable test and proposes adding the `#[tokio::test]` attribute to fix it. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>

mfreeman451 commented

2026-02-03 23:06:58 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2658#issuecomment-3844288052
Original created: 2026-02-03T23:06:58Z

closing, stale, already merged with another PR

Imported GitHub PR comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2658#issuecomment-3844288052 Original created: 2026-02-03T23:06:58Z --- closing, stale, already merged with another PR