Skip to content

fix(tracing): persist trimmed endpoint and treat null/nil baggage as empty#25

Merged
qnen merged 1 commit into
developfrom
fix/coderabbit-otel-normalizeendpoint-sanitize-tenant
Jun 16, 2026
Merged

fix(tracing): persist trimmed endpoint and treat null/nil baggage as empty#25
qnen merged 1 commit into
developfrom
fix/coderabbit-otel-normalizeendpoint-sanitize-tenant

Conversation

@gandalf-at-lerian

Copy link
Copy Markdown
Contributor

Summary

Applies two approved CodeRabbit review fixes from PR #19.

Fix #1tracing/otel.go · normalizeEndpoint

In the default (no-scheme) branch, the trimmed endpoint value (ep) was only held in a local variable; cfg.CollectorExporterEndpoint retained the original value with surrounding whitespace. Now persists the trimmed value back into the config.

Fix #2tracing/processor.go · sanitizeTenantFromBaggage

Treats literal "null" / "nil" (case-insensitive, via strings.EqualFold) as empty, mirroring the middleware tenant-sanitization path and preventing JSON-null serialization artifacts from leaking into span tenant.id.

Verification

  • go build ./... — OK
  • go test -tags unit ./... — all packages pass

References

Addresses CodeRabbit review comments on PR #19.

…empty

- normalizeEndpoint: persist trimmed value back to cfg.CollectorExporterEndpoint
  in the default (no-scheme) branch so surrounding whitespace is dropped.
- sanitizeTenantFromBaggage: treat literal "null"/"nil" (case-insensitive)
  as empty, mirroring the middleware tenant-sanitization path.

Addresses CodeRabbit review comments on PR #19.

Requested-by: @qnen
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 67d28a2e-0418-4e77-8500-3a34db630262

📥 Commits

Reviewing files that changed from the base of the PR and between 2428e19 and 44797f6.

📒 Files selected for processing (2)
  • tracing/otel.go
  • tracing/processor.go

Walkthrough

Two defensive fixes in the tracing package: normalizeEndpoint in otel.go now writes the whitespace-trimmed endpoint back to cfg.CollectorExporterEndpoint for the no-scheme case, and sanitizeTenantFromBaggage in processor.go adds a case-insensitive check that treats literal "null" or "nil" baggage values as empty strings.

Changes

Tracing Input Sanitization

Layer / File(s) Summary
Persist trimmed endpoint back to config
tracing/otel.go
In the no-scheme path of normalizeEndpoint, the locally trimmed ep value is now assigned back to cfg.CollectorExporterEndpoint, ensuring the config struct reflects the whitespace-stripped value.
Normalize null/nil baggage values to empty string
tracing/processor.go
sanitizeTenantFromBaggage adds a guard before the existing length checks: if the baggage value is the literal string "null" or "nil" (case-insensitive), it returns "" immediately, preventing JSON null serialization artifacts from propagating as a tenant.id span attribute.

Comment @coderabbitai help to get the list of available commands and usage tips.

@lerian-studio

Copy link
Copy Markdown
Contributor

🔒 Security Scan Results — lib-observability

✅ PR Mergeable — no blocking findings

Stage Status Blocking?
Filesystem Scan ✅ Clean
Docker Image Scan ➖ Skipped
Docker Hub Health Score ➖ Skipped
Pre-release Version Check ✅ Clean

Trivy

Filesystem Scan

✅ No vulnerabilities or secrets found.


Pre-release Version Check

✅ No unstable version pins found.


🔍 View full scan logs

@qnen
qnen marked this pull request as ready for review June 16, 2026 14:17
@lerian-studio lerian-studio added size/XS PR changes < 50 lines tracing Distributed tracing labels Jun 16, 2026
@lerian-studio

Copy link
Copy Markdown
Contributor

🔍 PR Validation Summary

✅ PR Mergeable — no blocking failures

Check Status Blocking
Source Branch ✅ success yes
PR Title ✅ success yes
PR Description ✅ success yes
PR Size ✅ success no
Auto Labels ✅ success no
PR Metadata ✅ success no

🔍 View workflow run

@lerian-studio

Copy link
Copy Markdown
Contributor

📊 Unit Test Coverage Report: lib-observability

Metric Value
Overall Coverage 85.7% ✅ PASS
Threshold 80%

Coverage by Package

Package Coverage
github.com/LerianStudio/lib-observability/assert 97.9%
github.com/LerianStudio/lib-observability/constants 83.3%
github.com/LerianStudio/lib-observability/log 94.9%
github.com/LerianStudio/lib-observability/metrics 91.4%
github.com/LerianStudio/lib-observability/middleware 76.9%
github.com/LerianStudio/lib-observability/redaction 95.8%
github.com/LerianStudio/lib-observability/runtime 80.4%
github.com/LerianStudio/lib-observability/tracing 84.8%
github.com/LerianStudio/lib-observability/zap 96.0%
github.com/LerianStudio/lib-observability 91.5%

Generated by Go PR Analysis workflow

@qnen
qnen merged commit 307fdac into develop Jun 16, 2026
26 checks passed
@github-actions
github-actions Bot deleted the fix/coderabbit-otel-normalizeendpoint-sanitize-tenant branch June 16, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS PR changes < 50 lines tracing Distributed tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants