linted/fixing tests #2236

Merged

mfreeman451 merged 1 commit from refs/pull/2236/head into main 2025-09-23 17:29:29 +00:00

Conversation 2 Commits 1 Files changed 44 +785 -510

mfreeman451 commented 2025-09-23 17:09:09 +00:00 (Migrated from github.com)

Owner

Copy link

Imported from GitHub pull request.

Original GitHub pull request: #1656
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1656
Original created: 2025-09-23T17:09:09Z
Original updated: 2025-09-23T17:29:33Z
Original head: carverauto/serviceradar:updates/cicd_tests_failing
Original base: main
Original merged: 2025-09-23T17:29:29Z by @mfreeman451

PR Type

Tests, Enhancement

---

Description

• Refactored test constants and function signatures

• Applied code formatting and import organization

• Fixed linting issues across multiple languages

• Improved code consistency and readability

---

Diagram Walkthrough

flowchart LR
  A["Test Files"] --> B["Constant Extraction"]
  C["Rust Files"] --> D["Import Organization"]
  E["Go Files"] --> F["Function Signature Updates"]
  B --> G["Code Quality"]
  D --> G
  F --> G

File Walkthrough

	Relevant files
Tests	2 files tcp_optimization_test.go Extract test constant and update function calls                    +15/-13  integration_tests.rs Format test code and fix spacing                                                  +38/-17
Formatting	18 files adapter.rs Organize imports and format code structure                              +109/-71 otel_logs.rs Format code and fix line spacing                                                  +42/-34  lib.rs Organize imports and improve code formatting                          +74/-26  lib.rs Reorder imports and fix formatting                                              +51/-34  sysmon.rs Reorder imports and format function calls                                +80/-38  nats_output.rs Format code structure and improve readability                        +42/-31  message_processor.rs Format test code and fix spacing                                                  +34/-15  main.rs Extract constant and improve code structure                            +44/-41  main.rs Organize imports and format code                                                  +25/-18  main.rs Organize imports and format async code                                      +37/-21  rperf.rs Reorder imports and format function signatures                      +36/-21  server.rs Organize imports and format HTTP handlers                                +31/-38  processor.rs Format trait definitions and function signatures                  +19/-12  tls_output.rs Format error handling and path operations                                +13/-11  metrics.rs Organize imports and fix formatting                                            +6/-4      test_protobuf_conversion.rs Organize imports and format example code                                  +4/-4      main.rs Organize module imports and format main function                  +10/-4    mod.rs Format configuration parsing and conditionals                        +8/-5
Additional files	24 files build.rs +3/-1      config.rs +18/-6    engine.rs +1/-1      grpc_server.rs +4/-5      lib.rs +1/-1      main.rs +1/-1      nats.rs +1/-1      rule_watcher.rs +1/-1      build.rs +0/-1      mod.rs +5/-7      mod.rs +4/-3      udp_input.rs +3/-1      mod.rs +3/-7      mod.rs +2/-2      test_fuzzer.rs +1/-1      config.rs +5/-2      build.rs +1/-1      mod.rs +1/-1      types.rs +1/-1      mod.rs +1/-1      config.rs +6/-2      proton_client.ml +2/-2      proton_client.mli +2/-2      sql_ir.ml +0/-1

---

Imported from GitHub pull request. Original GitHub pull request: #1656 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1656 Original created: 2025-09-23T17:09:09Z Original updated: 2025-09-23T17:29:33Z Original head: carverauto/serviceradar:updates/cicd_tests_failing Original base: main Original merged: 2025-09-23T17:29:29Z by @mfreeman451 --- ### **PR Type** Tests, Enhancement ___ ### **Description** - Refactored test constants and function signatures - Applied code formatting and import organization - Fixed linting issues across multiple languages - Improved code consistency and readability ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Test Files"] --> B["Constant Extraction"] C["Rust Files"] --> D["Import Organization"] E["Go Files"] --> F["Function Signature Updates"] B --> G["Code Quality"] D --> G F --> G ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Tests</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>tcp_optimization_test.go</strong><dd><code>Extract test constant and update function calls</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-1218aed38178a35178132b397803ada99d736109eacf734a56014f77631ea076">+15/-13</a>&nbsp; </td> </tr> <tr> <td><strong>integration_tests.rs</strong><dd><code>Format test code and fix spacing</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/1656/files#diff-8f8472cec653bcfc677523f13262b63dbf7ea390d52249db1e52c670fe0ce60f">+38/-17</a>&nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Formatting</strong></td><td><details><summary>18 files</summary><table> <tr> <td><strong>adapter.rs</strong><dd><code>Organize imports and format code structure</code>&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/1656/files#diff-80ea8a2e03ec54b66babde1b4e8e6e07ab0f4550a71c715260eac4392c3c3f38">+109/-71</a></td> </tr> <tr> <td><strong>otel_logs.rs</strong><dd><code>Format code and fix line spacing</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/1656/files#diff-4542ace311c2de0c5f9389169b9b4b3429e972dde67ea1c6efb9dff04ec1c725">+42/-34</a>&nbsp; </td> </tr> <tr> <td><strong>lib.rs</strong><dd><code>Organize imports and improve code formatting</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/1656/files#diff-91f4558d22540a64796ec2ad9844eaaec577d90b0d8d2738eea0d2041837ead7">+74/-26</a>&nbsp; </td> </tr> <tr> <td><strong>lib.rs</strong><dd><code>Reorder imports and fix formatting</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/1656/files#diff-8295e6c8105718065b7c8b401cee9efc20e2eb5010b2e1f8d91afe670498e2f7">+51/-34</a>&nbsp; </td> </tr> <tr> <td><strong>sysmon.rs</strong><dd><code>Reorder imports and format function calls</code>&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/1656/files#diff-fa0ff2891411b4ebd350bb32e01a931868937ae7329d07226101d89b624bd8e3">+80/-38</a>&nbsp; </td> </tr> <tr> <td><strong>nats_output.rs</strong><dd><code>Format code structure and improve readability</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-6b585ea3564a481174e04da1270e2e13edd4e2b980d02a2652d6d21e6d82a498">+42/-31</a>&nbsp; </td> </tr> <tr> <td><strong>message_processor.rs</strong><dd><code>Format test code and fix spacing</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/1656/files#diff-9fcbc5358a9009e60a8cd22d21e5a9ea652787c727732d0b869e0865495114c3">+34/-15</a>&nbsp; </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Extract constant and improve code structure</code>&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/1656/files#diff-0928ea32b4e5970a3103f9391ff4debac80f03967326e55aafabca9b95b6c86e">+44/-41</a>&nbsp; </td> </tr> <tr> <td><strong>main.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/1656/files#diff-33b655d8730ae3e9c844ee280787d11f1b0d5343119188273f89558805f814ba">+25/-18</a>&nbsp; </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Organize imports and format async code</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/1656/files#diff-c10b431511497d683869182aaf88fb2ec422f3db7fe27ce5af646a9a4f8f3899">+37/-21</a>&nbsp; </td> </tr> <tr> <td><strong>rperf.rs</strong><dd><code>Reorder imports and format function signatures</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-31c6206b75a08b14a7d5a04fcbd878b2a70fa2ec949500d76f011869869e19b9">+36/-21</a>&nbsp; </td> </tr> <tr> <td><strong>server.rs</strong><dd><code>Organize imports and format HTTP handlers</code>&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/1656/files#diff-51fd677fa9647a20e4fd03970adc76521c8617111115825f8c474790888d88ff">+31/-38</a>&nbsp; </td> </tr> <tr> <td><strong>processor.rs</strong><dd><code>Format trait definitions and function signatures</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-f4cd8762c1866978ea1c2271c4f7f1343434d67eb487c96d75bafe1c88dca803">+19/-12</a>&nbsp; </td> </tr> <tr> <td><strong>tls_output.rs</strong><dd><code>Format error handling and path operations</code>&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/1656/files#diff-16141d5b9703bf496ce2b9bb40601e3071ddbbd7f0a4e5c021a85c7a106d8cfa">+13/-11</a>&nbsp; </td> </tr> <tr> <td><strong>metrics.rs</strong><dd><code>Organize imports and fix formatting</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/1656/files#diff-587346e3318054d8844f3c98bd615c9e53126af3de262c405f929508cdb585ae">+6/-4</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>test_protobuf_conversion.rs</strong><dd><code>Organize imports and format example code</code>&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/1656/files#diff-c54c03c920ed756b176d45cc029df0465170694ef55b4d249a1f8898dad6c4d6">+4/-4</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Organize module imports and format main function</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-f6d1216ef2bdbded76dcfbfed0ebb1f07ce21c27d56f609168cc0bb55fe7bc1d">+10/-4</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong><dd><code>Format configuration parsing and conditionals</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-dd7b7ea7c9d376611bb19203bd29fa729051a6bc61e98a76a62c5ae8e6939545">+8/-5</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Additional files</strong></td><td><details><summary>24 files</summary><table> <tr> <td><strong>build.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-4153a5e986ccd32258cb542124c79a00d348dc72dc8539a004c16787882dd45e">+3/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>config.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-05038f3867985e757de9027609950e682bad6d1992dac6acd7c28962a3c65dc4">+18/-6</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>engine.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-90476c419197f41cf4ea7847c91cefbc25e935ad581ce74e8f6e72ed7398a4ae">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>grpc_server.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-e4564a93f6cf84ff91cd3d8141fc9272ec9b4ec19defd107afa42be01fcfed5b">+4/-5</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>lib.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-35fd9a3361cdb159c06c3a51dbadb73fd00d76696fffca3b19bcc7e3b0ad39e4">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>main.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-78494042ba6078c9704eeb6e374d97a3481a5a74426db8e95ed34da15f9133b2">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>nats.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-97f7335def0ad5d644b594a1076ae2d7080b11259cbb8de22c7946cc8e4b39f8">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>rule_watcher.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-22e47be42ea9309e19c65dac41d6cd8218f6ebdc4c10d9c5c8a0863f996d2187">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>build.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-76371a21f53984490f7974e3e4892e795d92a4f4708e6ad5c74fed331953a005">+0/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-89aa5e80212ba1e12b87863590cc800b54d74edeabb21a880ffa9142f072b743">+5/-7</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-587d9b1e3ad27618f00a2ef16e32e28aca3d5c9ea0eb5c8af23e3b989c3c43f4">+4/-3</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>udp_input.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-cffc68de2fb2c7898014fa938a6014047f92128f76a0d7dcbdcb9bf17f1c159a">+3/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-43777ae71be078e49f7fc95a9f2bc415e0b431f1eecc93f99a6f9fc682b0c5f6">+3/-7</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-19049bdd775fa6c5a03db1f8db912ae7eac67d0f8b62e554f1f084226d055965">+2/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>test_fuzzer.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-121f51655f273eb3e77191f31a3af0cc1c5f4f80a267b7a42069a2897f72173e">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>config.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-abbaec651da3d6af96b482e0f77bb909b65dbe0cabd78b5803769cc9dab0a1b0">+5/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>build.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-660177a66a475c1af352388464e4dcf4fab913c3411b6f1315322dba0013c220">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-3696a6db231e9d81fc6f9df66758504d7c1f5cbe11c2e32805c8017c98e5089a">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>types.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-19d813700210df89bd4db1c31dee0aec8de9131dc40f49b52c4bd9865a59e618">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>mod.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-1c3183d9972270ced5bcb737070a8548f63b7be36280adf94ea055d2c94e1925">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>config.rs</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-c89b88ba4d2bf0a054d0ba69a672a92c30140b8d19503d67b980a218ffe3106d">+6/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>proton_client.ml</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-474590ab3468b98ee363f3cd78535c90bc83cf3b2e77ae9871db741e2d8dea06">+2/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>proton_client.mli</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-2b549609ea8b875de4a039282ea09be7655ebda3f27f078ea603b8e77b9e31c6">+2/-2</a>&nbsp; &nbsp; &nbsp; </td> </tr> <tr> <td><strong>sql_ir.ml</strong></td> <td><a href="https://github.com/carverauto/serviceradar/pull/1656/files#diff-e8a30619726b7581ba83efc140c894b178903ecd638dbe08b430f75e29efd7a1">+0/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented 2025-09-23 17:10:03 +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/1656#issuecomment-3324886647
Original created: 2025-09-23T17:10:03Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Helper Consistency The new helper expectKVBootstrap hardcodes testKVNamespace and changes the previous signature that accepted a key. Verify all call sites and related tests still cover non-default namespaces and that no tests implicitly relied on passing different keys. func expectKVBootstrap(mockKV *MockKVStore) { mockKV.EXPECT().Get(gomock.Any(), testKVNamespace).Return(nil, false, nil).AnyTimes() mockKV.EXPECT().Watch(gomock.Any(), testKVNamespace).DoAndReturn(func(context.Context, string) (<-chan []byte, error) { Watch Stream Error Handling watch_apply spawns a task consuming the stream but discards errors from the spawned task and the stream; consider surfacing/logging stream errors and handling backoff/reconnect to avoid silent failures. pub async fn watch_apply<F>(&mut self, key: &str, mut apply: F) -> Result<()> where F: FnMut(&[u8]) + Send + 'static, { let resp = self .inner .watch(kvproto::WatchRequest { key: key.to_string(), }) .await .map_err(|e| KvError::Other(e.into()))?; let mut stream = resp.into_inner(); tokio::spawn(async move { while let Ok(Some(item)) = stream.next().await.transpose() { Attribute Parsing Robustness When extracting attributes for HTTP/gRPC, only StringValue is captured; numeric status codes are stringified if present as strings. Validate behavior when attributes are numeric (e.g., http.status_code as IntValue) to avoid missing metrics fields. // Extract additional context from span attributes let mut span_attrs = std::collections::HashMap::new(); for attr in &span.attributes { if let Some(value) = &attr.value && let Some( opentelemetry::proto::common::v1::any_value::Value::StringValue(s), ) = &value.value { span_attrs.insert(attr.key.as_str(), s.as_str()); } } // Record Prometheus metrics let span_kind = metrics::span_kind_to_string(span.kind); metrics::record_span_metrics( service_name, &span.name, span_kind, duration_seconds, &span_attrs, ); let is_slow = duration_ms > 100.0; // Create base performance metric let base_metric = PerformanceMetric { timestamp: current_time.clone(), trace_id: trace_id.clone(), span_id: span_id.clone(), service_name: service_name.to_string(), span_name: span.name.clone(), span_kind: span_kind.to_string(), duration_ms, duration_seconds, metric_type: "span".to_string(), http_method: None, http_route: None, http_status_code: None, grpc_service: None, grpc_method: None, grpc_status_code: None, is_slow, component: "otel-collector".to_string(), level: if is_slow { "warn".to_string() } else { "info".to_string() }, }; // Add base span metric performance_metrics.push(base_metric.clone()); // Add HTTP-specific metric if available if let (Some(method), Some(route)) = (span_attrs.get("http.method"), span_attrs.get("http.route")) { let mut http_metric = base_metric.clone(); http_metric.metric_type = "http".to_string(); http_metric.http_method = Some(method.to_string()); http_metric.http_route = Some(route.to_string()); http_metric.http_status_code = span_attrs.get("http.status_code").map(|s| s.to_string()); performance_metrics.push(http_metric); } // Add gRPC-specific metric if available if let Some(grpc_method) = span_attrs.get("rpc.method") { let grpc_service = span_attrs.get("rpc.service").map_or("unknown", |v| *v); let mut grpc_metric = base_metric.clone(); grpc_metric.metric_type = "grpc".to_string(); grpc_metric.grpc_service = Some(grpc_service.to_string()); grpc_metric.grpc_method = Some(grpc_method.to_string()); grpc_metric.grpc_status_code = span_attrs .get("rpc.grpc.status_code") .map(|s| s.to_string()); performance_metrics.push(grpc_metric);

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1656#issuecomment-3324886647 Original created: 2025-09-23T17:10:03Z --- ## PR Reviewer Guide 🔍 Here are some key observations to aid the review process: <table> <tr><td>⏱️&nbsp;<strong>Estimated effort to review</strong>: 4 🔵🔵🔵🔵⚪</td></tr> <tr><td>🧪&nbsp;<strong>PR contains tests</strong></td></tr> <tr><td>🔒&nbsp;<strong>No security concerns identified</strong></td></tr> <tr><td>⚡&nbsp;<strong>Recommended focus areas for review</strong><br><br> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1656/files#diff-1218aed38178a35178132b397803ada99d736109eacf734a56014f77631ea076R54-R56'><strong>Test Helper Consistency</strong></a> The new helper `expectKVBootstrap` hardcodes `testKVNamespace` and changes the previous signature that accepted a key. Verify all call sites and related tests still cover non-default namespaces and that no tests implicitly relied on passing different keys. </summary> ```go func expectKVBootstrap(mockKV *MockKVStore) { mockKV.EXPECT().Get(gomock.Any(), testKVNamespace).Return(nil, false, nil).AnyTimes() mockKV.EXPECT().Watch(gomock.Any(), testKVNamespace).DoAndReturn(func(context.Context, string) (<-chan []byte, error) { ``` </details> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1656/files#diff-91f4558d22540a64796ec2ad9844eaaec577d90b0d8d2738eea0d2041837ead7R88-R101'><strong>Watch Stream Error Handling</strong></a> `watch_apply` spawns a task consuming the stream but discards errors from the spawned task and the stream; consider surfacing/logging stream errors and handling backoff/reconnect to avoid silent failures. </summary> ```rust pub async fn watch_apply<F>(&mut self, key: &str, mut apply: F) -> Result<()> where F: FnMut(&[u8]) + Send + 'static, { let resp = self .inner .watch(kvproto::WatchRequest { key: key.to_string(), }) .await .map_err(|e| KvError::Other(e.into()))?; let mut stream = resp.into_inner(); tokio::spawn(async move { while let Ok(Some(item)) = stream.next().await.transpose() { ``` </details> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1656/files#diff-8295e6c8105718065b7c8b401cee9efc20e2eb5010b2e1f8d91afe670498e2f7R205-R281'><strong>Attribute Parsing Robustness</strong></a> When extracting attributes for HTTP/gRPC, only StringValue is captured; numeric status codes are stringified if present as strings. Validate behavior when attributes are numeric (e.g., http.status_code as IntValue) to avoid missing metrics fields. </summary> ```rust // Extract additional context from span attributes let mut span_attrs = std::collections::HashMap::new(); for attr in &span.attributes { if let Some(value) = &attr.value && let Some( opentelemetry::proto::common::v1::any_value::Value::StringValue(s), ) = &value.value { span_attrs.insert(attr.key.as_str(), s.as_str()); } } // Record Prometheus metrics let span_kind = metrics::span_kind_to_string(span.kind); metrics::record_span_metrics( service_name, &span.name, span_kind, duration_seconds, &span_attrs, ); let is_slow = duration_ms > 100.0; // Create base performance metric let base_metric = PerformanceMetric { timestamp: current_time.clone(), trace_id: trace_id.clone(), span_id: span_id.clone(), service_name: service_name.to_string(), span_name: span.name.clone(), span_kind: span_kind.to_string(), duration_ms, duration_seconds, metric_type: "span".to_string(), http_method: None, http_route: None, http_status_code: None, grpc_service: None, grpc_method: None, grpc_status_code: None, is_slow, component: "otel-collector".to_string(), level: if is_slow { "warn".to_string() } else { "info".to_string() }, }; // Add base span metric performance_metrics.push(base_metric.clone()); // Add HTTP-specific metric if available if let (Some(method), Some(route)) = (span_attrs.get("http.method"), span_attrs.get("http.route")) { let mut http_metric = base_metric.clone(); http_metric.metric_type = "http".to_string(); http_metric.http_method = Some(method.to_string()); http_metric.http_route = Some(route.to_string()); http_metric.http_status_code = span_attrs.get("http.status_code").map(|s| s.to_string()); performance_metrics.push(http_metric); } // Add gRPC-specific metric if available if let Some(grpc_method) = span_attrs.get("rpc.method") { let grpc_service = span_attrs.get("rpc.service").map_or("unknown", |v| *v); let mut grpc_metric = base_metric.clone(); grpc_metric.metric_type = "grpc".to_string(); grpc_metric.grpc_service = Some(grpc_service.to_string()); grpc_metric.grpc_method = Some(grpc_method.to_string()); grpc_metric.grpc_status_code = span_attrs .get("rpc.grpc.status_code") .map(|s| s.to_string()); performance_metrics.push(grpc_metric); ``` </details> </td></tr> </table>

qodo-code-review[bot] commented 2025-09-23 17:11:16 +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/1656#issuecomment-3324891756
Original created: 2025-09-23T17:11:16Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use correct scheme based on TLS configuration In create_channel, determine the connection scheme (http or https) based on the TLS configuration instead of unconditionally defaulting to https://. This ensures connections to non-TLS endpoints are possible. cmd/poller-ng/src/adapter.rs [108-112] let addr = if !addr.starts_with("http://") && !addr.starts_with("https://") { - format!("https://{}", addr) // Use https for mTLS + let scheme = if security.as_ref().map_or(false, |s| s.tls.enabled) { + "https" + } else { + "http" + }; + format!("{}://{}", scheme, addr) } else { addr.to_string() }; [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical bug that would prevent connections to non-TLS endpoints by incorrectly forcing https://. The fix is essential for the poller to function in mixed or non-TLS environments.	High
	Fix incorrect scheme for agent health checks In ensure_agent_health, remove the hardcoded http:// scheme and pass the raw agent_addr to create_channel. This allows create_channel to correctly infer the protocol scheme based on the security configuration. cmd/poller-ng/src/adapter.rs [451-473] -let addr = format!("http://{}", agent_addr); self.exponential_backoff( || async { - info!("Checking health of agent at {}", addr); - let channel = Self::create_channel(&addr, security) + info!("Checking health of agent at {}", agent_addr); + let channel = Self::create_channel(agent_addr, security) .await .context(format!("Failed to connect to agent at {}", agent_addr))?; let mut health_client = HealthClient::new(channel); let response = health_client .check(HealthCheckRequest { // service: "monitoring.AgentService".to_string(), service: "".to_string(), }) .await?; if response.get_ref().status != 1 { return Err(anyhow::anyhow!("Agent {} is unhealthy", agent_addr)); } Ok(()) }, 4, // Max retries &format!("health check for agent {}", agent_addr), ) .await [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical bug where hardcoding the http:// scheme for health checks breaks TLS-enabled agents. The proposed fix is crucial for ensuring health checks work correctly in all security configurations.	High
	Handle process spawn failure during restart Handle the error from cmd.spawn() when restarting the process on a configuration update. Only exit the current process if the new process spawns successfully to prevent an outage. cmd/flowgger/src/main.rs [56-58] -let _ = cmd.spawn(); // fire-and-forget -std::process::exit(0); +if let Err(e) = cmd.spawn() { + let _ = writeln!(stderr(), "Failed to spawn new process on config update: {e}"); + // Do not exit, so the current process continues running. +} else { + std::process::exit(0); +} Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a critical reliability issue where a failed process restart would cause an outage, and the proposed fix makes the auto-restart mechanism more robust.	Medium
General	Handle non-string span attribute types Expand span attribute extraction to handle non-string types like integers, booleans, and doubles by converting them to strings, preventing loss of metric data. cmd/otel/src/lib.rs [208-214] -if let Some(value) = &attr.value - && let Some( - opentelemetry::proto::common::v1::any_value::Value::StringValue(s), - ) = &value.value -{ - span_attrs.insert(attr.key.as_str(), s.as_str()); +if let Some(value) = &attr.value { + let str_val = match &value.value { + Some(opentelemetry::proto::common::v1::any_value::Value::StringValue(s)) => Some(s.as_str()), + Some(opentelemetry::proto::common::v1::any_value::Value::IntValue(i)) => Some(Box::leak(i.to_string().into_boxed_str())), + Some(opentelemetry::proto::common::v1::any_value::Value::DoubleValue(d)) => Some(Box::leak(d.to_string().into_boxed_str())), + Some(opentelemetry::proto::common::v1::any_value::Value::BoolValue(b)) => Some(if *b { "true" } else { "false" }), + _ => None, + }; + if let Some(s) = str_val { + span_attrs.insert(attr.key.as_str(), s); + } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that only string attributes are handled, leading to loss of valuable metric data. The proposed change improves metric completeness, but the use of Box::leak is not ideal and may introduce memory leaks.	Medium
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1656#issuecomment-3324891756 Original created: 2025-09-23T17:11:16Z --- ## PR Code Suggestions ✨ <!-- 95f9767 --> 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=3>Possible issue</td> <td> <details><summary>Use correct scheme based on TLS configuration</summary> ___ **In <code>create_channel</code>, determine the connection scheme (<code>http</code> or <code>https</code>) based on the <br>TLS configuration instead of unconditionally defaulting to <code>https://</code>. This <br>ensures connections to non-TLS endpoints are possible.** [cmd/poller-ng/src/adapter.rs [108-112]](https://github.com/carverauto/serviceradar/pull/1656/files#diff-80ea8a2e03ec54b66babde1b4e8e6e07ab0f4550a71c715260eac4392c3c3f38R108-R112) ```diff let addr = if !addr.starts_with("http://") && !addr.starts_with("https://") { - format!("https://{}", addr) // Use https for mTLS + let scheme = if security.as_ref().map_or(false, |s| s.tls.enabled) { + "https" + } else { + "http" + }; + format!("{}://{}", scheme, addr) } else { addr.to_string() }; ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies a critical bug that would prevent connections to non-TLS endpoints by incorrectly forcing `https://`. The fix is essential for the poller to function in mixed or non-TLS environments. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Fix incorrect scheme for agent health checks</summary> ___ **In <code>ensure_agent_health</code>, remove the hardcoded <code>http://</code> scheme and pass the raw <br><code>agent_addr</code> to <code>create_channel</code>. This allows <code>create_channel</code> to correctly infer the <br>protocol scheme based on the security configuration.** [cmd/poller-ng/src/adapter.rs [451-473]](https://github.com/carverauto/serviceradar/pull/1656/files#diff-80ea8a2e03ec54b66babde1b4e8e6e07ab0f4550a71c715260eac4392c3c3f38R451-R473) ```diff -let addr = format!("http://{}", agent_addr); self.exponential_backoff( || async { - info!("Checking health of agent at {}", addr); - let channel = Self::create_channel(&addr, security) + info!("Checking health of agent at {}", agent_addr); + let channel = Self::create_channel(agent_addr, security) .await .context(format!("Failed to connect to agent at {}", agent_addr))?; let mut health_client = HealthClient::new(channel); let response = health_client .check(HealthCheckRequest { // service: "monitoring.AgentService".to_string(), service: "".to_string(), }) .await?; if response.get_ref().status != 1 { return Err(anyhow::anyhow!("Agent {} is unhealthy", agent_addr)); } Ok(()) }, 4, // Max retries &format!("health check for agent {}", agent_addr), ) .await ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies a critical bug where hardcoding the `http://` scheme for health checks breaks TLS-enabled agents. The proposed fix is crucial for ensuring health checks work correctly in all security configurations. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Handle process spawn failure during restart</summary> ___ **Handle the error from <code>cmd.spawn()</code> when restarting the process on a configuration <br>update. Only exit the current process if the new process spawns successfully to <br>prevent an outage.** [cmd/flowgger/src/main.rs [56-58]](https://github.com/carverauto/serviceradar/pull/1656/files#diff-c10b431511497d683869182aaf88fb2ec422f3db7fe27ce5af646a9a4f8f3899R56-R58) ```diff -let _ = cmd.spawn(); // fire-and-forget -std::process::exit(0); +if let Err(e) = cmd.spawn() { + let _ = writeln!(stderr(), "Failed to spawn new process on config update: {e}"); + // Do not exit, so the current process continues running. +} else { + std::process::exit(0); +} ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies a critical reliability issue where a failed process restart would cause an outage, and the proposed fix makes the auto-restart mechanism more robust. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Handle non-string span attribute types</summary> ___ **Expand span attribute extraction to handle non-string types like integers, <br>booleans, and doubles by converting them to strings, preventing loss of metric <br>data.** [cmd/otel/src/lib.rs [208-214]](https://github.com/carverauto/serviceradar/pull/1656/files#diff-8295e6c8105718065b7c8b401cee9efc20e2eb5010b2e1f8d91afe670498e2f7R208-R214) ```diff -if let Some(value) = &attr.value - && let Some( - opentelemetry::proto::common::v1::any_value::Value::StringValue(s), - ) = &value.value -{ - span_attrs.insert(attr.key.as_str(), s.as_str()); +if let Some(value) = &attr.value { + let str_val = match &value.value { + Some(opentelemetry::proto::common::v1::any_value::Value::StringValue(s)) => Some(s.as_str()), + Some(opentelemetry::proto::common::v1::any_value::Value::IntValue(i)) => Some(Box::leak(i.to_string().into_boxed_str())), + Some(opentelemetry::proto::common::v1::any_value::Value::DoubleValue(d)) => Some(Box::leak(d.to_string().into_boxed_str())), + Some(opentelemetry::proto::common::v1::any_value::Value::BoolValue(b)) => Some(if *b { "true" } else { "false" }), + _ => None, + }; + if let Some(s) = str_val { + span_attrs.insert(attr.key.as_str(), s); + } } ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=3 --> <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out that only string attributes are handled, leading to loss of valuable metric data. The proposed change improves metric completeness, but the use of `Box::leak` is not ideal and may introduce memory leaks. </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.

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

Subscribe

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!2236

No description provided.