carverauto/serviceradar

Fork 0

2339 buglogs lost otel log formatschema #2699

Merged

mfreeman451 merged 2 commits from refs/pull/2699/head into staging

2026-01-18 17:50:22 +00:00

mfreeman451 commented

2026-01-18 17:47:30 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2354
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2354
Original created: 2026-01-18T17:47:30Z
Original updated: 2026-01-18T17:50:24Z
Original head: carverauto/serviceradar:2339-buglogs-lost-otel-log-formatschema
Original base: staging
Original merged: 2026-01-18T17:50:22Z 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

Restore OTEL log schema fields (resource_attributes, scope_name, scope_version) across ingestion and UI
Normalize non-OTEL log sources into OTEL record format with proper field mapping
Enhance log detail view to display resource attributes and scope metadata
Refactor log parsing to extract and preserve OTEL-specific fields from multiple source formats

Diagram Walkthrough

flowchart LR
  A["Log Ingestion<br/>OTEL/Syslog/GELF"] -->|"Parse & Normalize"| B["OTEL Log Record<br/>with Schema Fields"]
  B -->|"Extract Fields"| C["resource_attributes<br/>scope_name/version<br/>service metadata"]
  C -->|"Store & Query"| D["Database<br/>OTEL Schema"]
  D -->|"Return Fields"| E["Logs UI Detail View<br/>Display Metadata"]

File Walkthrough

Relevant files

Enhancement

4 files

logs.ex `Refactor log parsing to extract OTEL schema fields`	+65/-20
show.ex `Display resource attributes in log detail view`	+21/-11
json_logs.go `Refactor JSON log parsing to preserve OTEL fields`	+243/-67
otel_logs.rs `Extract and normalize OTEL log record fields`	+75/-5

Tests

2 files

show_test.exs `Add tests for resource attributes rendering`	+25/-1
integration_tests.rs `Update tests for OTEL field extraction`	+6/-3

Formatting

4 files

rules_live_test.exs `Fix formatting in test assertions`	+9/-3
index.ex `Minor spacing adjustment in UI text`	+1/-1
alerts.rs `Reformat query filter code for readability`	+3/-8
logs.rs `Reformat UUID parsing error handling`	+4/-6

Documentation

5 files

AGENTS.md `Update documentation on MCP tools usage`	+1/-3
proposal.md `Add change proposal for OTEL log schema restoration`	+14/-0
design.md `Add design document for OTEL schema changes`	+27/-0
spec.md `Add requirements for OTEL log schema visibility`	+22/-0
tasks.md `Add implementation tasks for OTEL schema restoration`	+6/-0

Imported from GitHub pull request. Original GitHub pull request: #2354 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2354 Original created: 2026-01-18T17:47:30Z Original updated: 2026-01-18T17:50:24Z Original head: carverauto/serviceradar:2339-buglogs-lost-otel-log-formatschema Original base: staging Original merged: 2026-01-18T17:50:22Z 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** - Restore OTEL log schema fields (resource_attributes, scope_name, scope_version) across ingestion and UI - Normalize non-OTEL log sources into OTEL record format with proper field mapping - Enhance log detail view to display resource attributes and scope metadata - Refactor log parsing to extract and preserve OTEL-specific fields from multiple source formats ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Log Ingestion<br/>OTEL/Syslog/GELF"] -->|"Parse & Normalize"| B["OTEL Log Record<br/>with Schema Fields"] B -->|"Extract Fields"| C["resource_attributes<br/>scope_name/version<br/>service metadata"] C -->|"Store & Query"| D["Database<br/>OTEL Schema"] D -->|"Return Fields"| E["Logs UI Detail View<br/>Display Metadata"] ``` <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>4 files</summary><table> <tr> <td><strong>logs.ex</strong><dd><code>Refactor log parsing to extract OTEL schema fields</code>              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-60c56675b539f19f1cce7702a571d883df17ce5c20a32bdc95afcd329b0b0efd">+65/-20</a>  </td> </tr> <tr> <td><strong>show.ex</strong><dd><code>Display resource attributes in log detail view</code>                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-4f9769353c55928a0d382cd7510379444445aea116e1ecdf7b8eb892d249ff26">+21/-11</a>  </td> </tr> <tr> <td><strong>json_logs.go</strong><dd><code>Refactor JSON log parsing to preserve OTEL fields</code>                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42">+243/-67</a></td> </tr> <tr> <td><strong>otel_logs.rs</strong><dd><code>Extract and normalize OTEL log record fields</code>                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-4542ace311c2de0c5f9389169b9b4b3429e972dde67ea1c6efb9dff04ec1c725">+75/-5</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>show_test.exs</strong><dd><code>Add tests for resource attributes rendering</code>                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-2d62472b3e212a4da643a2f66d2a07d358795602e680f31249a162b71fa0ea3b">+25/-1</a>    </td> </tr> <tr> <td><strong>integration_tests.rs</strong><dd><code>Update tests for OTEL field extraction</code>                                      </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-8f8472cec653bcfc677523f13262b63dbf7ea390d52249db1e52c670fe0ce60f">+6/-3</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Formatting</strong></td><td><details><summary>4 files</summary><table> <tr> <td><strong>rules_live_test.exs</strong><dd><code>Fix formatting in test assertions</code>                                                </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-dddd0d747bd725738a10e74770d765ed98fb22cdd5cd702bcbe98cc47c5af5d0">+9/-3</a>      </td> </tr> <tr> <td><strong>index.ex</strong><dd><code>Minor spacing adjustment in UI text</code>                                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-b2127e71582033bc6dfd2d7397f56bf43c1f7c613defffc504b3d8ee1e7406c4">+1/-1</a>      </td> </tr> <tr> <td><strong>alerts.rs</strong><dd><code>Reformat query filter code for readability</code>                              </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-b7cd072de79e393d78f338a0fd97a15798bf38372fe58cc1349636d049733f27">+3/-8</a>      </td> </tr> <tr> <td><strong>logs.rs</strong><dd><code>Reformat UUID parsing error handling</code>                                          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-f4d3b33667f2e79a6ebe4cfff931f93c728d9a81c305ac13586e623850a504db">+4/-6</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>5 files</summary><table> <tr> <td><strong>AGENTS.md</strong><dd><code>Update documentation on MCP tools usage</code>                                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-a54ff182c7e8acf56acfd6e4b9c3ff41e2c41a31c9b211b2deb9df75d9a478f9">+1/-3</a>      </td> </tr> <tr> <td><strong>proposal.md</strong><dd><code>Add change proposal for OTEL log schema restoration</code>            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-e2ec43171daa2c2aba7382d9bcd3a39f10f102f019ac8b2c718ef8e1209e8011">+14/-0</a>    </td> </tr> <tr> <td><strong>design.md</strong><dd><code>Add design document for OTEL schema changes</code>                            </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-636ed5005c5553d21f7fdfaf0334c56c6e49dfe3dcf2368fb83001cc51a26b82">+27/-0</a>    </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Add requirements for OTEL log schema visibility</code>                    </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-6f6a749ea2ab57cb2d0f4768d4b4817aff93d4c353779af296e9f2b8564734e3">+22/-0</a>    </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Add implementation tasks for OTEL schema restoration</code>          </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-d39d8463ab316bb79b9898a5225d03269d05bc29f59d8fc6e86bb45ddb56258d">+6/-0</a>      </td> </tr> </table></details></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-18 17:48:11 +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/2354#issuecomment-3765541377
Original created: 2026-01-18T17:48:11Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

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

🟡

🎫 #2339

🟢	Restore OTEL logs format/schema so OTEL-specific fields (resource attributes, scope metadata, attributes, trace/span ids, etc.) are preserved and visible again in the Logs UI rather than only showing `Time \| Level \| Service \| Message`.
🟢	Enhance log detail view to display OTEL metadata such as resource attributes and scope information.
⚪	Ensure OTEL Logs, OTEL Metrics, and OTEL Traces use OTEL format; normalize non-OTEL log sources (e.g., syslog/GELF) into an OTEL record format for consistency.

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:
Silent marshal failure: The new encodeAttributes helper silently returns an empty string when json.Marshal fails,
losing context without any error propagation or logging.

Referred Code

func encodeAttributes(attributes map[string]interface{}) string {
	if len(attributes) == 0 {
		return ""
	}

	encoded, err := json.Marshal(attributes)
	if err != nil {
		return ""
	}

	return string(encoded)
}

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:
Attribute passthrough risk: The new normalization merges arbitrary entry fields into attributes/resource_attributes
which could inadvertently store sensitive values from log payloads, and the diff does not
show any explicit allowlist/redaction controls.

