zen build fixes #2296

Merged
mfreeman451 merged 1 commit from refs/pull/2296/head into main 2025-10-06 20:10:01 +00:00
mfreeman451 commented 2025-10-06 20:04:25 +00:00 (Migrated from github.com)
Owner

Imported from GitHub pull request.

Original GitHub pull request: #1725
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1725
Original created: 2025-10-06T20:04:25Z
Original updated: 2025-10-06T20:10:05Z
Original head: carverauto/serviceradar:k8s/zen_fixes
Original base: main
Original merged: 2025-10-06T20:10:01Z by @mfreeman451

PR Type

Bug fix, Enhancement


Description

  • Fix NATS stream subjects configuration for event handling

  • Add SNMP severity rule and improve rule installation

  • Code formatting and import organization improvements

  • Fix newline endings and remove extra whitespace


Diagram Walkthrough

flowchart LR
  A["NATS Stream Config"] --> B["Updated Subjects"]
  C["Rule Installation"] --> D["SNMP Severity Rule"]
  E["Code Formatting"] --> F["Import Organization"]
  G["File Cleanup"] --> H["Newline Fixes"]

File Walkthrough

Relevant files
Bug fix
1 files
events.go
Fix NATS stream subjects configuration                                     
+1/-1     
Formatting
12 files
lib.rs
Remove extra whitespace and fix imports                                   
+1/-2     
main.rs
Format code and fix newline ending                                             
+13/-12 
poller.rs
Reorder imports and fix newline ending                                     
+3/-3     
rperf.rs
Format code and improve readability                                           
+26/-11 
server.rs
Organize imports and format code                                                 
+51/-23 
build.rs
Fix newline ending                                                                             
+1/-1     
config.rs
Format code and fix newline ending                                             
+8/-3     
lib.rs
Remove whitespace and reorder imports                                       
+2/-3     
main.rs
Format code and fix newline ending                                             
+5/-2     
poller.rs
Organize imports and format code                                                 
+57/-38 
server.rs
Organize imports and format code                                                 
+20/-9   
lib.rs
Format match pattern for readability                                         
+3/-3     
Enhancement
6 files
put_rule.rs
Add KV bucket creation fallback                                                   
+13/-2   
entrypoint-zen.sh
Add rule seeding functionality                                                     
+20/-0   
zen-install-rules.sh
Refactor rule installation with SNMP support                         
+37/-40 
snmp_severity.json
Add SNMP severity rule configuration                                         
+22/-0   
serviceradar-zen-rules.yaml
Update rules format and add SNMP                                                 
+80/-64 
snmp_severity.json
Add SNMP severity rule file                                                           
+22/-0   
Configuration changes
3 files
BUILD.bazel
Add SNMP severity rule export                                                       
+1/-0     
BUILD.bazel
Include SNMP severity rule in package                                       
+2/-0     
components.json
Add SNMP severity rule to packaging                                           
+5/-0     

