carverauto/serviceradar

Fork 0

2680 bug event rules not turning logs into events #2838

Merged

mfreeman451 merged 18 commits from refs/pull/2838/head into staging

2026-02-03 22:47:26 +00:00

mfreeman451 commented

2026-02-03 06:45:46 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2684
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2684
Original created: 2026-02-03T06:45:46Z
Original updated: 2026-02-03T22:47:43Z
Original head: carverauto/serviceradar:2680-bug-event-rules-not-turning-logs-into-events
Original base: staging
Original merged: 2026-02-03T22:47:26Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

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

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

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

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

PR Type

Bug fix, Enhancement

Description

Add log-promotion consumer to subscribe to processed logs and invoke promotion rules
Create ocsf_events hypertable migration with OCSF Event Log Activity schema
Implement log payload parser for normalized log extraction from JetStream subjects
Wire configuration, supervision, and health telemetry for promotion pipeline visibility
Enable log promotion consumer in demo and helm deployments by default

Diagram Walkthrough

flowchart LR
  A["Processed Logs<br/>logs.*.processed"] -->|JetStream| B["LogPromotionConsumer<br/>PullConsumer"]
  B -->|parse| C["LogPromotionParser<br/>normalize payload"]
  C -->|promote| D["LogPromotion.promote<br/>rule evaluation"]
  D -->|insert| E["ocsf_events<br/>hypertable"]
  B -->|health| F["ClusterHealth<br/>status tracking"]

File Walkthrough

Relevant files

Enhancement

5 files

log_promotion_consumer.ex `New JetStream pull consumer for log promotion`	+222/-0
log_promotion_parser.ex `New parser for processed log payload normalization`	+362/-0
20260203120000_create_ocsf_events.exs `New OCSF events hypertable migration with indexes`	+177/-0
application.ex `Add log promotion consumer to supervision tree`	+11/-0
cluster_health.ex `Track log promotion consumer health status`	+36/-6

Configuration changes

4 files

runtime.exs `Add log promotion consumer configuration and environment variables`	+14/-3
core.yaml `Add log promotion consumer enabled environment variable`	+2/-0
values.yaml `Enable log promotion consumer by default`	+1/-0
values-demo.yaml `Enable log promotion consumer in demo deployment`	+3/-0

Tests

1 files

log_promotion_parser_test.exs
Add tests for log promotion parser +64/-0

Formatting

1 files

.formatter.exs
Remove ash_authentication from formatter imports +0/-1

Documentation

5 files

proposal.md `Document log promotion pipeline restoration proposal`	+15/-0
design.md `Document design decisions and migration plan`	+35/-0
spec.md `Document log-to-event promotion requirements`	+14/-0
spec.md `Document OCSF events canonical table requirement`	+13/-0
tasks.md `Document implementation tasks and checklist`	+7/-0

Imported from GitHub pull request. Original GitHub pull request: #2684 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2684 Original created: 2026-02-03T06:45:46Z Original updated: 2026-02-03T22:47:43Z Original head: carverauto/serviceradar:2680-bug-event-rules-not-turning-logs-into-events Original base: staging Original merged: 2026-02-03T22:47:26Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Add log-promotion consumer to subscribe to processed logs and invoke promotion rules - Create `ocsf_events` hypertable migration with OCSF Event Log Activity schema - Implement log payload parser for normalized log extraction from JetStream subjects - Wire configuration, supervision, and health telemetry for promotion pipeline visibility - Enable log promotion consumer in demo and helm deployments by default ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Processed Logs<br/>logs.*.processed"] -->|JetStream| B["LogPromotionConsumer<br/>PullConsumer"] B -->|parse| C["LogPromotionParser<br/>normalize payload"] C -->|promote| D["LogPromotion.promote<br/>rule evaluation"] D -->|insert| E["ocsf_events<br/>hypertable"] B -->|health| F["ClusterHealth<br/>status tracking"] ``` <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>5 files</summary><table> <tr> <td><strong>log_promotion_consumer.ex</strong><dd><code>New JetStream pull consumer for log promotion</code>                        </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-045d588d5ff1296c4d2adce2e473071da1580f4c6ba46fdb73b6b83187e2f94e">+222/-0</a>  </td> </tr> <tr> <td><strong>log_promotion_parser.ex</strong><dd><code>New parser for processed log payload normalization</code>              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-a4f612616be94a9651763af208b2065f3d19065b7be4779d8c58cdcfed954016">+362/-0</a>  </td> </tr> <tr> <td><strong>20260203120000_create_ocsf_events.exs</strong><dd><code>New OCSF events hypertable migration with indexes</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-8650b38e310340a74acc1a3d5f347f1e43047ec55aba00e7e978a726680928da">+177/-0</a>  </td> </tr> <tr> <td><strong>application.ex</strong><dd><code>Add log promotion consumer to supervision tree</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-a9ffbf400b7f9b22cd8980c41286c54fe373f1f1a8684bb6a344a5fb39b178d0">+11/-0</a>    </td> </tr> <tr> <td><strong>cluster_health.ex</strong><dd><code>Track log promotion consumer health status</code>                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-65e2a238e7a24c82a3772842be52ddfeb155d66c6f1341e6e652ec3161c5d8bb">+36/-6</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Configuration changes</strong></td><td><details><summary>4 files</summary><table> <tr> <td><strong>runtime.exs</strong><dd><code>Add log promotion consumer configuration and environment variables</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-5d045e2af9f15590a9745058e25a00380ad7a2cc363d0a9934715bd11ea8eef3">+14/-3</a>    </td> </tr> <tr> <td><strong>core.yaml</strong><dd><code>Add log promotion consumer enabled environment variable</code>    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-06ab387d2c169d82a1de28b5e66c86f0417bd81b82a96246d0a2da8bfaa8d224">+2/-0</a>      </td> </tr> <tr> <td><strong>values.yaml</strong><dd><code>Enable log promotion consumer by default</code>                                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-d4449c7cb70362554b274f81eae5a4b81a8e81df494282e383d1b7ea3871c452">+1/-0</a>      </td> </tr> <tr> <td><strong>values-demo.yaml</strong><dd><code>Enable log promotion consumer in demo deployment</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-3a2c6c76ca4d5e8a336cd917d39b1704c03ea94a5cba4da1eb20629c63a5b914">+3/-0</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>log_promotion_parser_test.exs</strong><dd><code>Add tests for log promotion parser</code>                                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-336daacf4cdfcbbe1845dea441f3fde34d761c528e564c27a096568f2611388f">+64/-0</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Formatting</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>.formatter.exs</strong><dd><code>Remove ash_authentication from formatter imports</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-791d4757cee68d168d8ab62d3eca6562470207fa6b3ccbf124e35c3fbd9de875">+0/-1</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>5 files</summary><table> <tr> <td><strong>proposal.md</strong><dd><code>Document log promotion pipeline restoration proposal</code>          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-590c12ed43dbe47779232894231973bc5982adb83ea40ecf0701b69ecd79c3e5">+15/-0</a>    </td> </tr> <tr> <td><strong>design.md</strong><dd><code>Document design decisions and migration plan</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-4a277911385f5dda008a6a968ba84fecfd49dd2611f0eb5d2ccc90cd2cf8106a">+35/-0</a>    </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Document log-to-event promotion requirements</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-248ce75e0650179b1197e7d35962be167ebccb3bc339d9a04666fb26fcfe8cb3">+14/-0</a>    </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Document OCSF events canonical table requirement</code>                  </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-ac6f19f00b0bf7a66d610d31f39bdf8d36add70cba2c1a719c983c65ce486928">+13/-0</a>    </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Document implementation tasks and checklist</code>                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2684/files#diff-b6372ca306712bb74abd30ee98f7ca421e410cd9aa6d3ec149e6e05583b6398a">+7/-0</a>      </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-02-03 06:46: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/2684#issuecomment-3839395386
Original created: 2026-02-03T06:46:32Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

🟢

No security concerns identified

No security vulnerabilities detected by AI analysis. Human verification advised for critical code.

Ticket Compliance

🟡

🎫 #2680

🟢	Event rules created from recurring logs should result in events being created (i.e., logs should be promoted into events).
⚪	Created events should be visible in the Events UI (end-to-end pipeline works from log ingestion to UI query source).

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Comprehensive Audit Trails

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

Status: Passed

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

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

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

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

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

🔴

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unbounded retry loop: The consumer creation logic retries indefinitely (with fixed sleep) and the payload parser
silently drops invalid JSON, which can mask real failures and cause noisy/stuck behavior
without actionable error context.

Referred Code

          )

          {:ack,
           %{
             state
             | processed_count: state.processed_count + length(logs),
               promoted_count: state.promoted_count + count,
               last_error: nil
           }}

        {:error, reason} ->
          Logger.error("Log promotion failed", reason: inspect(reason), subject: subject)
          {:nack, %{state | last_error: reason}}
      end
  end
rescue
  error ->
    Logger.error("Log promotion consumer crashed", error: inspect(error), subject: subject)
    {:nack, %{state | last_error: error}}
end



 ... (clipped 66 lines)

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:
Potential sensitive logs: Error logs include inspect(reason)/inspect(error) which may serialize database errors or
other data that could include sensitive content depending on upstream failures and log
shipping configuration.

Referred Code

        {:error, reason} ->
          Logger.error("Log promotion failed", reason: inspect(reason), subject: subject)
          {:nack, %{state | last_error: reason}}
      end
  end
rescue
  error ->
    Logger.error("Log promotion consumer crashed", error: inspect(error), subject: subject)
    {:nack, %{state | last_error: error}}
end

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:
Unbounded attribute ingest: The parser merges essentially all non-reserved keys from external log payloads into
attributes without size/shape limits or sanitization, which could allow
oversized/untrusted data to be stored and processed downstream.

Referred Code

defp build_attributes(entry, subject) do
  base_attributes =
    entry
    |> first_value(["attributes"])
    |> FieldParser.encode_jsonb()
    |> ensure_map()

  extra_attributes = build_extra_attributes(entry)

  base_attributes
  |> Map.merge(extra_attributes)
  |> put_ingest_subject(subject)
end

defp build_extra_attributes(entry) do
  Enum.reduce(entry, %{}, fn {key, value}, acc ->
    if skip_attribute?(key, value) do
      acc
    else
      Map.put(acc, key, value)
    end


 ... (clipped 1 lines)

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/2684#issuecomment-3839395386 Original created: 2026-02-03T06:46:32Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2680>#2680</a></summary> <table width='100%'><tbody> <tr><td rowspan=1>🟢</td> <td>Event rules created from recurring logs should result in events being created (i.e., logs <br>should be promoted into events).</td></tr> <tr><td rowspan=1>⚪</td> <td>Created events should be visible in the Events UI (end-to-end pipeline works from log <br>ingestion to UI query source).</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=3>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=1>🔴</td> <td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2684/files#diff-045d588d5ff1296c4d2adce2e473071da1580f4c6ba46fdb73b6b83187e2f94eR90-R176'><strong>Unbounded retry loop</strong></a>: The consumer creation logic retries indefinitely (with fixed sleep) and the payload parser <br>silently drops invalid JSON, which can mask real failures and cause noisy/stuck behavior <br>without actionable error context.<br> <details open><summary>Referred Code</summary> ```elixir ) {:ack, %{ state | processed_count: state.processed_count + length(logs), promoted_count: state.promoted_count + count, last_error: nil }} {:error, reason} -> Logger.error("Log promotion failed", reason: inspect(reason), subject: subject) {:nack, %{state | last_error: reason}} end end rescue error -> Logger.error("Log promotion consumer crashed", error: inspect(error), subject: subject) {:nack, %{state | last_error: error}} end ... (clipped 66 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <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:** <br><a href='https://github.com/carverauto/serviceradar/pull/2684/files#diff-045d588d5ff1296c4d2adce2e473071da1580f4c6ba46fdb73b6b83187e2f94eR100-R109'><strong>Potential sensitive logs</strong></a>: Error logs include <code>inspect(reason)</code>/<code>inspect(error)</code> which may serialize database errors or <br>other data that could include sensitive content depending on upstream failures and log <br>shipping configuration.<br> <details open><summary>Referred Code</summary> ```elixir {:error, reason} -> Logger.error("Log promotion failed", reason: inspect(reason), subject: subject) {:nack, %{state | last_error: reason}} end end rescue error -> Logger.error("Log promotion consumer crashed", error: inspect(error), subject: subject) {:nack, %{state | last_error: error}} end ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><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:** <br><a href='https://github.com/carverauto/serviceradar/pull/2684/files#diff-a4f612616be94a9651763af208b2065f3d19065b7be4779d8c58cdcfed954016R255-R276'><strong>Unbounded attribute ingest</strong></a>: The parser merges essentially all non-reserved keys from external log payloads into <br><code>attributes</code> without size/shape limits or sanitization, which could allow <br>oversized/untrusted data to be stored and processed downstream.<br> <details open><summary>Referred Code</summary> ```elixir defp build_attributes(entry, subject) do base_attributes = entry |> first_value(["attributes"]) |> FieldParser.encode_jsonb() |> ensure_map() extra_attributes = build_extra_attributes(entry) base_attributes |> Map.merge(extra_attributes) |> put_ingest_subject(subject) end defp build_extra_attributes(entry) do Enum.reduce(entry, %{}, fn {key, value}, acc -> if skip_attribute?(key, value) do acc else Map.put(acc, key, value) end ... (clipped 1 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] 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-03 06:47:56 +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/2684#issuecomment-3839402121
Original created: 2026-02-03T06:47:56Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Filter out nil children After flattening the list of supervisor children, add a step to reject any `nil` entries to prevent the application from crashing if a child process is conditionally disabled. elixir/serviceradar_core/lib/serviceradar/application.ex [152] `... # after assembling children list \|> List.flatten() + \|> Enum.reject(&is_nil/1)` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: This suggestion prevents a potential runtime crash by filtering out `nil` values from the supervisor's child list, which is a critical fix for application stability.	Medium
Possible issue	Fix inconsistent metric counter logic In `handle_message`, avoid incrementing `processed_count` when the `logs` list is empty to ensure the metric consistently counts individual logs rather than messages. elixir/serviceradar_core/lib/serviceradar/observability/log_promotion_consumer.ex [79-98] `case logs do [] -> - {:ack, %{state \| processed_count: state.processed_count + 1}} + # No logs parsed, just ack the message. + {:ack, state} _ -> case LogPromotion.promote(logs) do {:ok, count} -> :telemetry.execute( [:serviceradar, :log_promotion, :consumer, :processed], %{logs: length(logs), events: count}, %{subject: subject} ) {:ack, %{ state \| processed_count: state.processed_count + length(logs), promoted_count: state.promoted_count + count, last_error: nil }} ...` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an inconsistency in how the `processed_count` metric is updated, which leads to inaccurate metrics if a message contains no logs.	Low
General	Use error codes for robust error handling In `create_consumer`, replace the fragile error string matching for an existing consumer with a more robust check on the specific NATS API error code (`err_code:` `10012`). elixir/serviceradar_core/lib/serviceradar/observability/log_promotion_consumer.ex [192-220] defp create_consumer(config) do consumer = %Consumer{ ... } case Consumer.create(config.connection_name, consumer) do {:ok, _} -> :ok + # err_code 10012 is "consumer name already exists" + {:error, {:jetstream_api_error, %{err_code: 10012}}} -> + :ok + {:error, %{"code" => 400, "description" => description}} when is_binary(description) -> - if String.contains?(description, "consumer name already") or - String.contains?(description, "consumer already exists") do + if String.contains?(description, "consumer name already", ignore_case: true) or + String.contains?(description, "consumer already exists", ignore_case: true) do :ok else {:error, %{"code" => 400, "description" => description}} end {:error, reason} -> {:error, reason} end end `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that matching on error strings is fragile and proposes a more robust solution by matching on a specific error code, improving maintainability.	Medium
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2684#issuecomment-3839402121 Original created: 2026-02-03T06:47:56Z --- ## 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=2>Possible issue</td> <td> <details><summary>Filter out nil children</summary> ___ **After flattening the list of supervisor children, add a step to reject any <code>nil</code> <br>entries to prevent the application from crashing if a child process is <br>conditionally disabled.** [elixir/serviceradar_core/lib/serviceradar/application.ex [152]](https://github.com/carverauto/serviceradar/pull/2684/files#diff-a9ffbf400b7f9b22cd8980c41286c54fe373f1f1a8684bb6a344a5fb39b178d0R152-R152) ```diff ... # after assembling children list |> List.flatten() + |> Enum.reject(&is_nil/1) ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This suggestion prevents a potential runtime crash by filtering out `nil` values from the supervisor's child list, which is a critical fix for application stability. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Fix inconsistent metric counter logic</summary> ___ **In <code>handle_message</code>, avoid incrementing <code>processed_count</code> when the <code>logs</code> list is <br>empty to ensure the metric consistently counts individual logs rather than <br>messages.** [elixir/serviceradar_core/lib/serviceradar/observability/log_promotion_consumer.ex [79-98]](https://github.com/carverauto/serviceradar/pull/2684/files#diff-045d588d5ff1296c4d2adce2e473071da1580f4c6ba46fdb73b6b83187e2f94eR79-R98) ```diff case logs do [] -> - {:ack, %{state | processed_count: state.processed_count + 1}} + # No logs parsed, just ack the message. + {:ack, state} _ -> case LogPromotion.promote(logs) do {:ok, count} -> :telemetry.execute( [:serviceradar, :log_promotion, :consumer, :processed], %{logs: length(logs), events: count}, %{subject: subject} ) {:ack, %{ state | processed_count: state.processed_count + length(logs), promoted_count: state.promoted_count + count, last_error: nil }} ... ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly identifies an inconsistency in how the `processed_count` metric is updated, which leads to inaccurate metrics if a message contains no logs. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Use error codes for robust error handling</summary> ___ **In <code>create_consumer</code>, replace the fragile error string matching for an existing <br>consumer with a more robust check on the specific NATS API error code (<code>err_code: </code><br><code>10012</code>).** [elixir/serviceradar_core/lib/serviceradar/observability/log_promotion_consumer.ex [192-220]](https://github.com/carverauto/serviceradar/pull/2684/files#diff-045d588d5ff1296c4d2adce2e473071da1580f4c6ba46fdb73b6b83187e2f94eR192-R220) ```diff defp create_consumer(config) do consumer = %Consumer{ ... } case Consumer.create(config.connection_name, consumer) do {:ok, _} -> :ok + # err_code 10012 is "consumer name already exists" + {:error, {:jetstream_api_error, %{err_code: 10012}}} -> + :ok + {:error, %{"code" => 400, "description" => description}} when is_binary(description) -> - if String.contains?(description, "consumer name already") or - String.contains?(description, "consumer already exists") do + if String.contains?(description, "consumer name already", ignore_case: true) or + String.contains?(description, "consumer already exists", ignore_case: true) do :ok else {:error, %{"code" => 400, "description" => description}} end {:error, reason} -> {:error, reason} end end ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly points out that matching on error strings is fragile and proposes a more robust solution by matching on a specific error code, improving maintainability. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>

qodo-code-review[bot] commented

2026-02-03 22:31:37 +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/2684#issuecomment-3844079085
Original created: 2026-02-03T22:31:37Z

CI Feedback 🧐

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

Action: build

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

Failed test name: ""

Failure summary:

The action failed during environment/secret validation because the required secret
SRQL_TEST_DATABASE_CA_CERT was not configured (it is empty in the job env).
The workflow explicitly
checks for this secret to verify the SRQL test fixture TLS and aborts with:
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS., exiting with code
1.

Relevant error logs:

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

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

356:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
357:  env:
358:  BUILDBUDDY_ORG_API_KEY: ***
359:  SRQL_TEST_DATABASE_URL: ***
360:  SRQL_TEST_ADMIN_URL: ***
361:  SRQL_TEST_DATABASE_CA_CERT: 
362:  DOCKERHUB_USERNAME: ***
363:  DOCKERHUB_TOKEN: ***
364:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
365:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
366:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
367:  ##[endgroup]
368:  ##[group]Run : install rustup if needed
369:  ^[[36;1m: install rustup if needed^[[0m
370:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
371:  ^[[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m
372:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

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

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

662:  env:
663:  BUILDBUDDY_ORG_API_KEY: ***
664:  SRQL_TEST_DATABASE_URL: ***
665:  SRQL_TEST_ADMIN_URL: ***
666:  SRQL_TEST_DATABASE_CA_CERT: 
667:  DOCKERHUB_USERNAME: ***
668:  DOCKERHUB_TOKEN: ***
669:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
670:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
671:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
672:  CARGO_HOME: /home/runner/.cargo
673:  CARGO_INCREMENTAL: 0
674:  CARGO_TERM_COLOR: always
675:  ##[endgroup]
676:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
677:  ##[error]Process completed with exit code 1.
678:  Post job cleanup.

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2684#issuecomment-3844079085 Original created: 2026-02-03T22:31:37Z --- ## CI Feedback 🧐 A test triggered by this PR failed. Here is an AI-generated analysis of the failure: <table><tr><td> **Action:** build</td></tr> <tr><td> **Failed stage:** [Configure SRQL fixture database for tests](https://github.com/carverauto/serviceradar/actions/runs/21650371644/job/62413131695) [❌] </td></tr> <tr><td> **Failed test name:** "" </td></tr> <tr><td> **Failure summary:** The action failed during environment/secret validation because the required secret <br><code>SRQL_TEST_DATABASE_CA_CERT</code> was not configured (it is empty in the job <code>env</code>).<br> The workflow explicitly <br>checks for this secret to verify the SRQL test fixture TLS and aborts with: <br><code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.</code>, exiting with code <br>1.<br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-q9bh5' 2: Runner group name: 'Default' ... 139: ^[[36;1mif command -v apt-get >/dev/null 2>&1; then^[[0m 140: ^[[36;1m sudo apt-get update^[[0m 141: ^[[36;1m sudo apt-get install -y build-essential pkg-config libssl-dev protobuf-compiler cmake flex bison^[[0m 142: ^[[36;1melif command -v dnf >/dev/null 2>&1; then^[[0m 143: ^[[36;1m sudo dnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 144: ^[[36;1melif command -v yum >/dev/null 2>&1; then^[[0m 145: ^[[36;1m sudo yum install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 146: ^[[36;1melif command -v microdnf >/dev/null 2>&1; then^[[0m 147: ^[[36;1m sudo microdnf install -y gcc gcc-c++ make openssl-devel protobuf-compiler cmake flex bison^[[0m 148: ^[[36;1melse^[[0m 149: ^[[36;1m echo "Unsupported package manager; please install gcc, g++ (or clang), make, OpenSSL headers, pkg-config, and protoc manually." >&2^[[0m 150: ^[[36;1m exit 1^[[0m 151: ^[[36;1mfi^[[0m 152: ^[[36;1m^[[0m 153: ^[[36;1mensure_pkg_config^[[0m 154: ^[[36;1mprotoc --version || (echo "protoc installation failed" && exit 1)^[[0m 155: shell: /usr/bin/bash -e {0} ... 356: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 357: env: 358: BUILDBUDDY_ORG_API_KEY: *** 359: SRQL_TEST_DATABASE_URL: *** 360: SRQL_TEST_ADMIN_URL: *** 361: SRQL_TEST_DATABASE_CA_CERT: 362: DOCKERHUB_USERNAME: *** 363: DOCKERHUB_TOKEN: *** 364: TEST_CNPG_DATABASE: serviceradar_web_ng_test 365: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 366: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 367: ##[endgroup] 368: ##[group]Run : install rustup if needed 369: ^[[36;1m: install rustup if needed^[[0m 370: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 371: ^[[36;1m curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y^[[0m 372: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 512: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 513: env: 514: BUILDBUDDY_ORG_API_KEY: *** 515: SRQL_TEST_DATABASE_URL: *** 516: SRQL_TEST_ADMIN_URL: *** 517: SRQL_TEST_DATABASE_CA_CERT: 518: DOCKERHUB_USERNAME: *** 519: DOCKERHUB_TOKEN: *** 520: TEST_CNPG_DATABASE: serviceradar_web_ng_test 521: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 522: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 523: CARGO_HOME: /home/runner/.cargo 524: CARGO_INCREMENTAL: 0 525: CARGO_TERM_COLOR: always 526: ##[endgroup] 527: ##[group]Run : work around spurious network errors in curl 8.0 528: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 529: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 580: SRQL_TEST_DATABASE_CA_CERT: 581: DOCKERHUB_USERNAME: *** 582: DOCKERHUB_TOKEN: *** 583: TEST_CNPG_DATABASE: serviceradar_web_ng_test 584: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 585: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 586: CARGO_HOME: /home/runner/.cargo 587: CARGO_INCREMENTAL: 0 588: CARGO_TERM_COLOR: always 589: ##[endgroup] 590: Attempting to download 1.x... 591: Acquiring v1.28.1 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.1/bazelisk-linux-amd64 592: Adding to the cache ... 593: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.1/x64 594: Added bazelisk to the path 595: ##[warning]Failed to restore: Cache service responded with 400 596: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 662: env: 663: BUILDBUDDY_ORG_API_KEY: *** 664: SRQL_TEST_DATABASE_URL: *** 665: SRQL_TEST_ADMIN_URL: *** 666: SRQL_TEST_DATABASE_CA_CERT: 667: DOCKERHUB_USERNAME: *** 668: DOCKERHUB_TOKEN: *** 669: TEST_CNPG_DATABASE: serviceradar_web_ng_test 670: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 671: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 672: CARGO_HOME: /home/runner/.cargo 673: CARGO_INCREMENTAL: 0 674: CARGO_TERM_COLOR: always 675: ##[endgroup] 676: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 677: ##[error]Process completed with exit code 1. 678: Post job cleanup. ``` </details></td></tr></table>