Referred Code

resourceMap := extractAttributesMap(entry, "resource_attributes", "resourceAttributes", "resource")
resourceAttribs := resourceMap
if len(resourceAttribs) == 0 {
	resourceAttribs = buildResourceAttributesMap(entry)
}

attributesMap := extractAttributesMap(entry, "attributes")
extraAttributes := buildAttributesMap(entry, jsonLogReservedKeys)
attributesMap = mergeAttributeMaps(attributesMap, extraAttributes)

serviceName := firstString(entry, "service.name", "service_name", "service", "serviceName")
host := firstString(entry, "host", "hostname")
if serviceName == "" {
	serviceName = host
}
if serviceName == "" {
	serviceName = firstStringFromMap(resourceAttribs, "service.name", "service_name", "serviceName")
}

serviceVersion := firstString(entry, "service.version", "service_version", "serviceVersion")
if serviceVersion == "" {


 ... (clipped 71 lines)

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 parsing: The new parseAttributeValue/parseKeyValueMap logic accepts and parses free-form strings
into maps without visible size/complexity limits, which may be risky for hostile or
extremely large log inputs.

Referred Code

func parseAttributeValue(value interface{}) map[string]interface{} {
	switch typed := value.(type) {
	case map[string]interface{}:
		return typed
	case string:
		text := strings.TrimSpace(typed)
		if text == "" {
			return nil
		}

		var decoded map[string]interface{}
		if err := json.Unmarshal([]byte(text), &decoded); err == nil {
			return decoded
		}

		return parseKeyValueMap(text)
	default:
		return nil
	}
}



 ... (clipped 37 lines)

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

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2354#issuecomment-3765541377 Original created: 2026-01-18T17:48:11Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## 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/2339>#2339</a></summary> <table width='100%'><tbody> <tr><td rowspan=2>🟢</td> <td>Restore OTEL logs format/schema so OTEL-specific fields (resource attributes, scope <br>metadata, attributes, trace/span ids, etc.) are preserved and visible again in the Logs UI <br>rather than only showing <code>Time | Level | Service | Message</code>.</td></tr> <tr><td>Enhance log detail view to display OTEL metadata such as resource attributes and scope <br>information.</td></tr> <tr><td rowspan=1>⚪</td> <td>Ensure OTEL Logs, OTEL Metrics, and OTEL Traces use OTEL format; normalize non-OTEL log <br>sources (e.g., syslog/GELF) into an OTEL record format for consistency.</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/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42R476-R487'><strong>Silent marshal failure</strong></a>: The new <code>encodeAttributes</code> helper silently returns an empty string when <code>json.Marshal</code> fails, <br>losing context without any error propagation or logging.<br> <details open><summary>Referred Code</summary> ```go func encodeAttributes(attributes map[string]interface{}) string { if len(attributes) == 0 { return "" } encoded, err := json.Marshal(attributes) if err != nil { return "" } return string(encoded) } ``` </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/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42R114-R205'><strong>Attribute passthrough risk</strong></a>: The new normalization merges arbitrary <code>entry</code> fields into <code>attributes</code>/<code>resource_attributes</code> <br>which could inadvertently store sensitive values from log payloads, and the diff does not <br>show any explicit allowlist/redaction controls.<br> <details open><summary>Referred Code</summary> ```go resourceMap := extractAttributesMap(entry, "resource_attributes", "resourceAttributes", "resource") resourceAttribs := resourceMap if len(resourceAttribs) == 0 { resourceAttribs = buildResourceAttributesMap(entry) } attributesMap := extractAttributesMap(entry, "attributes") extraAttributes := buildAttributesMap(entry, jsonLogReservedKeys) attributesMap = mergeAttributeMaps(attributesMap, extraAttributes) serviceName := firstString(entry, "service.name", "service_name", "service", "serviceName") host := firstString(entry, "host", "hostname") if serviceName == "" { serviceName = host } if serviceName == "" { serviceName = firstStringFromMap(resourceAttribs, "service.name", "service_name", "serviceName") } serviceVersion := firstString(entry, "service.version", "service_version", "serviceVersion") if serviceVersion == "" { ... (clipped 71 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><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/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42R417-R474'><strong>Unbounded parsing</strong></a>: The new <code>parseAttributeValue</code>/<code>parseKeyValueMap</code> logic accepts and parses free-form strings <br>into maps without visible size/complexity limits, which may be risky for hostile or <br>extremely large log inputs.<br> <details open><summary>Referred Code</summary> ```go func parseAttributeValue(value interface{}) map[string]interface{} { switch typed := value.(type) { case map[string]interface{}: return typed case string: text := strings.TrimSpace(typed) if text == "" { return nil } var decoded map[string]interface{} if err := json.Unmarshal([]byte(text), &decoded); err == nil { return decoded } return parseKeyValueMap(text) default: return nil } } ... (clipped 37 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">   </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-01-18 17:48:22 +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/2354#issuecomment-3765541543
Original created: 2026-01-18T17:48:22Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

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 at lines 540 and 626).
The
workflow explicitly checks for this secret to verify the SRQL fixture TLS and aborts with:
SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. (line 636), exiting
with code 1 (line 637).

Relevant error logs:

1:  Runner name: 'arc-runner-set-hk6mk-runner-vmc2t'
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}
...

316:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
317:  env:
318:  BUILDBUDDY_ORG_API_KEY: ***
319:  SRQL_TEST_DATABASE_URL: ***
320:  SRQL_TEST_ADMIN_URL: ***
321:  SRQL_TEST_DATABASE_CA_CERT: 
322:  DOCKERHUB_USERNAME: ***
323:  DOCKERHUB_TOKEN: ***
324:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
325:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
326:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
327:  ##[endgroup]
328:  ##[group]Run : install rustup if needed
329:  ^[[36;1m: install rustup if needed^[[0m
330:  ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m
331:  ^[[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
332:  ^[[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m
...

472:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
473:  env:
474:  BUILDBUDDY_ORG_API_KEY: ***
475:  SRQL_TEST_DATABASE_URL: ***
476:  SRQL_TEST_ADMIN_URL: ***
477:  SRQL_TEST_DATABASE_CA_CERT: 
478:  DOCKERHUB_USERNAME: ***
479:  DOCKERHUB_TOKEN: ***
480:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
481:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
482:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
483:  CARGO_HOME: /home/runner/.cargo
484:  CARGO_INCREMENTAL: 0
485:  CARGO_TERM_COLOR: always
486:  ##[endgroup]
487:  ##[group]Run : work around spurious network errors in curl 8.0
488:  ^[[36;1m: work around spurious network errors in curl 8.0^[[0m
489:  ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m
...

540:  SRQL_TEST_DATABASE_CA_CERT: 
541:  DOCKERHUB_USERNAME: ***
542:  DOCKERHUB_TOKEN: ***
543:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
544:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
545:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
546:  CARGO_HOME: /home/runner/.cargo
547:  CARGO_INCREMENTAL: 0
548:  CARGO_TERM_COLOR: always
549:  ##[endgroup]
550:  Attempting to download 1.x...
551:  Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64
552:  Adding to the cache ...
553:  Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64
554:  Added bazelisk to the path
555:  ##[warning]Failed to restore: Cache service responded with 400
556:  Restored bazelisk cache dir @ /home/runner/.cache/bazelisk
...

622:  env:
623:  BUILDBUDDY_ORG_API_KEY: ***
624:  SRQL_TEST_DATABASE_URL: ***
625:  SRQL_TEST_ADMIN_URL: ***
626:  SRQL_TEST_DATABASE_CA_CERT: 
627:  DOCKERHUB_USERNAME: ***
628:  DOCKERHUB_TOKEN: ***
629:  TEST_CNPG_DATABASE: serviceradar_web_ng_test
630:  INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp
631:  INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir
632:  CARGO_HOME: /home/runner/.cargo
633:  CARGO_INCREMENTAL: 0
634:  CARGO_TERM_COLOR: always
635:  ##[endgroup]
636:  SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.
637:  ##[error]Process completed with exit code 1.
638:  Post job cleanup.

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2354#issuecomment-3765541543 Original created: 2026-01-18T17:48:22Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## 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/21116007244/job/60721625212) [❌] </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 env at lines 540 and 626).<br> The <br>workflow explicitly checks for this secret to verify the SRQL fixture TLS and aborts with: <br><code>SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS.</code> (line 636), exiting <br>with code 1 (line 637).<br> </td></tr> <tr><td> <details><summary>Relevant error logs:</summary> ```yaml 1: Runner name: 'arc-runner-set-hk6mk-runner-vmc2t' 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} ... 316: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 317: env: 318: BUILDBUDDY_ORG_API_KEY: *** 319: SRQL_TEST_DATABASE_URL: *** 320: SRQL_TEST_ADMIN_URL: *** 321: SRQL_TEST_DATABASE_CA_CERT: 322: DOCKERHUB_USERNAME: *** 323: DOCKERHUB_TOKEN: *** 324: TEST_CNPG_DATABASE: serviceradar_web_ng_test 325: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 326: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 327: ##[endgroup] 328: ##[group]Run : install rustup if needed 329: ^[[36;1m: install rustup if needed^[[0m 330: ^[[36;1mif ! command -v rustup &>/dev/null; then^[[0m 331: ^[[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 332: ^[[36;1m echo "$CARGO_HOME/bin" >> $GITHUB_PATH^[[0m ... 472: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 473: env: 474: BUILDBUDDY_ORG_API_KEY: *** 475: SRQL_TEST_DATABASE_URL: *** 476: SRQL_TEST_ADMIN_URL: *** 477: SRQL_TEST_DATABASE_CA_CERT: 478: DOCKERHUB_USERNAME: *** 479: DOCKERHUB_TOKEN: *** 480: TEST_CNPG_DATABASE: serviceradar_web_ng_test 481: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 482: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 483: CARGO_HOME: /home/runner/.cargo 484: CARGO_INCREMENTAL: 0 485: CARGO_TERM_COLOR: always 486: ##[endgroup] 487: ##[group]Run : work around spurious network errors in curl 8.0 488: ^[[36;1m: work around spurious network errors in curl 8.0^[[0m 489: ^[[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation^[[0m ... 540: SRQL_TEST_DATABASE_CA_CERT: 541: DOCKERHUB_USERNAME: *** 542: DOCKERHUB_TOKEN: *** 543: TEST_CNPG_DATABASE: serviceradar_web_ng_test 544: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 545: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 546: CARGO_HOME: /home/runner/.cargo 547: CARGO_INCREMENTAL: 0 548: CARGO_TERM_COLOR: always 549: ##[endgroup] 550: Attempting to download 1.x... 551: Acquiring v1.28.0 from https://github.com/bazelbuild/bazelisk/releases/download/v1.28.0/bazelisk-linux-amd64 552: Adding to the cache ... 553: Successfully cached bazelisk to /home/runner/_work/_tool/bazelisk/1.28.0/x64 554: Added bazelisk to the path 555: ##[warning]Failed to restore: Cache service responded with 400 556: Restored bazelisk cache dir @ /home/runner/.cache/bazelisk ... 622: env: 623: BUILDBUDDY_ORG_API_KEY: *** 624: SRQL_TEST_DATABASE_URL: *** 625: SRQL_TEST_ADMIN_URL: *** 626: SRQL_TEST_DATABASE_CA_CERT: 627: DOCKERHUB_USERNAME: *** 628: DOCKERHUB_TOKEN: *** 629: TEST_CNPG_DATABASE: serviceradar_web_ng_test 630: INSTALL_DIR_FOR_OTP: /home/runner/_work/_temp/.setup-beam/otp 631: INSTALL_DIR_FOR_ELIXIR: /home/runner/_work/_temp/.setup-beam/elixir 632: CARGO_HOME: /home/runner/.cargo 633: CARGO_INCREMENTAL: 0 634: CARGO_TERM_COLOR: always 635: ##[endgroup] 636: SRQL_TEST_DATABASE_CA_CERT secret must be configured to verify SRQL fixture TLS. 637: ##[error]Process completed with exit code 1. 638: Post job cleanup. ``` </details></td></tr></table>

qodo-code-review[bot] commented

2026-01-18 17:49:26 +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/2354#issuecomment-3765542567
Original created: 2026-01-18T17:49:26Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate redundant log processing logic The PR introduces nearly identical log normalization logic in both Go and Elixir components. This should be consolidated into a single service to avoid redundancy and reduce maintenance. Examples: pkg/consumers/db-event-writer/json_logs.go [99-172] `func buildJSONLogRow(entry map[string]interface{}, subject string) models.OTELLogRow { timestamp := time.Now().UTC() if value, ok := firstValue(entry, "timestamp", "time", "ts", "@timestamp"); ok { if parsed, ok := parseFlexibleTime(value); ok { timestamp = parsed } } body := firstString(entry, "message", "short_message", "msg", "body", "event", "log", "summary") if body == "" { ... (clipped 64 lines)` elixir/serviceradar_core/lib/serviceradar/event_writer/processors/logs.ex [82-117] `defp parse_json_log(json, metadata) when is_map(json) do log_id = Ash.UUID.generate() attributes = FieldParser.encode_jsonb(json["attributes"]) \|\| %{} attributes = attach_ingest_metadata(attributes, metadata) resource_attributes = normalize_resource_attributes(json) {scope_name, scope_version} = parse_scope_fields(json) %{ id: log_id, timestamp: parse_timestamp(json), ... (clipped 26 lines)` Solution Walkthrough: Before: // In pkg/consumers/db-event-writer/json_logs.go func buildJSONLogRow(entry map[string]interface{}, ...) models.OTELLogRow { // ... complex logic to find service_name, scope, resource attributes ... resourceMap := extractAttributesMap(entry, "resource_attributes", "resource") serviceName := firstString(entry, "service.name", "service_name") if serviceName == "" { serviceName = firstStringFromMap(resourceMap, "service.name") } scopeName, scopeVersion = applyScopeFallbacks(entry, scopeName, scopeVersion) // ... } # In elixir/serviceradar_core/lib/serviceradar/event_writer/processors/logs.ex defp parse_json_log(json, metadata) do # ... similar complex logic ... resource_attributes = normalize_resource_attributes(json) {scope_name, scope_version} = parse_scope_fields(json) service_name = service_field(json, resource_attributes, "service_name", ..., "service.name") # ... end After: // In a single, canonical log processing service (e.g., Go) func normalizeLog(entry map[string]interface{}) models.OTELLogRow { // Centralized complex logic to find service_name, scope, resource attributes resourceMap := extractAttributesMap(entry, "resource_attributes", "resource") serviceName := firstString(entry, "service.name", "service_name") if serviceName == "" { serviceName = firstStringFromMap(resourceMap, "service.name") } scopeName, scopeVersion = applyScopeFallbacks(entry, scopeName, scopeVersion) // ... } # Other services (e.g., Elixir) would call this canonical service/library # instead of reimplementing the normalization logic. defp parse_json_log(json, metadata) do # Call the canonical normalization service normalized_log = CanonicalLogNormalizer.normalize(json) # ... end Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies significant duplication of complex log normalization logic between the Go (`json_logs.go`) and Elixir (`logs.ex`) components, which is a major architectural concern impacting maintainability.	High
General	Skip all-zero IDs Update `bytes_to_hex` to return `None` if the input byte slice contains only zeros, preventing invalid all-zero trace or span IDs from being generated. cmd/consumers/zen/src/otel_logs.rs [146-156] `fn bytes_to_hex(bytes: &[u8]) -> Option<String> { - if bytes.is_empty() { + if bytes.is_empty() \|\| bytes.iter().all(\|&b\| b == 0) { return None; } let mut out = String::with_capacity(bytes.len() * 2); for byte in bytes { let _ = write!(out, "{:02x}", byte); } Some(out) }` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that all-zero trace/span IDs are invalid and should be treated as absent, improving data quality by preventing meaningless IDs from being processed.	Medium
	Support numeric & bool attrs Modify `get_attr_string` to convert numeric and boolean attribute values to strings, ensuring they are not dropped if they are not already string types. cmd/consumers/zen/src/otel_logs.rs [135-144] `fn get_attr_string(attrs: &Value, key: &str) -> String { match attrs { - Value::Object(map) => map - .get(key) - .and_then(\|value\| value.as_str()) - .unwrap_or_default() - .to_string(), + Value::Object(map) => { + map.get(key).map(\|v\| { + if let Some(s) = v.as_str() { + s.to_string() + } else { + v.to_string() + } + }).unwrap_or_default() + } _ => String::new(), } }` Apply / Chat Suggestion importance[1-10]: 7 __ Why: This change makes the `get_attr_string` function more robust by handling non-string attribute values, preventing potential data loss when attributes like version numbers are numeric.	Medium
	Correct attribute map merge priority Refactor `mergeAttributeMaps` to be more idiomatic by iterating over the `extra` map first, then the `base` map, which simplifies the logic by removing an existence check. pkg/consumers/db-event-writer/json_logs.go [489-507] `func mergeAttributeMaps(base, extra map[string]interface{}) map[string]interface{} { if len(base) == 0 && len(extra) == 0 { return nil } merged := make(map[string]interface{}) - for key, value := range base { + for key, value := range extra { merged[key] = value } - for key, value := range extra { - if _, exists := merged[key]; exists { - continue - } + for key, value := range base { merged[key] = value } return merged }` Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion correctly identifies the desired merge priority but misinterprets the existing code, which already implements it; the proposed change is a minor refactoring for readability with no functional difference.	Low
Possible issue	Use empty JSON object In `encodeAttributes`, return an empty JSON object (`{}`) instead of an empty string when the attribute map is empty or marshaling fails to ensure consumers always receive valid JSON. pkg/consumers/db-event-writer/json_logs.go [476-487] `func encodeAttributes(attributes map[string]interface{}) string { if len(attributes) == 0 { - return "" + return "{}" } encoded, err := json.Marshal(attributes) if err != nil { - return "" + return "{}" } return string(encoded) }` Apply / Chat Suggestion importance[1-10]: 6 __ Why: Returning an empty JSON object (`{}`) instead of an empty string for attribute fields ensures type consistency for downstream consumers, which is a good practice for data integrity.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2354#issuecomment-3765542567 Original created: 2026-01-18T17:49:26Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Consolidate redundant log processing logic</summary> ___ **The PR introduces nearly identical log normalization logic in both Go and Elixir <br>components. This should be consolidated into a single service to avoid <br>redundancy and reduce maintenance.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42R99-R172">pkg/consumers/db-event-writer/json_logs.go [99-172]</a> </summary> ```go func buildJSONLogRow(entry map[string]interface{}, subject string) models.OTELLogRow { timestamp := time.Now().UTC() if value, ok := firstValue(entry, "timestamp", "time", "ts", "@timestamp"); ok { if parsed, ok := parseFlexibleTime(value); ok { timestamp = parsed } } body := firstString(entry, "message", "short_message", "msg", "body", "event", "log", "summary") if body == "" { ... (clipped 64 lines) ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2354/files#diff-60c56675b539f19f1cce7702a571d883df17ce5c20a32bdc95afcd329b0b0efdR82-R117">elixir/serviceradar_core/lib/serviceradar/event_writer/processors/logs.ex [82-117]</a> </summary> ```elixir defp parse_json_log(json, metadata) when is_map(json) do log_id = Ash.UUID.generate() attributes = FieldParser.encode_jsonb(json["attributes"]) || %{} attributes = attach_ingest_metadata(attributes, metadata) resource_attributes = normalize_resource_attributes(json) {scope_name, scope_version} = parse_scope_fields(json) %{ id: log_id, timestamp: parse_timestamp(json), ... (clipped 26 lines) ``` </details> ### Solution Walkthrough: #### Before: ```elixir // In pkg/consumers/db-event-writer/json_logs.go func buildJSONLogRow(entry map[string]interface{}, ...) models.OTELLogRow { // ... complex logic to find service_name, scope, resource attributes ... resourceMap := extractAttributesMap(entry, "resource_attributes", "resource") serviceName := firstString(entry, "service.name", "service_name") if serviceName == "" { serviceName = firstStringFromMap(resourceMap, "service.name") } scopeName, scopeVersion = applyScopeFallbacks(entry, scopeName, scopeVersion) // ... } # In elixir/serviceradar_core/lib/serviceradar/event_writer/processors/logs.ex defp parse_json_log(json, metadata) do # ... similar complex logic ... resource_attributes = normalize_resource_attributes(json) {scope_name, scope_version} = parse_scope_fields(json) service_name = service_field(json, resource_attributes, "service_name", ..., "service.name") # ... end ``` #### After: ```elixir // In a single, canonical log processing service (e.g., Go) func normalizeLog(entry map[string]interface{}) models.OTELLogRow { // Centralized complex logic to find service_name, scope, resource attributes resourceMap := extractAttributesMap(entry, "resource_attributes", "resource") serviceName := firstString(entry, "service.name", "service_name") if serviceName == "" { serviceName = firstStringFromMap(resourceMap, "service.name") } scopeName, scopeVersion = applyScopeFallbacks(entry, scopeName, scopeVersion) // ... } # Other services (e.g., Elixir) would call this canonical service/library # instead of reimplementing the normalization logic. defp parse_json_log(json, metadata) do # Call the canonical normalization service normalized_log = CanonicalLogNormalizer.normalize(json) # ... end ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies significant duplication of complex log normalization logic between the Go (`json_logs.go`) and Elixir (`logs.ex`) components, which is a major architectural concern impacting maintainability. </details></details></td><td align=center>High </td></tr><tr><td rowspan=3>General</td> <td> <details><summary>Skip all-zero IDs</summary> ___ **Update <code>bytes_to_hex</code> to return <code>None</code> if the input byte slice contains only zeros, <br>preventing invalid all-zero trace or span IDs from being generated.** [cmd/consumers/zen/src/otel_logs.rs [146-156]](https://github.com/carverauto/serviceradar/pull/2354/files#diff-4542ace311c2de0c5f9389169b9b4b3429e972dde67ea1c6efb9dff04ec1c725R146-R156) ```diff fn bytes_to_hex(bytes: &[u8]) -> Option<String> { - if bytes.is_empty() { + if bytes.is_empty() || bytes.iter().all(|&b| b == 0) { return None; } let mut out = String::with_capacity(bytes.len() * 2); for byte in bytes { let _ = write!(out, "{:02x}", byte); } Some(out) } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that all-zero trace/span IDs are invalid and should be treated as absent, improving data quality by preventing meaningless IDs from being processed. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Support numeric & bool attrs</summary> ___ **Modify <code>get_attr_string</code> to convert numeric and boolean attribute values to <br>strings, ensuring they are not dropped if they are not already string types.** [cmd/consumers/zen/src/otel_logs.rs [135-144]](https://github.com/carverauto/serviceradar/pull/2354/files#diff-4542ace311c2de0c5f9389169b9b4b3429e972dde67ea1c6efb9dff04ec1c725R135-R144) ```diff fn get_attr_string(attrs: &Value, key: &str) -> String { match attrs { - Value::Object(map) => map - .get(key) - .and_then(|value| value.as_str()) - .unwrap_or_default() - .to_string(), + Value::Object(map) => { + map.get(key).map(|v| { + if let Some(s) = v.as_str() { + s.to_string() + } else { + v.to_string() + } + }).unwrap_or_default() + } _ => String::new(), } } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This change makes the `get_attr_string` function more robust by handling non-string attribute values, preventing potential data loss when attributes like version numbers are numeric. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Correct attribute map merge priority</summary> ___ **Refactor <code>mergeAttributeMaps</code> to be more idiomatic by iterating over the <code>extra</code> map <br>first, then the <code>base</code> map, which simplifies the logic by removing an existence <br>check.** [pkg/consumers/db-event-writer/json_logs.go [489-507]](https://github.com/carverauto/serviceradar/pull/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42R489-R507) ```diff func mergeAttributeMaps(base, extra map[string]interface{}) map[string]interface{} { if len(base) == 0 && len(extra) == 0 { return nil } merged := make(map[string]interface{}) - for key, value := range base { + for key, value := range extra { merged[key] = value } - for key, value := range extra { - if _, exists := merged[key]; exists { - continue - } + for key, value := range base { merged[key] = value } return merged } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 2</summary> __ Why: The suggestion correctly identifies the desired merge priority but misinterprets the existing code, which already implements it; the proposed change is a minor refactoring for readability with no functional difference. </details></details></td><td align=center>Low </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Use empty JSON object</summary> ___ **In <code>encodeAttributes</code>, return an empty JSON object (<code>{}</code>) instead of an empty string <br>when the attribute map is empty or marshaling fails to ensure consumers always <br>receive valid JSON.** [pkg/consumers/db-event-writer/json_logs.go [476-487]](https://github.com/carverauto/serviceradar/pull/2354/files#diff-c16044ec608122e3cb559e6e21779d3b25753581b58ff01da701cec5d19a2d42R476-R487) ```diff func encodeAttributes(attributes map[string]interface{}) string { if len(attributes) == 0 { - return "" + return "{}" } encoded, err := json.Marshal(attributes) if err != nil { - return "" + return "{}" } return string(encoded) } ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: Returning an empty JSON object (`{}`) instead of an empty string for attribute fields ensures type consistency for downstream consumers, which is a good practice for data integrity. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>