Imported from GitHub pull request. Original GitHub pull request: #1725 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1725 Original created: 2025-10-06T20:04:25Z Original updated: 2025-10-06T20:10:05Z Original head: carverauto/serviceradar:k8s/zen_fixes Original base: main Original merged: 2025-10-06T20:10:01Z by @mfreeman451 --- ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Fix NATS stream subjects configuration for event handling - Add SNMP severity rule and improve rule installation - Code formatting and import organization improvements - Fix newline endings and remove extra whitespace ___ ### Diagram Walkthrough ```mermaid flowchart LR A["NATS Stream Config"] --> B["Updated Subjects"] C["Rule Installation"] --> D["SNMP Severity Rule"] E["Code Formatting"] --> F["Import Organization"] G["File Cleanup"] --> H["Newline Fixes"] ``` <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><details><summary>1 files</summary><table> <tr> <td><strong>events.go</strong><dd><code>Fix NATS stream subjects configuration</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-3503010fc6a66fb16b8fbc055b7daf53be305d7284efe58d37a7fbf1813a6b63">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Formatting</strong></td><td><details><summary>12 files</summary><table> <tr> <td><strong>lib.rs</strong><dd><code>Remove extra whitespace and fix imports</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-273545b9f1e8a68d7e2d2a1372f282f539ef49cdca809b37a81e621188d826ca">+1/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Format code and fix newline ending</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></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-fb8dc04ef7087f15c7498a2d197e37fd629444ebff0291a3cc8ca49aabdc94cc">+13/-12</a>&nbsp; </td> </tr> <tr> <td><strong>poller.rs</strong><dd><code>Reorder imports and fix newline ending</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-889b4699b32d153c10acda4d076746d0014d4b97cc0021c789c0ad2f5687c187">+3/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>rperf.rs</strong><dd><code>Format code and improve readability</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-c1bff1ac0ccebf64439cfcd4d22255f9606b02de5c29d05929c590c6580b6bea">+26/-11</a>&nbsp; </td> </tr> <tr> <td><strong>server.rs</strong><dd><code>Organize imports and format code</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; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-bce0f4ca6548712f224b73816825d28e831acbbff7dbed3c98671ed50f65d028">+51/-23</a>&nbsp; </td> </tr> <tr> <td><strong>build.rs</strong><dd><code>Fix newline ending</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; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-2e519950c72a7e0bd53e22628bcd1a5dc31f6a6a0070b4c47221b1948a3b2099">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>config.rs</strong><dd><code>Format code and fix newline ending</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></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-46f593479dc3970ff83d81b23db32d37c4b22ff4491baa7a418f120dcb90db03">+8/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>lib.rs</strong><dd><code>Remove whitespace and reorder imports</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-e7e8897bb436806d2e6fb61aa2bdf32e7bf6c97915595d0e0987e43c725efad5">+2/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Format code and fix newline ending</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></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-cef134403622089f759cfa006eb04dc8f1cfda08497a9d38586cc4de03d14820">+5/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>poller.rs</strong><dd><code>Organize imports and format code</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; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-0d4828713c5d17e6ed70074eb16b948b8a7b6b14a8b43cfe76e3c78665a392ad">+57/-38</a>&nbsp; </td> </tr> <tr> <td><strong>server.rs</strong><dd><code>Organize imports and format code</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; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-2c4395fee16396339c3eea518ad9bec739174c67c9cedf62e6848c17136dd33e">+20/-9</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>lib.rs</strong><dd><code>Format match pattern for readability</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-8295e6c8105718065b7c8b401cee9efc20e2eb5010b2e1f8d91afe670498e2f7">+3/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Enhancement</strong></td><td><details><summary>6 files</summary><table> <tr> <td><strong>put_rule.rs</strong><dd><code>Add KV bucket creation fallback</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; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-beb89dda0f6336f7028dad3a2c0ff6e8874544eb71175df4263b11b8de41b3ad">+13/-2</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>entrypoint-zen.sh</strong><dd><code>Add rule seeding functionality</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></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-96e16293fe8e63a043f43ef9a4cca28c3b4933056b43e00ae11b6de7b1fede83">+20/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>zen-install-rules.sh</strong><dd><code>Refactor rule installation with SNMP support</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-74122772ee2960d407d0a0e24ac0cc9d970167ba0f51913e7edf29e092188003">+37/-40</a>&nbsp; </td> </tr> <tr> <td><strong>snmp_severity.json</strong><dd><code>Add SNMP severity rule configuration</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-c878688a2230e487cfe0e49de71b174e50975579c6b84988aa2dd32bc4fea4da">+22/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>serviceradar-zen-rules.yaml</strong><dd><code>Update rules format and add SNMP</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; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-68ac3ec024077fba26aed84721356fac5cc71e68f05e311c6ba7cd050dc5a485">+80/-64</a>&nbsp; </td> </tr> <tr> <td><strong>snmp_severity.json</strong><dd><code>Add SNMP severity rule file</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; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-e0bb2631982a545c8da636307f79a2785c8b0ee9fe3cbe37a112b78d07ca2fa1">+22/-0</a>&nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Configuration changes</strong></td><td><details><summary>3 files</summary><table> <tr> <td><strong>BUILD.bazel</strong><dd><code>Add SNMP severity rule export</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; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-90cac68b6fbfbbd4bf13f53b28eba4521b280f84f7745088a27978a956bd3297">+1/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>BUILD.bazel</strong><dd><code>Include SNMP severity rule in package</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-0e4db31c224a8f72ae8e870a849e38a59d74a2c7f7b04347b0b3eb07e20c5a80">+2/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>components.json</strong><dd><code>Add SNMP severity rule to packaging</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-3ae5949d89b0252d10fce9bf950231c8151a73b2154dccfe4e7261acc116582c">+5/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-10-06 20:05:32 +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/1725#issuecomment-3373804763
Original created: 2025-10-06T20:05:32Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Insecure default KV config

Description: The tool auto-creates a JetStream KV bucket with default settings if missing, which may
bypass intended server-side retention/replication policies and should be verified against
deployment security and data governance requirements.
put_rule.rs [85-99]

Referred Code
async fn main() -> Result<()> {
    let cli = Cli::parse();
    let cfg = Config::from_file(&cli.config)?;
    let js = connect_nats(&cfg).await?;
    let store = match js.get_key_value(&cfg.kv_bucket).await {
        Ok(store) => store,
        Err(_) => {
            let kv_config = KvConfig {
                bucket: cfg.kv_bucket.clone(),
                ..Default::default()
            };
            js.create_key_value(kv_config).await?;
            js.get_key_value(&cfg.kv_bucket).await?
        }
    };
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
No custom compliance provided

Follow the guide to enable custom compliance check.

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/1725#issuecomment-3373804763 Original created: 2025-10-06T20:05:32Z --- ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/d702f783b71b2898a1ffb6000a8f2b51b1add0d3 --> 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>Insecure default KV config </strong></summary><br> <b>Description:</b> The tool auto-creates a JetStream KV bucket with default settings if missing, which may <br>bypass intended server-side retention/replication policies and should be verified against <br>deployment security and data governance requirements.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/1725/files#diff-beb89dda0f6336f7028dad3a2c0ff6e8874544eb71175df4263b11b8de41b3adR85-R99'>put_rule.rs [85-99]</a></strong><br> <details open><summary>Referred Code</summary> ```rust async fn main() -> Result<()> { let cli = Cli::parse(); let cfg = Config::from_file(&cli.config)?; let js = connect_nats(&cfg).await?; let store = match js.get_key_value(&cfg.kv_bucket).await { Ok(store) => store, Err(_) => { let kv_config = KvConfig { bucket: cfg.kv_bucket.clone(), ..Default::default() }; js.create_key_value(kv_config).await?; js.get_key_value(&cfg.kv_bucket).await? } }; ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </summary></strong> - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </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>⚪</td><td><details><summary><strong>No custom compliance provided</strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/'>guide</a> to enable custom compliance check. </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 2025-10-06 20:06:47 +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/1725#issuecomment-3373811883
Original created: 2025-10-06T20:06:47Z

PR Code Suggestions

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Document the significant rule engine format change

The PR changes the Zen rule engine format from a list of rules to a graph-based
structure. This is a major breaking change that should be documented in the PR
description.

Examples:

k8s/demo/base/serviceradar-zen-rules.yaml [7-36]
  cef_severity.json: |
    {
      "nodes": [
        { "id": "inputNode", "type": "inputNode", "name": "Request", "position": {"x": 80, "y": 150} },
        {
          "id": "cefTable",
          "type": "decisionTableNode",
          "name": "CEF Severity",
          "position": {"x": 300, "y": 150},
          "content": {

 ... (clipped 20 lines)

Solution Walkthrough:

Before:

{
  "name": "CEF Severity Mapping",
  "description": "Map CEF severity levels...",
  "rules": [
    {
      "condition": {
        "field": "severity",
        "operator": "in",
        "value": ["0", "1", "2"]
      },
      "action": {
        "set_field": "sr_severity",
        "value": "low"
      }
    },
    ...
  ]
}

After:

{
  "nodes": [
    { "id": "inputNode", "type": "inputNode", ... },
    {
      "id": "cefTable",
      "type": "decisionTableNode",
      "name": "CEF Severity",
      "content": { ... }
    },
    { "id": "outputNode", "type": "outputNode", ... }
  ],
  "edges": [
    {"id": "e1", "sourceId": "inputNode", "targetId": "cefTable", "type": "edge"},
    ...
  ]
}

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a major, undocumented breaking change in the Zen rule engine's data format, which is critical for understanding the PR's impact and future maintenance.

High
Possible issue
Remove bypass edge in rule

Remove the bypass edge with id: "e3" from the snmp_severity.json rule. This edge
incorrectly routes data directly from input to output, skipping the setSeverity
processing node.

cmd/consumers/zen/data/snmp_severity.json [20]

-{ "id": "e3", "sourceId": "inputNode", "targetId": "outputNode", "type": "edge" }
 
+
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a logical bug in a new rule definition where a bypass edge (e3) would cause the setSeverity processing node to be skipped, defeating the purpose of the rule.

High
General
Handle specific error for bucket creation

Refactor the error handling for js.get_key_value. Instead of catching any error,
specifically check for StoreErrorKind::BucketNotFound before attempting to
create the key-value store, and propagate other errors.

cmd/consumers/zen/src/bin/put_rule.rs [89-99]

+use async_nats::jetstream::kv::StoreErrorKind;
+
 let store = match js.get_key_value(&cfg.kv_bucket).await {
     Ok(store) => store,
-    Err(_) => {
+    Err(err) if err.kind() == StoreErrorKind::BucketNotFound => {
         let kv_config = KvConfig {
             bucket: cfg.kv_bucket.clone(),
             ..Default::default()
         };
         js.create_key_value(kv_config).await?;
         js.get_key_value(&cfg.kv_bucket).await?
     }
+    Err(err) => return Err(err.into()),
 };
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly points out that blanket error handling can mask underlying issues like network failures. Handling the specific BucketNotFound error makes the error handling more robust and prevents incorrect state transitions.

Medium
Handle specific error for stream creation

Refactor the NATS stream creation logic. Specifically check for
jetstream.ErrStreamNotFound before attempting to create a stream, and use
js.CreateStream instead of js.CreateOrUpdateStream for safer creation.

pkg/natsutil/events.go [135-149]

 	// Ensure the stream exists
 	_, err = js.Stream(ctx, streamName)
 	if err != nil {
-		// Try to create the stream if it doesn't exist
-		streamConfig := jetstream.StreamConfig{
-			Name:     streamName,
-			Subjects: []string{"events.>", "snmp.traps"},
-		}
+		if err == jetstream.ErrStreamNotFound {
+			// Try to create the stream if it doesn't exist
+			streamConfig := jetstream.StreamConfig{
+				Name:     streamName,
+				Subjects: []string{"events.>", "snmp.traps"},
+			}
 
-		_, err = js.CreateOrUpdateStream(ctx, streamConfig)
-		if err != nil {
+			_, err = js.CreateStream(ctx, streamConfig)
+			if err != nil && err != jetstream.ErrStreamNameAlreadyInUse {
+				nc.Close()
+				return nil, nil, fmt.Errorf("failed to create stream %s: %w", streamName, err)
+			}
+		} else {
 			nc.Close()
-			return nil, nil, fmt.Errorf("failed to create or get stream %s: %w", streamName, err)
+			return nil, nil, fmt.Errorf("failed to get stream %s: %w", streamName, err)
 		}
 	}

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion improves error handling by checking for a specific ErrStreamNotFound error instead of any error, which prevents masking other potential problems. It also correctly recommends using CreateStream over CreateOrUpdateStream for safer, more explicit stream creation logic.

Medium
  • More
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1725#issuecomment-3373811883 Original created: 2025-10-06T20:06:47Z --- ## PR Code Suggestions ✨ <!-- d702f78 --> 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>Document the significant rule engine format change</summary> ___ **The PR changes the Zen rule engine format from a list of rules to a graph-based <br>structure. This is a major breaking change that should be documented in the PR <br>description.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1725/files#diff-68ac3ec024077fba26aed84721356fac5cc71e68f05e311c6ba7cd050dc5a485R7-R36">k8s/demo/base/serviceradar-zen-rules.yaml [7-36]</a> </summary> ```yaml cef_severity.json: | { "nodes": [ { "id": "inputNode", "type": "inputNode", "name": "Request", "position": {"x": 80, "y": 150} }, { "id": "cefTable", "type": "decisionTableNode", "name": "CEF Severity", "position": {"x": 300, "y": 150}, "content": { ... (clipped 20 lines) ``` </details> ### Solution Walkthrough: #### Before: ```yaml { "name": "CEF Severity Mapping", "description": "Map CEF severity levels...", "rules": [ { "condition": { "field": "severity", "operator": "in", "value": ["0", "1", "2"] }, "action": { "set_field": "sr_severity", "value": "low" } }, ... ] } ``` #### After: ```yaml { "nodes": [ { "id": "inputNode", "type": "inputNode", ... }, { "id": "cefTable", "type": "decisionTableNode", "name": "CEF Severity", "content": { ... } }, { "id": "outputNode", "type": "outputNode", ... } ], "edges": [ {"id": "e1", "sourceId": "inputNode", "targetId": "cefTable", "type": "edge"}, ... ] } ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a major, undocumented breaking change in the Zen rule engine's data format, which is critical for understanding the PR's impact and future maintenance. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Remove bypass edge in rule</summary> ___ **Remove the bypass edge with <code>id: "e3"</code> from the <code>snmp_severity.json</code> rule. This edge <br>incorrectly routes data directly from input to output, skipping the <code>setSeverity</code> <br>processing node.** [cmd/consumers/zen/data/snmp_severity.json [20]](https://github.com/carverauto/serviceradar/pull/1725/files#diff-c878688a2230e487cfe0e49de71b174e50975579c6b84988aa2dd32bc4fea4daR20-R20) ```diff -{ "id": "e3", "sourceId": "inputNode", "targetId": "outputNode", "type": "edge" } + ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies a logical bug in a new rule definition where a bypass edge (`e3`) would cause the `setSeverity` processing node to be skipped, defeating the purpose of the rule. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Handle specific error for bucket creation</summary> ___ **Refactor the error handling for <code>js.get_key_value</code>. Instead of catching any error, <br>specifically check for <code>StoreErrorKind::BucketNotFound</code> before attempting to <br>create the key-value store, and propagate other errors.** [cmd/consumers/zen/src/bin/put_rule.rs [89-99]](https://github.com/carverauto/serviceradar/pull/1725/files#diff-beb89dda0f6336f7028dad3a2c0ff6e8874544eb71175df4263b11b8de41b3adR89-R99) ```diff +use async_nats::jetstream::kv::StoreErrorKind; + let store = match js.get_key_value(&cfg.kv_bucket).await { Ok(store) => store, - Err(_) => { + Err(err) if err.kind() == StoreErrorKind::BucketNotFound => { let kv_config = KvConfig { bucket: cfg.kv_bucket.clone(), ..Default::default() }; js.create_key_value(kv_config).await?; js.get_key_value(&cfg.kv_bucket).await? } + Err(err) => return Err(err.into()), }; ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly points out that blanket error handling can mask underlying issues like network failures. Handling the specific `BucketNotFound` error makes the error handling more robust and prevents incorrect state transitions. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Handle specific error for stream creation</summary> ___ **Refactor the NATS stream creation logic. Specifically check for <br><code>jetstream.ErrStreamNotFound</code> before attempting to create a stream, and use <br><code>js.CreateStream</code> instead of <code>js.CreateOrUpdateStream</code> for safer creation.** [pkg/natsutil/events.go [135-149]](https://github.com/carverauto/serviceradar/pull/1725/files#diff-3503010fc6a66fb16b8fbc055b7daf53be305d7284efe58d37a7fbf1813a6b63R135-R149) ```diff // Ensure the stream exists _, err = js.Stream(ctx, streamName) if err != nil { - // Try to create the stream if it doesn't exist - streamConfig := jetstream.StreamConfig{ - Name: streamName, - Subjects: []string{"events.>", "snmp.traps"}, - } + if err == jetstream.ErrStreamNotFound { + // Try to create the stream if it doesn't exist + streamConfig := jetstream.StreamConfig{ + Name: streamName, + Subjects: []string{"events.>", "snmp.traps"}, + } - _, err = js.CreateOrUpdateStream(ctx, streamConfig) - if err != nil { + _, err = js.CreateStream(ctx, streamConfig) + if err != nil && err != jetstream.ErrStreamNameAlreadyInUse { + nc.Close() + return nil, nil, fmt.Errorf("failed to create stream %s: %w", streamName, err) + } + } else { nc.Close() - return nil, nil, fmt.Errorf("failed to create or get stream %s: %w", streamName, err) + return nil, nil, fmt.Errorf("failed to get stream %s: %w", streamName, err) } } ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion improves error handling by checking for a specific `ErrStreamNotFound` error instead of any error, which prevents masking other potential problems. It also correctly recommends using `CreateStream` over `CreateOrUpdateStream` for safer, more explicit stream creation logic. </details></details></td><td align=center>Medium </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.
No reviewers
No milestone
No project
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!2296
No description provided.