Skip to content

feat: merge develop into main#19

Merged
qnen merged 47 commits into
mainfrom
develop
Jun 16, 2026
Merged

feat: merge develop into main#19
qnen merged 47 commits into
mainfrom
develop

Conversation

@qnen

@qnen qnen commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator
Lerian

Lib Observability


Description

Type of Change

  • feat: New feature or capability
  • fix: Bug fix
  • perf: Performance improvement
  • refactor: Internal restructuring with no behavior change
  • docs: Documentation only (README, docs/, inline comments)
  • style: Formatting, whitespace, naming (no logic change)
  • test: Adding or updating tests
  • ci: CI pipeline or workflow changes
  • build: Build system, Go module dependencies
  • chore: Maintenance, config, tooling
  • revert: Reverts a previous commit
  • BREAKING CHANGE: Consumers must update their integration

Breaking Changes

None.

Testing

  • make test passes
  • make test-int passes if integration paths are exercised
  • make lint passes
  • Coverage threshold respected (see Go Combined Analysis)

Test evidence / Actions run:

Architectural Checklist

  • No panic() in production paths — uses assert helpers or wrapped errors
  • Timestamps use time.Now().UTC()
  • Errors wrapped with %w
  • Public API additions documented (godoc + README if user-facing)
  • No leaking of context or goroutines (instrumentation must be safe under load)
  • Backwards-compatible by default; breaking changes flagged above
  • No lib-commons import (circular — see CLAUDE.md)

Related Issues

Closes #

lerian-studio and others added 30 commits May 16, 2026 00:08
Upgrade the project's Go version from 1.25.10 to 1.26.3 to keep up with
the latest language improvements and security patches. Update go.mod, CI
workflows, and documentation to reflect the new version.

Update multiple dependencies including grpc, fasthttp, and x/sys pkgs.
Bump tooling versions in Makefile (golangci-lint v2.12.2, gotestsum
v1.13.0, and gosec v2.26.1) to leverage new features and bug fixes.

Improve the Makefile by replacing brittle grep filters with -tags=unit
for test discovery. Add a linker workaround for macOS to suppress noisy
LC_DYSYMTAB warnings during test execution. Enhance the lint target to
conditionally run perfsprint for performance checks.

Refactor test files to use httptest.NewRequest instead of
http.NewRequest to simplify test setup and remove unnecessary error
checks, and group type definitions in metrics tests for readability.
feat(middleware): skip /readyz by default and add WithExcludedRoutes
Implement the OpenTelemetry HTTP semantic-convention server metric
http.server.request.duration as a Float64 seconds histogram on the Fiber
WithTelemetry middleware. The instrument is created lazily at handler-
construction time from the configured MeterProvider and is gated on the
presence of a non-nil MetricsFactory so callers without a metrics pipeline
are unaffected.

Recording happens for every non-excluded request alongside the existing
span behavior, attaching low-cardinality OTel attributes derived from the
captured Fiber context: http.request.method (heap-copied before c.Next to
survive fasthttp buffer recycling), http.route from c.Route().Path (route
template, never the raw path), http.response.status_code, and error.type
when the handler returns an error or the response is 5xx. Nil telemetry,
nil MeterProvider, nil MetricsFactory, excluded routes, and instrument
creation errors all silently skip the metric without affecting the request
path. Existing span attributes and EndTracingSpans behavior are unchanged.

Originally tracked as LerianStudio/lib-commons#462; the middleware now
lives in lib-observability after the v5 split, so the change lands here.

Tests cover success (200), client error (404), server status (503),
handler-returned error, excluded route, nil telemetry, nil MeterProvider,
and nil MetricsFactory paths using a real OTel SDK ManualReader.

Requested-by: @gauchito91
X-Lerian-Ref: 0x1
…metrics test

TestWithTelemetry_RecordsDurationOnSuccess used http.NewRequest with a
relative path, which produces a *http.Request with no Host header. Fiber's
App.Test serializes this via httputil.DumpRequest and feeds the bytes to
fasthttp, which rejects HTTP/1.1 requests missing a Host header with
"missing required Host header in request" — surfacing as a test failure
on the CI runner while passing locally due to scheduling differences.

httptest.NewRequest automatically sets Host="example.com", matching the
pattern already used by every other test in this file and eliminating the
flake.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per OpenTelemetry HTTP semantic conventions, http.request.method MUST be
normalized to a known set (GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS,
TRACE, PATCH); unknown verbs MUST be reported as "_OTHER" with the original
preserved in http.request.method_original (span attribute only — never on
the metric, to keep cardinality bounded).

Fiber v2's default RequestMethods already filters unknown verbs with 400
before middleware runs, so this is primarily a spec-conformance fix for
callers that customize fiber.Config{RequestMethods: ...} (e.g., WebDAV
verbs like PROPFIND).

Refs: https://opentelemetry.io/docs/specs/semconv/http/http-metrics/

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion

Replaces the previous "Go type name" classification with a status-driven
label per the OpenTelemetry semconv guidance that error.type SHOULD be
predictable and low-cardinality:

- Metric: error.type = string(statusCode) when statusCode >= 500; omitted
  otherwise. A 503 raised via fiber.NewError(503) and a 503 written via
  c.SendStatus(503) now produce the SAME time series, so alert rules of
  the form error_type=~"5.." aggregate reliably.
- Span: same numeric error.type plus a separate error.type_original
  attribute carrying the originating Go type name when a handler error
  is present (high cardinality acceptable on spans, useful for debugging).

This supersedes the 4xx behavior added in b57e0cf (error.type set to
"fiber.Error" for any fiber.NewError, regardless of status class), which
re-introduces inconsistency between fiber.NewError and SendStatus and
unbounded cardinality across application error types. The originating
type identity is preserved on the span via error.type_original.

Reuses the existing httpStatusCode helper for status reconciliation; no
new helper introduced.

Refs: https://opentelemetry.io/docs/specs/semconv/general/attributes/#error-type

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add the 0.075, 0.75, 7.5 boundaries to match the current OpenTelemetry
HTTP semantic conventions advisory layout. Improves resolution near
common SLO thresholds (e.g., p99 at 100ms benefits from the 0.075 bucket).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fiber v2's default catch-all for unmatched paths exposes Path="/" via
c.Route(), causing scanner/404 traffic to be conflated with the actual
root handler in dashboards. Detect this case (effective status==404 &&
route=="/" && request path != "/") and omit http.route entirely from
both the span and the metric, matching OTel guidance that the attribute
SHOULD be absent when no route matched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bootstrap requires MetricsFactory != nil even though the histogram is
created directly from MeterProvider. Document the rationale so future
refactors do not remove the gate and accidentally invert the contract
asserted by TestWithTelemetry_NilMetricsFactoryDoesNotRecord.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ytes

Unbounded User-Agent strings inflate trace storage and indexing cost in
Tempo. Cap at 256 bytes — sufficient for canonical client/library/version
identifiers and matching common observability backend recommendations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves conflict in middleware/helpers.go by keeping the new helpers
introduced on this branch (knownHTTPMethods, normalizeHTTPMethod,
routeAttribute, truncateUserAgent, errorTypeOriginal, maxUserAgentAttrLen)
alongside the updated isRouteExcludedFromList docstring brought in from
develop (originating in commit f43c51d "skip /readyz by default").

Brings develop's Go toolchain bump (1.25.10 -> 1.26.3) and minor dep
updates (grpc 1.81.0 -> 1.81.1, brotli 1.2.0 -> 1.2.1). Full unit
suite + -race tests pass under Go 1.26.3.

Also addresses two CodeRabbit review comments on helpers.go:

  - truncateUserAgent now truncates at a rune boundary using a
    last-fit tracker, ensuring the returned string is always valid
    UTF-8 even for multi-byte user-agents and never exceeds the
    256-byte cap.
  - errorTypeOriginal docstring corrected to state that ALL pointer
    levels are unwrapped (matches the existing loop implementation),
    not "a single pointer level".

Adds TestWithTelemetry_TruncatesUserAgentAtRuneBoundary as a
regression guard for the UTF-8 fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new error.type, error.type_original, http.request.method_original,
and http.route conditional attributes added by this branch pushed
WithTelemetry's cyclomatic complexity from 14 to 20, above the repo's
gocyclo budget of 16 (.golangci.yml).

Extracts the post-c.Next() attribute application + status finalization
into a private helper applyTelemetrySpanAttributes(span, c, statusCode,
telemetryRequestAttrs{...}). Behavior is byte-for-byte identical; the
test suite (including the new metric/span assertions added on this
branch) passes unchanged.

golangci-lint v2.12.2 now reports 0 issues. Full unit suite + -race
remain green under Go 1.26.3.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rics

feat(middleware): record HTTP server request duration metrics
Bumps the go-dependencies group with 12 updates:

| Package | From | To |
| --- | --- | --- |
| [go.opentelemetry.io/contrib/bridges/otelzap](https://github.com/open-telemetry/opentelemetry-go-contrib) | `0.18.0` | `0.19.0` |
| [go.opentelemetry.io/otel](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc](https://github.com/open-telemetry/opentelemetry-go) | `0.19.0` | `0.20.0` |
| [go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/exporters/otlp/otlptrace](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/log](https://github.com/open-telemetry/opentelemetry-go) | `0.19.0` | `0.20.0` |
| [go.opentelemetry.io/otel/metric](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/sdk](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/sdk/log](https://github.com/open-telemetry/opentelemetry-go) | `0.19.0` | `0.20.0` |
| [go.opentelemetry.io/otel/sdk/metric](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |
| [go.opentelemetry.io/otel/trace](https://github.com/open-telemetry/opentelemetry-go) | `1.43.0` | `1.44.0` |


Updates `go.opentelemetry.io/contrib/bridges/otelzap` from 0.18.0 to 0.19.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go-contrib/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go-contrib@v0.18.0...v0.19.0)

Updates `go.opentelemetry.io/otel` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` from 0.19.0 to 0.20.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v0.19.0...v0.20.0)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlptrace` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/log` from 0.19.0 to 0.20.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v0.19.0...v0.20.0)

Updates `go.opentelemetry.io/otel/metric` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/sdk` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/sdk/log` from 0.19.0 to 0.20.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v0.19.0...v0.20.0)

Updates `go.opentelemetry.io/otel/sdk/metric` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

Updates `go.opentelemetry.io/otel/trace` from 1.43.0 to 1.44.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/contrib/bridges/otelzap
  dependency-version: 0.19.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc
  dependency-version: 0.20.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlptrace
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/log
  dependency-version: 0.20.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/metric
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/sdk
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/sdk/log
  dependency-version: 0.20.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/sdk/metric
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
- dependency-name: go.opentelemetry.io/otel/trace
  dependency-version: 1.44.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: go-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>
Drops the metrics-builder context channel introduced in d9dbdd2 in favor
of an explicit opt-in helper (middleware.RequestAttributes). Logs and
traces remain automatic through the AttrBag carrier and the existing
span processor; metric labels are now caller-controlled to keep
cardinality decisions out of the library.

- ContextWithSpanAttributes: last-wins dedup by key, allowing downstream
  layers (e.g. JWT auth) to override a header-supplied tenant.id without
  inflating the bag or producing stale reads.
- middleware/tenant.go: canonical-only header (X-Tenant-Id) / metadata
  (tenant-id) with 128-byte cap to bound telemetry cardinality.
- middleware/logging.go: HTTP/gRPC parity, both resolve through the
  AttrBag instead of re-reading the request.
- middleware/telemetry.go: idempotent tenant write via dedup, no
  duplicate AttrBag entries when both middlewares run.
- middleware/request_attrs.go: explicit helper for metric callers.
- README: tenant.id propagation section with trust boundary guidance.
- Tests: dedup ordering, override semantics, helpers, edge cases.

Reverts metrics/builders.go and middleware/helpers.go to develop.

X-Lerian-Ref: 0x1
…-label-develop

feat(log): add tenant.id shared observability attribute
Adds tenant.id as a label on the http.server.request.duration histogram
when the request carries the canonical X-Tenant-Id header. The label is
omitted when no tenant was supplied so non-tenant traffic (probes,
internal callers) does not split the time series with an empty value.

Enables per-tenant filtering in dashboards and alerts directly on the
built-in HTTP server metric, removing the need to derive equivalent
series from the spanmetrics connector (calls_total{tenant_id}).

Cardinality is bounded by the 128-byte tenant cap already enforced by
ResolveTenantIDFromHTTP - an attacker that floods random oversized
values into X-Tenant-Id cannot inflate the Prometheus series set.

- middleware/telemetry.go: read tenant from AttrBag in
  recordHTTPServerDuration, append as attribute when non-empty
- middleware/telemetry_metrics_test.go: cover three cases - present,
  absent, oversized-and-dropped
- README.md: document tenant.id as a metric label on the built-in HTTP
  histogram

X-Lerian-Ref: 0x1
Addresses two CodeRabbit findings on #16:

1. tenantIDFromAttrBag now funnels the bag value through sanitizeTenantID
   before returning it. Callers can populate tenant.id directly via
   observability.ContextWithSpanAttributes (the documented escape hatch
   for handlers that resolve a real tenant from a JWT). Without
   re-sanitization an oversized or control-byte-laden value injected
   this way would bypass the cap enforced at HTTP/gRPC ingestion and
   inflate log fields and metric label cardinality. Defense in depth at
   the read site benefits every consumer (logging fields and the HTTP
   server duration metric) without each having to repeat the check.

2. TestWithTelemetry_RecordsDurationDropsOversizedTenantID now derives
   the oversized header from constant.MaxTenantIDLen+1 instead of a
   hardcoded 129, so the test tracks the production cap if it ever
   changes.

Two new tests guard the new behavior at the helper level:
TestTenantIDFromAttrBag_DropsOversizedFromContext and
TestTenantIDFromAttrBag_StripsControlBytesFromContext.

X-Lerian-Ref: 0x1
…evelop

feat(middleware): add tenant.id to http.server.request.duration metric
A regression in normalizeEndpoint() dropped the lib-commons default that
treats a scheme-less endpoint (host:port) as insecure. Without it,
InsecureExporter stayed false, so the otlp{trace,metric,log}grpc exporters
omitted WithInsecure() and attempted a TLS handshake against the collector's
plaintext port, failing silently in the background (services start fine but
no telemetry reaches the backend).

This is the default in our charts: OTEL_EXPORTER_OTLP_ENDPOINT=$(HOST_IP):4317.

Restore the no-scheme insecure inference and reinstate normalizeEndpointEnvVars()
so the OTEL SDK envconfig does not choke on bare host:port values. The existing
production guard (isStrictEnvironment + ALLOW_INSECURE_OTEL) still blocks
insecure exporters in strict environments.

X-Lerian-Ref: 0x1
normalizeEndpointEnvVars silently discarded os.Setenv errors. A failure
there means the OTEL SDK would still parse the bare host:port value and emit
the very errors this normalization avoids, so surface it via the configured
logger instead. Addresses review feedback on PR #17.

X-Lerian-Ref: 0x1
fix(tracing): infer insecure exporter for scheme-less OTLP endpoint
@lerian-studio

lerian-studio commented Jun 5, 2026

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

@lerian-studio

lerian-studio commented Jun 5, 2026

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tracing/otel.go`:
- Around line 154-157: The no-scheme branch sets cfg.InsecureExporter = true but
doesn't propagate the trimmed endpoint, so inputs like " otel-collector:4317 "
keep whitespace; update that branch to assign the trimmed ep to
cfg.CollectorExporterEndpoint (the same field used elsewhere) after trimming so
the exporter sees the sanitized endpoint while still marking
cfg.InsecureExporter = true.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1ecf0f21-9e78-466b-b110-3930454f24e1

📥 Commits

Reviewing files that changed from the base of the PR and between 5f2f704 and d2da99a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (22)
  • .github/workflows/go-combined-analysis.yml
  • CLAUDE.md
  • Makefile
  • README.md
  • constants/tenant.go
  • go.mod
  • metrics/metrics_test.go
  • middleware/helpers.go
  • middleware/helpers_test.go
  • middleware/logging.go
  • middleware/logging_test.go
  • middleware/request_attrs.go
  • middleware/telemetry.go
  • middleware/telemetry_metrics_test.go
  • middleware/telemetry_test.go
  • middleware/tenant.go
  • middleware/tenant_test.go
  • observability.go
  • observability_test.go
  • tracing/otel.go
  • tracing/otel_test.go
  • tracing/processor.go

Comment thread middleware/logging.go
Comment thread tracing/otel.go
gandalf-at-lerian and others added 4 commits June 8, 2026 18:02
baggageBaseAttributes wrote the raw baggage tenant.id straight into
span attributes, bypassing the control-byte stripping and length cap
that the logging path applies via sanitizeTenantID. Oversized or dirty
baggage could leak into traces and cause cardinality drift between
logs and spans. Apply the same guards (strip CR/LF/NUL, trim, drop
values over MaxTenantIDLen) via a local helper; it is duplicated
rather than imported because middleware depends on tracing.

Requested-by: @qnen
Resolves wsl_v5 'missing whitespace above this line' lint error
blocking CI on PR #20.

Requested-by: @qnen
…l-baggage

feat: read tenant.id from OTel baggage for spans and logs
@qnen
qnen temporarily deployed to create_release June 9, 2026 13:51 — with GitHub Actions Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tracing/processor.go`:
- Around line 86-95: sanitizeTenantFromBaggage currently returns the literal
"null" from baggage which diverges from middleware normalization that drops
"null" values; update sanitizeTenantFromBaggage (the function shown) to treat
the string "null" (at least case-insensitive) as empty so it returns "" for
those values, preserving the same normalization behavior as the middleware and
preventing tenant.id="null" in spans while logs have an empty tenant.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 37f388dd-9841-4a6d-a988-91b20cee10aa

📥 Commits

Reviewing files that changed from the base of the PR and between d2da99a and 0f7ced0.

📒 Files selected for processing (5)
  • middleware/logging.go
  • middleware/tenant.go
  • middleware/tenant_test.go
  • tracing/otel_test.go
  • tracing/processor.go

Comment thread tracing/processor.go
gandalf-at-lerian and others added 4 commits June 10, 2026 16:57
…ibutes

SetHandlerSpanAttributes set tenant.id/context.id only on the child span,
leaving the request AttrBag empty. WithTelemetry reads tenant.id back from
the AttrBag (tenantIDFromAttrBag) after c.Next() to label
http.server.request.duration and seed the root span, so the metric/root span
lost tenant.id.

Refactor the helper to also accept and return context.Context, funneling the
same attributes through observability.ContextWithSpanAttributes so the AttrBag
(and thus the duration metric + root span) carry tenant.id.

Add context_span_test.go covering tenant.id/context.id propagation, nil
contextID omission, nil span, and nil context.
SetTenantSpanAttribute, SetExceptionSpanAttributes, and
SetDisputeSpanAttributes were still using the "tenant.id" string literal
directly while SetHandlerSpanAttributes (fixed in this branch) already
uses constant.AttrKeyTenantID. Normalise all four helpers to the shared
constant so a rename in constants/tenant.go propagates everywhere.

Requested-by: @qnen
…nstant

Add AttrKeyContextID = "context.id" to constants/opentelemetry.go alongside
the existing AttrKeyTenantID pattern. Remove the local contextIDAttrKey
from middleware/context_span.go and update all references (production code
and tests) to use the shared constant.

Requested-by: @qnen
…tes-context-propagation

fix(middleware): propagate tenant.id to AttrBag in SetHandlerSpanAttributes
gandalf-at-lerian and others added 2 commits June 10, 2026 18:18
…request.duration metric

The duration metric resolved tenant.id from the request AttrBag only
(tenantIDFromAttrBag), while the logging middleware and span processor
also fall back to OTel baggage. tenant.id propagated cross-service via
baggage (PR #20) therefore reached spans and logs but produced an empty
tenant_id label on the metric for downstream traffic.

Route the metric through the shared resolveTenantIDForTelemetry helper
(renamed from resolveTenantIDForLogging) so logs, spans, and metrics use
the same AttrBag->baggage precedence and the tenant_id label stays
consistent across all three pipelines. Adds a regression test covering a
baggage-only tenant with no local X-Tenant-Id header.

Relates to PR #21 (tracing/AttrBag propagation fix).

Requested-by: @gauchito91
fix(middleware): include baggage-propagated tenant.id in HTTP duration metric
gandalf-at-lerian and others added 3 commits June 11, 2026 17:29
Logs request duration in milliseconds (from RequestInfo.Duration) as a
structured field alongside the existing http_status_code/method/path
fields, enabling latency queries and alerting in observability backends.

Requested-by: @qnen
…fields

fix: add structured HTTP fields to WithHTTPLogging
@qnen
qnen temporarily deployed to create_release June 11, 2026 21:24 — with GitHub Actions Inactive
…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
…eendpoint-sanitize-tenant

fix(tracing): persist trimmed endpoint and treat null/nil baggage as empty
@qnen
qnen temporarily deployed to create_release June 16, 2026 14:24 — with GitHub Actions Inactive
@qnen
qnen merged commit 1e13f4f into main Jun 16, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous integration pipelines constants Shared constants core Top-level public API surface deps Go module dependencies (usually opened by Dependabot) docs Documentation, llms.txt and markdown content metrics Metrics instrumentation middleware HTTP/gRPC observability middleware scripts Build and tooling scripts size/XL PR changes ≥ 1000 lines — consider splitting tests Unit, integration and end-to-end tests tracing Distributed tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants