diff --git a/.github/workflows/go-combined-analysis.yml b/.github/workflows/go-combined-analysis.yml index 044c479..4e8e916 100644 --- a/.github/workflows/go-combined-analysis.yml +++ b/.github/workflows/go-combined-analysis.yml @@ -24,8 +24,8 @@ jobs: with: runner_type: "blacksmith-4vcpu-ubuntu-2404" app_name_prefix: "lib-observability" - go_version: "1.25.10" - golangci_lint_version: "v2.11.2" + go_version: "1.26.3" + golangci_lint_version: "v2.12.2" golangci_lint_args: "--timeout=5m" coverage_threshold: 80 fail_on_coverage_threshold: true diff --git a/CLAUDE.md b/CLAUDE.md index 9a40378..246fedc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,7 +3,7 @@ ## What this repo is `github.com/LerianStudio/lib-observability` — standalone Go library for observability/telemetry, -extracted from `lib-commons` (`../lib-commons`). Go 1.25.10. +extracted from `lib-commons` (`../lib-commons`). Go 1.26.3. ## Architecture decisions (non-negotiable) diff --git a/Makefile b/Makefile index c73b81e..bcd47c3 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,10 @@ endef # ------------------------------------------------------ # Integration test filter +# RUN: specific test name pattern (e.g., TestIntegration_FeatureName) +# PKG: specific package to test (e.g., ./...) +# Usage: make test-integration RUN=TestIntegration_FeatureName +# make test-integration PKG=./... RUN ?= PKG ?= @@ -32,6 +36,11 @@ else endif # Low-resource mode for limited machines (sets -p=1 -parallel=1, disables -race) +# Usage: make test LOW_RESOURCE=1 +# make test-unit LOW_RESOURCE=1 +# make test-integration LOW_RESOURCE=1 +# make coverage-unit LOW_RESOURCE=1 +# make coverage-integration LOW_RESOURCE=1 LOW_RESOURCE ?= 0 # Computed flags for low-resource mode @@ -45,9 +54,12 @@ else LOW_RES_RACE_FLAG := -race endif -# macOS ld64 workaround +# macOS ld64 workaround: newer ld emits noisy LC_DYSYMTAB warnings when linking test binaries with -race. +# If available, prefer Apple's classic linker to silence them. UNAME_S := $(shell uname -s) ifeq ($(UNAME_S),Darwin) + # Prefer classic mode to suppress LC_DYSYMTAB warnings on macOS. + # Set DISABLE_OSX_LINKER_WORKAROUND=1 to disable this behavior. ifneq ($(DISABLE_OSX_LINKER_WORKAROUND),1) GO_TEST_LDFLAGS := -ldflags="-linkmode=external -extldflags=-ld_classic" else @@ -61,18 +73,22 @@ endif # Test tooling configuration # ------------------------------------------------------ -GOTESTSUM_VERSION ?= v1.12.0 -GOSEC_VERSION ?= v2.22.4 -GOLANGCI_LINT_VERSION ?= v2.1.6 +# Pinned tool versions for reproducibility (update as needed) +GOTESTSUM_VERSION ?= v1.13.0 +GOSEC_VERSION ?= v2.26.1 +GOLANGCI_LINT_VERSION ?= v2.12.2 TEST_REPORTS_DIR ?= ./reports GOTESTSUM = $(shell command -v gotestsum 2>/dev/null) -GOSEC = $(shell command -v gosec 2>/dev/null || \ - GOBIN="$$(go env GOBIN)"; \ - if [ -n "$$GOBIN" ]; then \ - printf "%s/gosec" "$$GOBIN"; \ +GOSEC = $(shell if command -v gosec >/dev/null 2>&1; then \ + command -v gosec; \ else \ - printf "%s/bin/gosec" "$$(go env GOPATH)"; \ + GOBIN="$$(go env GOBIN)"; \ + if [ -n "$$GOBIN" ]; then \ + printf "%s/gosec" "$$GOBIN"; \ + else \ + printf "%s/bin/gosec" "$$(go env GOPATH)"; \ + fi; \ fi) RETRY_ON_FAIL ?= 0 @@ -216,7 +232,7 @@ test-unit: $(call print_title,Running Go unit tests) $(call check_command,go,"Install Go from https://golang.org/doc/install") @set -e; mkdir -p $(TEST_REPORTS_DIR); \ - pkgs=$$(go list ./... | grep -v '/tests'); \ + pkgs=$$(go list -tags=unit ./...); \ if [ -z "$$pkgs" ]; then \ echo "No unit test packages found"; \ else \ @@ -318,9 +334,9 @@ coverage-unit: @set -e; mkdir -p $(TEST_REPORTS_DIR); \ if [ -n "$(PKG)" ]; then \ echo "Using specified package: $(PKG)"; \ - pkgs=$$(go list $(PKG) 2>/dev/null | grep -v '/tests' | tr '\n' ' '); \ + pkgs=$$(go list -tags=unit $(PKG) 2>/dev/null | tr '\n' ' '); \ else \ - pkgs=$$(go list ./... | grep -v '/tests'); \ + pkgs=$$(go list -tags=unit ./...); \ fi; \ if [ -z "$$pkgs" ]; then \ echo "No unit test packages found"; \ @@ -440,8 +456,29 @@ coverage: lint: $(call print_title,Running linters on all packages (read-only)) $(call check_command,golangci-lint,"go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)") - @golangci-lint run ./... - @echo "$(GREEN)$(BOLD)[ok]$(NC) Lint checks passed successfully$(GREEN) ✔️$(NC)" + @out=$$(golangci-lint run ./... 2>&1); \ + out_err=$$?; \ + if command -v perfsprint >/dev/null 2>&1; then \ + perf_out=$$(perfsprint ./... 2>&1); \ + perf_err=$$?; \ + else \ + perf_out=""; \ + perf_err=0; \ + echo "Note: perfsprint not installed, skipping performance checks (go install github.com/catenacyber/perfsprint@latest)"; \ + fi; \ + echo "$$out"; \ + if [ -n "$$perf_out" ]; then echo "$$perf_out"; fi; \ + if [ $$out_err -ne 0 ]; then \ + printf "\n%s\n" "$(BOLD)$(RED)An error has occurred during the lint process:$(NC)"; \ + printf "%s\n" "$$out"; \ + exit 1; \ + fi; \ + if [ $$perf_err -ne 0 ]; then \ + printf "\n%s\n" "$(BOLD)$(RED)An error has occurred during the performance check:$(NC)"; \ + printf "%s\n" "$$perf_out"; \ + exit 1; \ + fi + @echo "$(GREEN)$(BOLD)[ok]$(NC) Lint and performance checks passed successfully$(GREEN) ✔️$(NC)" .PHONY: lint-fix lint-fix: diff --git a/README.md b/README.md index 7d62742..b4fd44c 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,44 @@ A configurable `Redactor` with rule-based field processing supporting mask, hash - **Redaction-first** — sensitive fields are masked in spans, logs, and attributes by default - **Interface-driven** — `Logger`, `MetricsFactory`, `ErrorReporter`, and `DLQMetrics` are all interface-bound for testability +## Tenant ID propagation + +The HTTP and gRPC middleware automatically read a tenant identifier from the request and propagate it through telemetry as the `tenant.id` attribute / log field. + +### Wire protocol + +- **HTTP:** canonical header `X-Tenant-Id`. No aliases. +- **gRPC:** canonical metadata key `tenant-id`. No aliases. + +Values are normalized (trimmed, control chars stripped) and dropped silently when empty or longer than 128 bytes to bound telemetry cardinality. + +### Where it shows up + +| Signal | How it gets there | Action required by caller | +|---|---|---| +| Logs | `WithHTTPLogging` / `WithGrpcLogging` add `tenant.id` as a structured field. | None | +| Traces | The `AttrBagSpanProcessor` (registered by default in `tracing.NewTelemetry`) copies the request attribute bag onto every span at start. | None | +| `http.server.request.duration` (built-in metric) | `WithTelemetry` adds `tenant.id` to the histogram when present in the request bag. Label is omitted when no tenant was supplied so non-tenant traffic does not split the time series. | None | +| Custom application metrics | Not automatic. Metric labels are a high-impact cardinality decision left to the caller. | Use `middleware.RequestAttributes(ctx)` to opt in per metric. | + +Example for custom metrics: + +```go +import "github.com/LerianStudio/lib-observability/middleware" + +counter, _ := factory.Counter("orders.created") +_ = counter. + WithAttributes(middleware.RequestAttributes(ctx)...). + Add(ctx, 1) +``` + +### Trust boundary + +The header is **client-controlled**. The middleware treats it as an observability hint, not an authenticated identifier: + +- Run authentication (JWT, mTLS, etc.) **before** these middlewares apply effects you care about. +- If your auth layer resolves the real tenant from a signed credential, call `observability.ContextWithSpanAttributes(ctx, attribute.String("tenant.id", real))` from the handler. The attribute bag is deduplicated by key with last-wins semantics, so the override surfaces in subsequent logs/traces and replaces the header-supplied value. + ## Relationship to lib-commons This library was extracted from `lib-commons` to decouple observability infrastructure from service primitives and data connectors. Services that previously imported `lib-commons` for telemetry can migrate to `lib-observability` for a lighter dependency graph. `lib-commons` will depend on `lib-observability` for its own instrumentation needs (database spans, streaming metrics, middleware telemetry). diff --git a/constants/opentelemetry.go b/constants/opentelemetry.go index 3c33d3c..95e9668 100644 --- a/constants/opentelemetry.go +++ b/constants/opentelemetry.go @@ -17,6 +17,13 @@ const ( AttrPrefixPanic = "panic." ) +// Telemetry attribute keys for request identity. +const ( + // AttrKeyContextID is the OpenTelemetry attribute / log field key used + // everywhere context.id is emitted (logs, traces, metrics). + AttrKeyContextID = "context.id" +) + // Telemetry attribute keys for database connectors. const ( // AttrDBSystem is the OTEL semantic convention attribute key for the database system name. diff --git a/constants/tenant.go b/constants/tenant.go new file mode 100644 index 0000000..ea82e13 --- /dev/null +++ b/constants/tenant.go @@ -0,0 +1,20 @@ +package constants + +const ( + // HeaderTenantID is the canonical HTTP header carrying the tenant + // identifier across Lerian services. + HeaderTenantID = "X-Tenant-Id" + + // MetadataTenantID is the canonical gRPC metadata key carrying the + // tenant identifier. gRPC metadata keys are lowercase by spec. + MetadataTenantID = "tenant-id" + + // AttrKeyTenantID is the OpenTelemetry attribute / log field key used + // everywhere tenant.id is emitted (logs, traces, metrics). + AttrKeyTenantID = "tenant.id" + + // MaxTenantIDLen caps tenant IDs extracted from request headers to keep + // telemetry cardinality bounded. Values exceeding the cap are dropped + // silently. + MaxTenantIDLen = 128 +) diff --git a/go.mod b/go.mod index 9e1b461..ff1ee7d 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/LerianStudio/lib-observability -go 1.25.10 +go 1.26.3 require ( github.com/gofiber/fiber/v2 v2.52.13 @@ -8,25 +8,25 @@ require ( github.com/shirou/gopsutil v3.21.11+incompatible github.com/shopspring/decimal v1.4.0 github.com/stretchr/testify v1.11.1 - go.opentelemetry.io/contrib/bridges/otelzap v0.18.0 - go.opentelemetry.io/otel v1.43.0 - go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.19.0 - go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.43.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0 - go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.43.0 - go.opentelemetry.io/otel/log v0.19.0 - go.opentelemetry.io/otel/metric v1.43.0 - go.opentelemetry.io/otel/sdk v1.43.0 - go.opentelemetry.io/otel/sdk/log v0.19.0 - go.opentelemetry.io/otel/sdk/metric v1.43.0 - go.opentelemetry.io/otel/trace v1.43.0 + go.opentelemetry.io/contrib/bridges/otelzap v0.19.0 + go.opentelemetry.io/otel v1.44.0 + go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.20.0 + go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.44.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.44.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.44.0 + go.opentelemetry.io/otel/log v0.20.0 + go.opentelemetry.io/otel/metric v1.44.0 + go.opentelemetry.io/otel/sdk v1.44.0 + go.opentelemetry.io/otel/sdk/log v0.20.0 + go.opentelemetry.io/otel/sdk/metric v1.44.0 + go.opentelemetry.io/otel/trace v1.44.0 go.uber.org/mock v0.6.0 go.uber.org/zap v1.28.0 - google.golang.org/grpc v1.81.0 + google.golang.org/grpc v1.81.1 ) require ( - github.com/andybalholm/brotli v1.2.0 // indirect + github.com/andybalholm/brotli v1.2.1 // indirect github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/clipperhouse/uax29/v2 v2.7.0 // indirect @@ -34,25 +34,25 @@ require ( github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.3.0 // indirect - github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 // indirect - github.com/klauspost/compress v1.18.5 // indirect + github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect + github.com/klauspost/compress v1.18.6 // indirect github.com/mattn/go-colorable v0.1.14 // indirect - github.com/mattn/go-isatty v0.0.20 // indirect - github.com/mattn/go-runewidth v0.0.21 // indirect + github.com/mattn/go-isatty v0.0.22 // indirect + github.com/mattn/go-runewidth v0.0.23 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/tklauser/go-sysconf v0.3.16 // indirect - github.com/tklauser/numcpus v0.11.0 // indirect + github.com/tklauser/go-sysconf v0.4.0 // indirect + github.com/tklauser/numcpus v0.12.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect - github.com/valyala/fasthttp v1.69.0 // indirect + github.com/valyala/fasthttp v1.71.0 // indirect github.com/yusufpapurcu/wmi v1.2.4 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/net v0.53.0 // indirect - golang.org/x/sys v0.43.0 // indirect - golang.org/x/text v0.36.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect + golang.org/x/net v0.55.0 // indirect + golang.org/x/sys v0.45.0 // indirect + golang.org/x/text v0.37.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect google.golang.org/protobuf v1.36.11 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 340e165..591309d 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwToPjQ= -github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= +github.com/andybalholm/brotli v1.2.1 h1:R+f5xP285VArJDRgowrfb9DqL18yVK0gKAW/F+eTWro= +github.com/andybalholm/brotli v1.2.1/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= @@ -24,20 +24,20 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 h1:HWRh5R2+9EifMyIHV7ZV+MIZqgz+PMpZ14Jynv3O2Zs= -github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0/go.mod h1:JfhWUomR1baixubs02l85lZYYOm7LV6om4ceouMv45c= -github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= -github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 h1:5VipnvEpbqr2gA2VbM+nYVbkIF28c5ZQfqCBQ5g2xfk= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0/go.mod h1:Hyl3n6Twe1hvtd9XUXDec4pTvgMSEixRuQKPTMH2bNs= +github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= +github.com/klauspost/compress v1.18.6/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/mattn/go-runewidth v0.0.21 h1:jJKAZiQH+2mIinzCJIaIG9Be1+0NR+5sz/lYEEjdM8w= -github.com/mattn/go-runewidth v0.0.21/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= +github.com/mattn/go-isatty v0.0.22 h1:j8l17JJ9i6VGPUFUYoTUKPSgKe/83EYU2zBC7YNKMw4= +github.com/mattn/go-isatty v0.0.22/go.mod h1:ZXfXG4SQHsB/w3ZeOYbR0PrPwLy+n6xiMrJlRFqopa4= +github.com/mattn/go-runewidth v0.0.23 h1:7ykA0T0jkPpzSvMS5i9uoNn2Xy3R383f9HDx3RybWcw= +github.com/mattn/go-runewidth v0.0.23/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= @@ -48,48 +48,50 @@ github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -github.com/tklauser/go-sysconf v0.3.16 h1:frioLaCQSsF5Cy1jgRBrzr6t502KIIwQ0MArYICU0nA= -github.com/tklauser/go-sysconf v0.3.16/go.mod h1:/qNL9xxDhc7tx3HSRsLWNnuzbVfh3e7gh/BmM179nYI= -github.com/tklauser/numcpus v0.11.0 h1:nSTwhKH5e1dMNsCdVBukSZrURJRoHbSEQjdEbY+9RXw= -github.com/tklauser/numcpus v0.11.0/go.mod h1:z+LwcLq54uWZTX0u/bGobaV34u6V7KNlTZejzM6/3MQ= +github.com/tklauser/go-sysconf v0.4.0 h1:7H0uAN+7RkwWRaxhYXDLqa5V3LPrJeV8wmD9dRUgPQU= +github.com/tklauser/go-sysconf v0.4.0/go.mod h1:8mTNWyog7H+MpKijp4VmKJAd2bbYQ2zuUwkYRbUArPI= +github.com/tklauser/numcpus v0.12.0 h1:NR85qdvHA9pFse3x3weVZ0r0ST8R6l5RHbZrlRaqob4= +github.com/tklauser/numcpus v0.12.0/go.mod h1:ABHeXzJnr/qqwguhClkZKT1/8VABcYrsyUiUGobwWJg= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= -github.com/valyala/fasthttp v1.69.0 h1:fNLLESD2SooWeh2cidsuFtOcrEi4uB4m1mPrkJMZyVI= -github.com/valyala/fasthttp v1.69.0/go.mod h1:4wA4PfAraPlAsJ5jMSqCE2ug5tqUPwKXxVj8oNECGcw= +github.com/valyala/fasthttp v1.71.0 h1:tepR7H+Guh9VUqxxcPggYi8R3lGUu2Rsdh+z7/FCY3k= +github.com/valyala/fasthttp v1.71.0/go.mod h1:z1sDUvOShhXq/C9mwH/fSm1Vb71tUJwmQdgkBrBNwnA= github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU= github.com/xyproto/randomstring v1.0.5/go.mod h1:rgmS5DeNXLivK7YprL0pY+lTuhNQW3iGxZ18UQApw/E= github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/bridges/otelzap v0.18.0 h1:EkWTww6Nqs2P29r01NeuNsG7qNJtoWWaT1fx/CKode8= -go.opentelemetry.io/contrib/bridges/otelzap v0.18.0/go.mod h1:lj3bgA/c7nJy0NhxqyvWJFC30aTgB+G0RKDdLbvJ4QM= -go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I= -go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0= -go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.19.0 h1:Dn8rkudDzY6KV9dr/D/bTUuWgqDf9xe0rr4G2elrn0Y= -go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.19.0/go.mod h1:gMk9F0xDgyN9M/3Ed5Y1wKcx/9mlU91NXY2SNq7RQuU= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.43.0 h1:8UQVDcZxOJLtX6gxtDt3vY2WTgvZqMQRzjsqiIHQdkc= -go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.43.0/go.mod h1:2lmweYCiHYpEjQ/lSJBYhj9jP1zvCvQW4BqL9dnT7FQ= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0 h1:88Y4s2C8oTui1LGM6bTWkw0ICGcOLCAI5l6zsD1j20k= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0/go.mod h1:Vl1/iaggsuRlrHf/hfPJPvVag77kKyvrLeD10kpMl+A= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.43.0 h1:RAE+JPfvEmvy+0LzyUA25/SGawPwIUbZ6u0Wug54sLc= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.43.0/go.mod h1:AGmbycVGEsRx9mXMZ75CsOyhSP6MFIcj/6dnG+vhVjk= -go.opentelemetry.io/otel/log v0.19.0 h1:KUZs/GOsw79TBBMfDWsXS+KZ4g2Ckzksd1ymzsIEbo4= -go.opentelemetry.io/otel/log v0.19.0/go.mod h1:5DQYeGmxVIr4n0/BcJvF4upsraHjg6vudJJpnkL6Ipk= -go.opentelemetry.io/otel/log/logtest v0.19.0 h1:HdSsl4ndTK15LtJGLWBfMsSlLrCgSeE3VMzwOrLYiYs= -go.opentelemetry.io/otel/log/logtest v0.19.0/go.mod h1:c1sH1nOHTwfMCWhhQTdWGqxgDjZhtkbkzAqGGyj0Ijs= -go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM= -go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY= -go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg= -go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg= -go.opentelemetry.io/otel/sdk/log v0.19.0 h1:scYVLqT22D2gqXItnWiocLUKGH9yvkkeql5dBDiXyko= -go.opentelemetry.io/otel/sdk/log v0.19.0/go.mod h1:vFBowwXGLlW9AvpuF7bMgnNI95LiW10szrOdvzBHlAg= -go.opentelemetry.io/otel/sdk/log/logtest v0.19.0 h1:BEbF7ZBB6qQloV/Ub1+3NQoOUnVtcGkU3XX4Ws3GQfk= -go.opentelemetry.io/otel/sdk/log/logtest v0.19.0/go.mod h1:Lua81/3yM0wOmoHTokLj9y9ADeA02v1naRrVrkAZuKk= -go.opentelemetry.io/otel/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw= -go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A= -go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A= -go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0= +go.opentelemetry.io/contrib/bridges/otelzap v0.19.0 h1:48Eq3xxFx2KlL/tF7lnl42kKJBDlhNTLRzv0h154JnM= +go.opentelemetry.io/contrib/bridges/otelzap v0.19.0/go.mod h1:cQbV77F0u6HmtZPiQD9oxp2esaOEb4uLqIta6OFIKOk= +go.opentelemetry.io/otel v1.44.0 h1:JjwHmHpA4iZ3wBxluu2fbbE7j4kqlE8jXyAyPXH7HqU= +go.opentelemetry.io/otel v1.44.0/go.mod h1:BMgjTHL9WPRlRjL2oZCBTL4whCGtXch2H4BhOPIAyYc= +go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.20.0 h1:rydZ9sxbcFdm/oWrVyfLTjHIygMgv0bEeMd+3B/BvoM= +go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.20.0/go.mod h1:earQ25dooT0Hhspq59DZ8YCC50jWfOlFEeWoxy/P444= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.44.0 h1:SUplec5dp06reu1zaXmOXdvqH398taqrDXqUl99jxSc= +go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.44.0/go.mod h1:ho2g4N+ane+swq5I/VBkKWnRDY4kUINH3FuqyZqX/Ug= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.44.0 h1:4YsVu3B8+3qtWYYrsUYgn0OG78pN0rnNPRGX4SbokQI= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.44.0/go.mod h1:+wnlSn0mD1ADVMe3v9Z/WIaiz6q6gL2J/ejaAmdmv80= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.44.0 h1:qazEJlUOQzhCpzQpFETGby7EdqjI1wsd0W+6Gg1SCTU= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.44.0/go.mod h1:fOD2Yefuxixkx3ahVNf0O/PERb6r4OlbxfATVnYvzCo= +go.opentelemetry.io/otel/log v0.20.0 h1:/5i0vuHxCLWUfChWG41K9wkM0jafruPw9NU1/RCJirs= +go.opentelemetry.io/otel/log v0.20.0/go.mod h1:wOcMcjsZpG8x7Bak7IhSi/lg8wscV2C1VdrKCLPlt0E= +go.opentelemetry.io/otel/log/logtest v0.20.0 h1:+tsZVE15N+RWyN9lUzsRyw7hMZXNMepGu105Eim82/k= +go.opentelemetry.io/otel/log/logtest v0.20.0/go.mod h1:zS9Ryx9RrEAG2tgapMBSvacwhVSSOGSaSiWWgW3NPlQ= +go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSYCdDdmc= +go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= +go.opentelemetry.io/otel/metric/x v0.66.0 h1:YkCrx1zLOChi9ZcZ6euupOcsgzbVlec7D/xoEU1+cTA= +go.opentelemetry.io/otel/metric/x v0.66.0/go.mod h1:d1+BDj9t96do0/1LoU1ayfCv79ZgNE41qbhBvnMOBZk= +go.opentelemetry.io/otel/sdk v1.44.0 h1:nHYwb9lK+fJPU/dnT6s7W7Z8itMWyqrnVfbheVYrZ58= +go.opentelemetry.io/otel/sdk v1.44.0/go.mod h1:Osuydd3Se74nqjAKxid74N5eC+jfEqfTegHRnq58oK0= +go.opentelemetry.io/otel/sdk/log v0.20.0 h1:vM3xI7TQgKPiSghe6urZtAkyFY7SodrSpC83CffDFuY= +go.opentelemetry.io/otel/sdk/log v0.20.0/go.mod h1:Knej2nmsTUzN79T2eeXdRsjjPcoxoq2pUyUHz9TFyyU= +go.opentelemetry.io/otel/sdk/log/logtest v0.20.0 h1:OqdRZ1guyzamK3M6LlRsmGqRrjkHWw6WZOKKli5ELpg= +go.opentelemetry.io/otel/sdk/log/logtest v0.20.0/go.mod h1:PuMIlm7zAt7c3z8zfOI5ox4iT1Z87We+PF6YoINux/M= +go.opentelemetry.io/otel/sdk/metric v1.44.0 h1:3LlKgI+VjbVsjNRFZJZAJ30WjXC5VkNRks6si09iEfI= +go.opentelemetry.io/otel/sdk/metric v1.44.0/go.mod h1:5B5pMARnXxKhltooO4xUuCBorl65a4EpnTalObqOigA= +go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= +go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= go.opentelemetry.io/proto/otlp v1.10.0 h1:IQRWgT5srOCYfiWnpqUYz9CVmbO8bFmKcwYxpuCSL2g= go.opentelemetry.io/proto/otlp v1.10.0/go.mod h1:/CV4QoCR/S9yaPj8utp3lvQPoqMtxXdzn7ozvvozVqk= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= @@ -102,23 +104,22 @@ go.uber.org/zap v1.28.0 h1:IZzaP1Fv73/T/pBMLk4VutPl36uNC+OSUh3JLG3FIjo= go.uber.org/zap v1.28.0/go.mod h1:rDLpOi171uODNm/mxFcuYWxDsqWSAVkFdX4XojSKg/Q= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= -golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= -golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= -golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= -google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9 h1:VPWxll4HlMw1Vs/qXtN7BvhZqsS9cdAittCNvVENElA= -google.golang.org/genproto/googleapis/api v0.0.0-20260401024825-9d38bb4040a9/go.mod h1:7QBABkRtR8z+TEnmXTqIqwJLlzrZKVfAUm7tY3yGv0M= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 h1:m8qni9SQFH0tJc1X0vmnpw/0t+AImlSvp30sEupozUg= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= -google.golang.org/grpc v1.81.0 h1:W3G9N3KQf3BU+YuCtGKJk0CmxQNbAISICD/9AORxLIw= -google.golang.org/grpc v1.81.0/go.mod h1:xGH9GfzOyMTGIOXBJmXt+BX/V0kcdQbdcuwQ/zNw42I= +google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa h1:Kjn0N0tCrDgiAFW+lGO4JZ3ck44CehvJQMAwj9QF0G8= +google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa/go.mod h1:q4lMZS6kskjT5HvCPrnnypcDPVJqT/f4nfxmkE7gryY= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa h1:mZHHdPZl0dbGHCflZgAq/Q468DWVFcU2whhB2KAo8fk= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.81.1 h1:VnnIIZ88UzOOKLukQi+ImGz8O1Wdp8nAGGnvOfEIWQQ= +google.golang.org/grpc v1.81.1/go.mod h1:xGH9GfzOyMTGIOXBJmXt+BX/V0kcdQbdcuwQ/zNw42I= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index 9a04334..2f7ef6c 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -31,9 +31,11 @@ type cacheTestMeter struct { histogram *cacheTestHistogram } -type cacheTestCounter struct{ metric.Int64Counter } -type cacheTestGauge struct{ metric.Int64Gauge } -type cacheTestHistogram struct{ metric.Int64Histogram } +type ( + cacheTestCounter struct{ metric.Int64Counter } + cacheTestGauge struct{ metric.Int64Gauge } + cacheTestHistogram struct{ metric.Int64Histogram } +) func newCacheTestFactory(t *testing.T) *MetricsFactory { t.Helper() diff --git a/middleware/context_span.go b/middleware/context_span.go index 356cd41..30ff927 100644 --- a/middleware/context_span.go +++ b/middleware/context_span.go @@ -1,8 +1,11 @@ package middleware import ( + "context" "reflect" + observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" "github.com/google/uuid" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -26,48 +29,70 @@ func isNilSpan(span trace.Span) bool { } } -// SetHandlerSpanAttributes adds tenant_id and context_id attributes to a trace span. -func SetHandlerSpanAttributes(span trace.Span, tenantID, contextID uuid.UUID) { - if isNilSpan(span) { - return +// SetHandlerSpanAttributes adds tenant.id and context.id attributes to the +// current trace span AND propagates them into the request-wide AttrBag carried +// by ctx, returning the enriched context. +// +// Why both sinks: setting attributes only on the child span (the previous +// behavior) left the AttrBag empty, so when WithTelemetry read the tenant.id +// back from the AttrBag after c.Next() to label http.server.request.duration +// (via tenantIDFromAttrBag) it found nothing, and the metric/root span lost the +// tenant.id. Funneling the same attributes through ContextWithSpanAttributes +// fixes that: the AttrBagSpanProcessor copies them onto the root span and the +// duration metric gains the tenant.id label. +// +// Callers MUST use the returned context for downstream work (handler chain, +// c.SetUserContext) so the propagated attributes are visible; the AttrBag lives +// in an immutable context value and cannot be mutated in place. +func SetHandlerSpanAttributes(ctx context.Context, span trace.Span, tenantID, contextID uuid.UUID) context.Context { + if ctx == nil { + ctx = context.Background() } - span.SetAttributes(attribute.String("tenant.id", tenantID.String())) + attrs := []attribute.KeyValue{ + attribute.String(constant.AttrKeyTenantID, tenantID.String()), + } if contextID != uuid.Nil { - span.SetAttributes(attribute.String("context.id", contextID.String())) + attrs = append(attrs, attribute.String(constant.AttrKeyContextID, contextID.String())) } + + if !isNilSpan(span) { + span.SetAttributes(attrs...) + } + + return observability.ContextWithSpanAttributes(ctx, attrs...) } -// SetTenantSpanAttribute adds tenant_id attribute to a trace span. +// SetTenantSpanAttribute adds tenant.id attribute to a trace span. func SetTenantSpanAttribute(span trace.Span, tenantID uuid.UUID) { if isNilSpan(span) { return } - span.SetAttributes(attribute.String("tenant.id", tenantID.String())) + span.SetAttributes(attribute.String(constant.AttrKeyTenantID, tenantID.String())) } -// SetExceptionSpanAttributes adds tenant_id and exception_id attributes to a trace span. +// SetExceptionSpanAttributes adds tenant.id and exception.id attributes to a trace span. func SetExceptionSpanAttributes(span trace.Span, tenantID, exceptionID uuid.UUID) { if isNilSpan(span) { return } span.SetAttributes( - attribute.String("tenant.id", tenantID.String()), + attribute.String(constant.AttrKeyTenantID, tenantID.String()), attribute.String("exception.id", exceptionID.String()), ) } -// SetDisputeSpanAttributes adds tenant_id and dispute_id attributes to a trace span. +// SetDisputeSpanAttributes adds tenant.id and dispute.id attributes to a trace span. func SetDisputeSpanAttributes(span trace.Span, tenantID, disputeID uuid.UUID) { if isNilSpan(span) { return } span.SetAttributes( - attribute.String("tenant.id", tenantID.String()), + attribute.String(constant.AttrKeyTenantID, tenantID.String()), attribute.String("dispute.id", disputeID.String()), ) } diff --git a/middleware/context_span_test.go b/middleware/context_span_test.go new file mode 100644 index 0000000..65f2f40 --- /dev/null +++ b/middleware/context_span_test.go @@ -0,0 +1,123 @@ +//go:build unit + +package middleware + +import ( + "context" + "testing" + + observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/attribute" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" +) + +// attrBagValue returns the value stored in the request AttrBag for key, or "" +// when absent. It reads through the public AttributesFromContext accessor so +// the test exercises the same path the span processor and metric labeling use. +func attrBagValue(ctx context.Context, key string) (string, bool) { + for _, attr := range observability.AttributesFromContext(ctx) { + if attr.Key == attribute.Key(key) { + return attr.Value.AsString(), true + } + } + + return "", false +} + +// spanAttrValue returns the value of key on the recorded span, or "" when absent. +func spanAttrValue(span sdktrace.ReadOnlySpan, key string) (string, bool) { + for _, attr := range span.Attributes() { + if attr.Key == attribute.Key(key) { + return attr.Value.AsString(), true + } + } + + return "", false +} + +// TestSetHandlerSpanAttributes_PropagatesTenantToAttrBag is the regression +// guard for the root cause: the helper must push tenant.id into the AttrBag so +// WithTelemetry can read it back (tenantIDFromAttrBag) when labeling +// http.server.request.duration and seeding the root span. +func TestSetHandlerSpanAttributes_PropagatesTenantToAttrBag(t *testing.T) { + recorder := tracetest.NewSpanRecorder() + tp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(recorder)) + _, span := tp.Tracer("test").Start(context.Background(), "handler-span") + + tenantID := uuid.New() + contextID := uuid.New() + + ctx := SetHandlerSpanAttributes(context.Background(), span, tenantID, contextID) + span.End() + + // AttrBag carries tenant.id (the previously missing propagation). + got, ok := attrBagValue(ctx, constant.AttrKeyTenantID) + assert.True(t, ok, "tenant.id must be present in the AttrBag") + assert.Equal(t, tenantID.String(), got) + + // tenantIDFromAttrBag (the exact path WithTelemetry uses) resolves it. + assert.Equal(t, tenantID.String(), tenantIDFromAttrBag(ctx)) + + // context.id is also propagated. + gotCtxID, ok := attrBagValue(ctx, constant.AttrKeyContextID) + assert.True(t, ok, "context.id must be present in the AttrBag") + assert.Equal(t, contextID.String(), gotCtxID) + + // The span still receives both attributes (behavior preserved). + recorded := recorder.Ended()[0] + gotSpanTenant, ok := spanAttrValue(recorded, constant.AttrKeyTenantID) + assert.True(t, ok) + assert.Equal(t, tenantID.String(), gotSpanTenant) + + gotSpanCtxID, ok := spanAttrValue(recorded, constant.AttrKeyContextID) + assert.True(t, ok) + assert.Equal(t, contextID.String(), gotSpanCtxID) +} + +// TestSetHandlerSpanAttributes_NilContextIDOmitsContextID confirms a Nil +// contextID is not propagated to either sink, matching the prior span-only +// behavior of skipping the empty UUID. +func TestSetHandlerSpanAttributes_NilContextIDOmitsContextID(t *testing.T) { + recorder := tracetest.NewSpanRecorder() + tp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(recorder)) + _, span := tp.Tracer("test").Start(context.Background(), "handler-span") + + tenantID := uuid.New() + + ctx := SetHandlerSpanAttributes(context.Background(), span, tenantID, uuid.Nil) + span.End() + + assert.Equal(t, tenantID.String(), tenantIDFromAttrBag(ctx)) + + _, ok := attrBagValue(ctx, constant.AttrKeyContextID) + assert.False(t, ok, "context.id must be omitted when contextID is uuid.Nil") + + _, ok = spanAttrValue(recorder.Ended()[0], constant.AttrKeyContextID) + assert.False(t, ok, "context.id must be omitted on the span when contextID is uuid.Nil") +} + +// TestSetHandlerSpanAttributes_NilSpanStillPropagates ensures a nil/typed-nil +// span does not panic and the AttrBag propagation still happens, so the metric +// path keeps the tenant.id even when no recording span is active. +func TestSetHandlerSpanAttributes_NilSpanStillPropagates(t *testing.T) { + tenantID := uuid.New() + + ctx := SetHandlerSpanAttributes(context.Background(), nil, tenantID, uuid.Nil) + + assert.Equal(t, tenantID.String(), tenantIDFromAttrBag(ctx)) +} + +// TestSetHandlerSpanAttributes_NilContextDefaults guards against a nil context +// input: the helper must fall back to a background context rather than panic. +func TestSetHandlerSpanAttributes_NilContextDefaults(t *testing.T) { + tenantID := uuid.New() + + ctx := SetHandlerSpanAttributes(nil, nil, tenantID, uuid.Nil) //nolint:staticcheck // intentional nil ctx + + assert.NotNil(t, ctx) + assert.Equal(t, tenantID.String(), tenantIDFromAttrBag(ctx)) +} diff --git a/middleware/helpers.go b/middleware/helpers.go index c356ba3..2789a45 100644 --- a/middleware/helpers.go +++ b/middleware/helpers.go @@ -3,6 +3,7 @@ package middleware import ( "context" "net/url" + "reflect" "regexp" "strings" @@ -18,12 +19,126 @@ var uuidPattern = regexp.MustCompile(`[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{ // internalServicePattern matches Lerian internal service user-agent strings. var internalServicePattern = regexp.MustCompile(`^[\w-]+/[\d.]+\s+LerianStudio$`) -// isRouteExcludedFromList reports whether the request path matches any excluded route prefix. -// This standalone function is used to evaluate route exclusions independently of whether -// the TelemetryMiddleware receiver is nil. +// knownHTTPMethods is the canonical case-sensitive set per OpenTelemetry +// HTTP semantic conventions; methods outside this set are reported as +// "_OTHER" on telemetry to keep label cardinality bounded. +var knownHTTPMethods = map[string]struct{}{ + "GET": {}, "HEAD": {}, "POST": {}, "PUT": {}, "DELETE": {}, + "CONNECT": {}, "OPTIONS": {}, "TRACE": {}, "PATCH": {}, +} + +// normalizeHTTPMethod returns the canonical method label and, if a +// substitution happened, the original verb. Comparison is intentionally +// case-sensitive: compliant clients send uppercase, and lowercase variants +// genuinely belong in "_OTHER". +func normalizeHTTPMethod(raw string) (normalized, original string, replaced bool) { + if _, ok := knownHTTPMethods[raw]; ok { + return raw, "", false + } + + return "_OTHER", raw, true +} + +// routeAttribute returns the route template suitable for the http.route +// telemetry attribute, plus a present flag. Fiber v2 exposes Route().Path +// == "/" for unmatched requests (its default catch-all), which would +// conflate scanner/404 traffic with the actual root handler in dashboards. +// We detect this case (effective status == 404 AND route == "/" AND the +// request path is NOT "/") and report the attribute as absent so callers +// can omit it entirely, matching OTel guidance that http.route SHOULD be +// absent when no route matched. +func routeAttribute(c *fiber.Ctx, effectiveStatus int) (string, bool) { + if c == nil { + return "", false + } + + r := c.Route() + if r == nil { + return "", false + } + + if effectiveStatus == fiber.StatusNotFound && r.Path == "/" && c.Path() != "/" { + return "", false + } + + return r.Path, true +} + +// maxUserAgentAttrLen caps the user_agent.original span attribute to avoid +// inflating trace storage/index cost. 256 bytes is sufficient for canonical +// client/library/version identifiers in practice. +const maxUserAgentAttrLen = 256 + +// truncateUserAgent caps the user-agent string at maxUserAgentAttrLen bytes, +// truncating at a rune boundary so the returned string is always valid UTF-8. +// Compliant user-agents are ASCII, but defensive callers may receive +// multi-byte sequences; a byte-level slice could leave a partial rune in the +// span attribute. +func truncateUserAgent(ua string) string { + if len(ua) <= maxUserAgentAttrLen { + return ua + } + + // for i := range ua iterates over rune boundaries; i is the byte index + // at the start of each rune. We track the last boundary that still fits + // within the cap and return up to it, so the result never exceeds + // maxUserAgentAttrLen bytes and never splits a rune. + lastFit := 0 + + for i := range ua { + if i > maxUserAgentAttrLen { + return ua[:lastFit] + } + + lastFit = i + } + + return ua[:lastFit] +} + +// errorTypeOriginal returns the originating Go type name of handlerErr, +// suitable as a high-cardinality debugging attribute on spans. Returns +// "" if handlerErr is nil. Unwraps all pointer levels so "***fiber.Error" +// surfaces as "fiber.Error". Falls back to "error" when reflect cannot +// resolve a meaningful name. +func errorTypeOriginal(handlerErr error) string { + if handlerErr == nil { + return "" + } + + t := reflect.TypeOf(handlerErr) + if t == nil { + return "error" + } + + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + + if name := t.String(); name != "" { + return name + } + + return "error" +} + +// isRouteExcludedFromList reports whether the request path matches any +// excluded route on a path-segment boundary. A route matches when the +// path equals it exactly or starts with "route + /", so "/health" excludes +// "/health" and "/health/check" but NOT "/healthz" or "/health-check". +// +// Trailing slashes on excluded entries are tolerated, and empty entries +// are ignored so they cannot act as accidental wildcards. func isRouteExcludedFromList(c *fiber.Ctx, excludedRoutes []string) bool { + path := c.Path() + for _, route := range excludedRoutes { - if strings.HasPrefix(c.Path(), route) { + route = strings.TrimRight(route, "/") + if route == "" { + continue + } + + if path == route || strings.HasPrefix(path, route+"/") { return true } } diff --git a/middleware/helpers_test.go b/middleware/helpers_test.go new file mode 100644 index 0000000..8968f34 --- /dev/null +++ b/middleware/helpers_test.go @@ -0,0 +1,66 @@ +//go:build unit + +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gofiber/fiber/v2" + "github.com/stretchr/testify/assert" +) + +func TestIsRouteExcludedFromList(t *testing.T) { + cases := []struct { + name string + path string + excluded []string + want bool + }{ + // exact match + {name: "exact match", path: "/health", excluded: []string{"/health"}, want: true}, + + // subpath match (segment boundary) + {name: "subpath under excluded route", path: "/health/check", excluded: []string{"/health"}, want: true}, + {name: "deep subpath", path: "/api/v1/users/42", excluded: []string{"/api/v1"}, want: true}, + + // regression guards against the original raw-prefix bug + {name: "sibling with shared prefix (suffix letter)", path: "/healthz", excluded: []string{"/health"}, want: false}, + {name: "sibling with shared prefix (hyphenated)", path: "/health-check", excluded: []string{"/health"}, want: false}, + {name: "sibling with shared prefix (under /metrics)", path: "/metricsproxy", excluded: []string{"/metrics"}, want: false}, + {name: "minor version not a child", path: "/api/v1.0/users", excluded: []string{"/api/v1"}, want: false}, + + // trailing slash tolerance on the excluded entry + {name: "excluded route with trailing slash matches path without", path: "/metrics", excluded: []string{"/metrics/"}, want: true}, + {name: "excluded route with trailing slash matches subpath", path: "/metrics/cpu", excluded: []string{"/metrics/"}, want: true}, + + // empty / root entries are not wildcards + {name: "empty string entry is not a wildcard", path: "/anything", excluded: []string{""}, want: false}, + {name: "root slash entry is not a wildcard", path: "/anything", excluded: []string{"/"}, want: false}, + + // list semantics + {name: "no excluded routes", path: "/anywhere", excluded: nil, want: false}, + {name: "no match in list", path: "/v1/orders", excluded: []string{"/health", "/readyz"}, want: false}, + {name: "match second entry", path: "/readyz", excluded: []string{"/health", "/readyz"}, want: true}, + {name: "match wins over later non-matches", path: "/health/x", excluded: []string{"/health", "/no-match"}, want: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + app := fiber.New() + var got bool + app.Use(func(c *fiber.Ctx) error { + got = isRouteExcludedFromList(c, tc.excluded) + return c.SendStatus(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodGet, tc.path, nil) + resp, err := app.Test(req) + assert.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + + assert.Equal(t, tc.want, got, "isRouteExcludedFromList(path=%q, excluded=%v)", tc.path, tc.excluded) + }) + } +} diff --git a/middleware/logging.go b/middleware/logging.go index b9e8179..306ac57 100644 --- a/middleware/logging.go +++ b/middleware/logging.go @@ -52,9 +52,18 @@ type ResponseMetricsWrapper struct { Size int } +// defaultLogExcludedRoutes is the canonical set of probe and scrape paths +// that are suppressed from access logging by default. Readiness probes and +// Prometheus scrapes fire every few seconds per pod and would otherwise +// dominate the access log. Failures still surface through the per-route +// observability emitted by the handler itself (e.g. the "readyz_unhealthy" +// Warn entry on 503). +var defaultLogExcludedRoutes = []string{"/health", "/readyz", "/metrics"} + type logMiddleware struct { Logger obslog.Logger ObfuscationDisabled bool + ExcludedRoutes []string } // LogMiddlewareOption configures HTTP and gRPC logging middleware. @@ -76,10 +85,27 @@ func WithObfuscationDisabled(disabled bool) LogMiddlewareOption { } } +// WithExcludedRoutes suppresses access logs for any request whose path is +// prefixed by one of the supplied routes. Matches the prefix semantics used +// by TelemetryMiddleware.WithTelemetry so a single env-driven list can be +// threaded through both middlewares. Repeated calls append. +func WithExcludedRoutes(routes ...string) LogMiddlewareOption { + return func(l *logMiddleware) { + for _, r := range routes { + if r == "" { + continue + } + + l.ExcludedRoutes = append(l.ExcludedRoutes, r) + } + } +} + func buildOpts(opts ...LogMiddlewareOption) *logMiddleware { mid := &logMiddleware{ Logger: &obslog.GoLogger{}, ObfuscationDisabled: logObfuscationDisabled, + ExcludedRoutes: append([]string(nil), defaultLogExcludedRoutes...), } for _, opt := range opts { @@ -180,9 +206,16 @@ func (r *RequestInfo) FinishRequestInfo(rw *ResponseMetricsWrapper) { } // WithHTTPLogging logs Fiber HTTP access requests. +// +// By default the probe paths /health and /readyz, the Prometheus scrape +// path /metrics, and Swagger asset routes are skipped. Use +// WithExcludedRoutes to suppress additional paths without losing the +// defaults. func WithHTTPLogging(opts ...LogMiddlewareOption) fiber.Handler { return func(c *fiber.Ctx) error { - if c.Path() == "/health" { + mid := buildOpts(opts...) + + if isRouteExcludedFromList(c, mid.ExcludedRoutes) { return c.Next() } @@ -192,15 +225,24 @@ func WithHTTPLogging(opts ...LogMiddlewareOption) fiber.Handler { setRequestHeaderID(c) - mid := buildOpts(opts...) info := NewRequestInfo(c, mid.ObfuscationDisabled) requestID := c.Get(headerID) + ctx := c.UserContext() + + if tenantID := ResolveTenantIDFromHTTP(c); tenantID != "" { + ctx = observability.ContextWithSpanAttributes(ctx, attribute.String(constant.AttrKeyTenantID, tenantID)) + } + logger := mid.Logger. With(obslog.String(headerID, info.TraceID)). With(obslog.String("message_prefix", requestID+constant.LoggerDefaultSeparator)) - ctx := observability.ContextWithLogger(c.UserContext(), logger) + if tenantID := resolveTenantIDForTelemetry(ctx); tenantID != "" { + logger = logger.With(obslog.String(constant.AttrKeyTenantID, tenantID)) + } + + ctx = observability.ContextWithLogger(ctx, logger) c.SetUserContext(ctx) err := c.Next() @@ -213,7 +255,12 @@ func WithHTTPLogging(opts ...LogMiddlewareOption) fiber.Handler { } info.FinishRequestInfo(&rw) - logger.Log(c.UserContext(), obslog.LevelInfo, info.CLFString()) + logger.With( + obslog.Int("http_status_code", info.Status), + obslog.String("http_method", info.Method), + obslog.String("http_path", info.URI), + obslog.Int("http_latency_ms", int(info.Duration.Milliseconds())), + ).Log(c.UserContext(), obslog.LevelInfo, info.CLFString()) return err } @@ -261,6 +308,10 @@ func WithGrpcLogging(opts ...LogMiddlewareOption) grpc.UnaryServerInterceptor { ctx = observability.ContextWithHeaderID(ctx, requestID) ctx = observability.ContextWithSpanAttributes(ctx, attribute.String("app.request.request_id", requestID)) + if tenantID := ResolveTenantIDFromGRPC(ctx); tenantID != "" { + ctx = observability.ContextWithSpanAttributes(ctx, attribute.String(constant.AttrKeyTenantID, tenantID)) + } + _, _, reqID, _ := observability.NewTrackingFromContext(ctx) mid := buildOpts(opts...) @@ -268,6 +319,10 @@ func WithGrpcLogging(opts ...LogMiddlewareOption) grpc.UnaryServerInterceptor { With(obslog.String(headerID, reqID)). With(obslog.String("message_prefix", reqID+constant.LoggerDefaultSeparator)) + if tenantID := resolveTenantIDForTelemetry(ctx); tenantID != "" { + logger = logger.With(obslog.String(constant.AttrKeyTenantID, tenantID)) + } + ctx = observability.ContextWithLogger(ctx, logger) start := time.Now() diff --git a/middleware/logging_test.go b/middleware/logging_test.go index 7df247f..50fa7e4 100644 --- a/middleware/logging_test.go +++ b/middleware/logging_test.go @@ -122,6 +122,22 @@ func TestWithHTTPLoggingAttachesLoggerAndLogsAccessEntry(t *testing.T) { require.Len(t, messages, 1) assert.Contains(t, messages[0], "GET /ok") assert.Contains(t, fields, obslog.String(headerID, resp.Header.Get(headerID))) + + var latencyFound bool + + for _, f := range fields { + if f.Key != "http_latency_ms" { + continue + } + + latencyFound = true + + ms, ok := f.Value.(int) + require.True(t, ok, "http_latency_ms should be an int") + assert.GreaterOrEqual(t, ms, 0) + } + + assert.True(t, latencyFound, "expected http_latency_ms field on access log entry") } func TestWithHTTPLoggingIgnoresTypedNilLogger(t *testing.T) { @@ -140,6 +156,95 @@ func TestWithHTTPLoggingIgnoresTypedNilLogger(t *testing.T) { assert.Equal(t, http.StatusNoContent, resp.StatusCode) } +func TestWithHTTPLoggingSkipsDefaultProbePaths(t *testing.T) { + probePaths := []string{"/health", "/readyz", "/metrics"} + + for _, path := range probePaths { + t.Run(path, func(t *testing.T) { + logger := &captureLogger{} + app := fiber.New() + app.Use(WithHTTPLogging(WithCustomLogger(logger))) + app.Get(path, func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, path, nil) + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + + messages, _ := logger.snapshot() + assert.Empty(t, messages, "no access log expected for default probe path %s", path) + }) + } +} + +func TestWithHTTPLoggingExcludedRoutesOptionSuppressesLogs(t *testing.T) { + logger := &captureLogger{} + app := fiber.New() + app.Use(WithHTTPLogging( + WithCustomLogger(logger), + WithExcludedRoutes("/internal"), + )) + app.Get("/internal/diag", func(c *fiber.Ctx) error { + return c.SendString("ok") + }) + + req := httptest.NewRequest(http.MethodGet, "/internal/diag", nil) + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + messages, _ := logger.snapshot() + assert.Empty(t, messages, "prefix-excluded path should not produce an access log") +} + +func TestWithHTTPLoggingExcludedRoutesOptionPreservesDefaults(t *testing.T) { + logger := &captureLogger{} + app := fiber.New() + app.Use(WithHTTPLogging( + WithCustomLogger(logger), + WithExcludedRoutes("/metrics"), + )) + app.Get("/readyz", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, "/readyz", nil) + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + messages, _ := logger.snapshot() + assert.Empty(t, messages, "supplying custom excluded routes must not remove the default probe skip set") +} + +func TestWithHTTPLoggingExcludedRoutesIgnoresEmptyStrings(t *testing.T) { + logger := &captureLogger{} + app := fiber.New() + app.Use(WithHTTPLogging( + WithCustomLogger(logger), + WithExcludedRoutes("", "/skip"), + )) + app.Get("/ok", func(c *fiber.Ctx) error { + return c.SendString("ok") + }) + + req := httptest.NewRequest(http.MethodGet, "/ok", nil) + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + messages, _ := logger.snapshot() + require.Len(t, messages, 1, "empty exclusion entries must not swallow every request") + assert.Contains(t, messages[0], "GET /ok") +} + func TestWithHTTPLoggingLogsErrorStatus(t *testing.T) { logger := &captureLogger{} app := fiber.New() diff --git a/middleware/request_attrs.go b/middleware/request_attrs.go new file mode 100644 index 0000000..ad887a7 --- /dev/null +++ b/middleware/request_attrs.go @@ -0,0 +1,26 @@ +package middleware + +import ( + "context" + + observability "github.com/LerianStudio/lib-observability" + "go.opentelemetry.io/otel/attribute" +) + +// RequestAttributes returns a copy of the request-scoped attribute bag +// populated by the HTTP/gRPC middleware (tenant.id, app.request.request_id, +// etc.). It is intended for explicit, opt-in inclusion of those attributes +// as metric labels — for example: +// +// meter.CounterBuilder("orders.created"). +// WithAttributes(middleware.RequestAttributes(ctx)...). +// Add(ctx, 1) +// +// The middleware does NOT add these attributes to metrics automatically: +// metric labels are a high-impact cardinality decision that must remain in +// the hands of the caller. Logs and traces already receive the bag via the +// logging middleware and AttrBagSpanProcessor respectively, so no caller +// action is required there. +func RequestAttributes(ctx context.Context) []attribute.KeyValue { + return observability.AttributesFromContext(ctx) +} diff --git a/middleware/telemetry.go b/middleware/telemetry.go index 9d47fe3..8ebf25c 100644 --- a/middleware/telemetry.go +++ b/middleware/telemetry.go @@ -7,21 +7,59 @@ import ( "errors" "fmt" "reflect" + "strconv" "strings" "sync" + "time" observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" "github.com/LerianStudio/lib-observability/tracing" "github.com/gofiber/fiber/v2" "github.com/google/uuid" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) +// httpServerRequestDurationMetric is the OpenTelemetry semantic-convention metric name +// for HTTP server request duration. Recorded as a Float64 histogram in seconds. +const httpServerRequestDurationMetric = "http.server.request.duration" + +// httpServerDurationBuckets follows the current OpenTelemetry HTTP semantic +// conventions advisory layout for http.server.request.duration. Update only +// in lockstep with the spec. +var httpServerDurationBuckets = []float64{ + 0.005, 0.01, 0.025, 0.05, 0.075, + 0.1, 0.25, 0.5, 0.75, + 1, 2.5, 5, 7.5, 10, +} + +// newHTTPServerDurationHistogram builds the float64 histogram instrument for +// http.server.request.duration on the given meter. Returns nil if the meter is +// nil or instrument creation fails - callers must treat nil as "do not record". +func newHTTPServerDurationHistogram(meter metric.Meter) metric.Float64Histogram { + if meter == nil { + return nil + } + + hist, err := meter.Float64Histogram( + httpServerRequestDurationMetric, + metric.WithUnit("s"), + metric.WithDescription("Duration of HTTP server requests."), + metric.WithExplicitBucketBoundaries(httpServerDurationBuckets...), + ) + if err != nil { + return nil + } + + return hist +} + // Header and metadata key constants used by the middleware. const ( // headerID is the request identifier header key. @@ -83,7 +121,41 @@ func NewTelemetryMiddleware(tl *tracing.Telemetry) *TelemetryMiddleware { } // WithTelemetry is a middleware that adds tracing to the context. +// +// When the effective Telemetry has a non-nil MeterProvider AND a non-nil +// MetricsFactory, the middleware also records the OpenTelemetry semantic- +// convention HTTP server metric `http.server.request.duration` (Float64 seconds +// histogram) for every non-excluded request. Recording is best-effort: nil +// telemetry, nil MeterProvider, nil MetricsFactory, excluded routes, and +// instrument creation errors all silently skip the metric without affecting +// the request path or existing span behavior. func (tm *TelemetryMiddleware) WithTelemetry(tl *tracing.Telemetry, excludedRoutes ...string) fiber.Handler { + // Build the duration histogram once at handler-construction time. The + // effective Telemetry may be supplied either via the explicit `tl` argument + // or via the receiver's stored Telemetry, mirroring the per-request logic + // below. If neither resolves, or any required component is nil, the + // histogram is left nil and recording is skipped. + var durationHistogram metric.Float64Histogram + + bootstrapTelemetry := tl + if bootstrapTelemetry == nil && tm != nil { + bootstrapTelemetry = tm.Telemetry + } + + // MetricsFactory presence is used here as the canonical "metrics subsystem + // enabled" signal across this library, even though the histogram itself is + // built directly from MeterProvider below. Keeping this gate aligned with + // the rest of the metrics package (see metrics/doc.go) ensures callers that + // disable metrics by nil-ing MetricsFactory also stop receiving the duration + // histogram, without us needing a separate enablement flag. + if bootstrapTelemetry != nil && + bootstrapTelemetry.MeterProvider != nil && + bootstrapTelemetry.MetricsFactory != nil { + durationHistogram = newHTTPServerDurationHistogram( + bootstrapTelemetry.MeterProvider.Meter(bootstrapTelemetry.LibraryName), + ) + } + return func(c *fiber.Ctx) error { effectiveTelemetry := tl if effectiveTelemetry == nil && tm != nil { @@ -100,23 +172,38 @@ func (tm *TelemetryMiddleware) WithTelemetry(tl *tracing.Telemetry, excludedRout setRequestHeaderID(c) + // Capture the request start time before any downstream work so the + // duration metric reflects the full handler chain, regardless of + // whether tracing is enabled below. + requestStart := time.Now() + ctx := c.UserContext() _, _, reqId, _ := observability.NewTrackingFromContext(ctx) + if tenantID := ResolveTenantIDFromHTTP(c); tenantID != "" { + ctx = observability.ContextWithSpanAttributes(ctx, attribute.String(constant.AttrKeyTenantID, tenantID)) + } + c.SetUserContext(observability.ContextWithSpanAttributes(ctx, attribute.String("app.request.request_id", reqId), )) - if effectiveTelemetry.TracerProvider == nil { - return c.Next() - } - // Capture all Fiber context string values BEFORE c.Next(). Fiber v2 uses // utils.UnsafeString which returns pointers into fasthttp's request buffer. // After c.Next() returns, fasthttp may recycle the underlying RequestCtx // for the next connection, corrupting any previously returned string slices. // Safe copies via string([]byte(...)) ensure the data is heap-owned. - method := string([]byte(c.Method())) + rawMethod := string([]byte(c.Method())) + method, methodOriginal, methodReplaced := normalizeHTTPMethod(rawMethod) + + if effectiveTelemetry.TracerProvider == nil { + err := c.Next() + + recordHTTPServerDuration(c, durationHistogram, method, requestStart, err) + + return err + } + originalURL := string([]byte(c.OriginalURL())) protocol := string([]byte(c.Protocol())) hostname := string([]byte(c.Hostname())) @@ -150,27 +237,164 @@ func (tm *TelemetryMiddleware) WithTelemetry(tl *tracing.Telemetry, excludedRout err = c.Next() - statusCode := c.Response().StatusCode() - span.SetAttributes( - attribute.String("http.request.method", method), - attribute.String("url.path", sanitizeURL(originalURL)), - attribute.String("http.route", c.Route().Path), - attribute.String("url.scheme", protocol), - attribute.String("server.address", hostname), - attribute.String("user_agent.original", userAgent), - attribute.Int("http.response.status_code", statusCode), - ) + // Reconcile the effective status the client will observe (same helper + // the metric uses) so the span's status code, error.type, and + // error.type_original stay consistent with the duration metric. + statusCode := httpStatusCode(c, err) - if err != nil { - tracing.HandleSpanError(span, "handler error", err) - } else if statusCode >= 500 { - span.SetStatus(codes.Error, fmt.Sprintf("HTTP %d", statusCode)) - } + applyTelemetrySpanAttributes(span, c, statusCode, telemetryRequestAttrs{ + method: method, + methodOriginal: methodOriginal, + methodReplaced: methodReplaced, + originalURL: originalURL, + protocol: protocol, + hostname: hostname, + userAgent: userAgent, + handlerErr: err, + }) + + recordHTTPServerDuration(c, durationHistogram, method, requestStart, err) return err } } +// telemetryRequestAttrs groups the per-request fields needed to apply OTel +// span attributes after c.Next() returns. Kept package-private; only +// applyTelemetrySpanAttributes consumes it. +type telemetryRequestAttrs struct { + method string + methodOriginal string + methodReplaced bool + originalURL string + protocol string + hostname string + userAgent string + handlerErr error +} + +// applyTelemetrySpanAttributes sets the OTel HTTP semantic-convention +// attributes on the request span and finalizes its status. Extracted from +// WithTelemetry to keep that function's cyclomatic complexity within the +// repo's lint budget; the behavior is identical to setting the attributes +// inline. +func applyTelemetrySpanAttributes( + span trace.Span, + c *fiber.Ctx, + statusCode int, + req telemetryRequestAttrs, +) { + spanAttrs := []attribute.KeyValue{ + attribute.String("http.request.method", req.method), + attribute.String("url.path", sanitizeURL(req.originalURL)), + attribute.String("url.scheme", req.protocol), + attribute.String("server.address", req.hostname), + attribute.String("user_agent.original", truncateUserAgent(req.userAgent)), + attribute.Int("http.response.status_code", statusCode), + } + if routePath, present := routeAttribute(c, statusCode); present { + spanAttrs = append(spanAttrs, attribute.String("http.route", routePath)) + } + + if req.methodReplaced { + spanAttrs = append(spanAttrs, attribute.String("http.request.method_original", req.methodOriginal)) + } + + if errType := classifyHTTPErrorType(statusCode); errType != "" { + spanAttrs = append(spanAttrs, attribute.String("error.type", errType)) + } + + if origType := errorTypeOriginal(req.handlerErr); origType != "" { + spanAttrs = append(spanAttrs, attribute.String("error.type_original", origType)) + } + + span.SetAttributes(spanAttrs...) + + if req.handlerErr != nil { + tracing.HandleSpanError(span, "handler error", req.handlerErr) + return + } + + if statusCode >= 500 { + span.SetStatus(codes.Error, fmt.Sprintf("HTTP %d", statusCode)) + } +} + +// recordHTTPServerDuration emits the http.server.request.duration histogram +// observation for a completed Fiber request. It is a no-op when the histogram +// is nil (telemetry/MeterProvider/MetricsFactory absent or instrument creation +// failed) so callers can invoke it unconditionally without nil checks. +// +// Attribute set follows OpenTelemetry HTTP semantic conventions: +// - http.request.method: captured before c.Next() to survive fasthttp recycling +// - http.route: c.Route().Path - low-cardinality route template, never raw paths; +// omitted entirely when no route matched (Fiber's catch-all 404), so scanner/ +// unmatched traffic does not pollute the root-route series. +// - http.response.status_code: the effective status the client will observe; +// derived from the handler error (*fiber.Error.Code, or 500 for generic +// errors) when Fiber's error handler has not yet rewritten the response, +// otherwise read directly from the response. This matches httpStatusCode +// used by the logging middleware and avoids reporting 200 for failures. +// - error.type: only set when effective status >= 500, using the numeric +// status code as a stable, low-cardinality label. +// - tenant.id: resolved via resolveTenantIDForTelemetry, the same +// AttrBag→baggage precedence used by the logging middleware and span +// processor. This covers both the local-hop X-Tenant-Id header (resolved +// into the AttrBag) AND tenant.id propagated cross-service via OTel +// baggage; reading the AttrBag alone previously dropped the baggage case, +// emitting an empty tenant_id label for downstream traffic. Omitted when +// neither source carries a value so series for non-tenant traffic do not +// gain an empty label. Cardinality is bounded by the 128-byte tenant cap in +// middleware/tenant.go, keeping the label safe for use in dashboards and +// alerts that filter by tenant. +func recordHTTPServerDuration( + c *fiber.Ctx, + hist metric.Float64Histogram, + method string, + start time.Time, + handlerErr error, +) { + if hist == nil || c == nil { + return + } + + statusCode := httpStatusCode(c, handlerErr) + + attrs := []attribute.KeyValue{ + attribute.String("http.request.method", method), + attribute.Int("http.response.status_code", statusCode), + } + if routePath, present := routeAttribute(c, statusCode); present { + attrs = append(attrs, attribute.String("http.route", routePath)) + } + + if errType := classifyHTTPErrorType(statusCode); errType != "" { + attrs = append(attrs, attribute.String("error.type", errType)) + } + + if tenantID := resolveTenantIDForTelemetry(c.UserContext()); tenantID != "" { + attrs = append(attrs, attribute.String(constant.AttrKeyTenantID, tenantID)) + } + + durationSeconds := time.Since(start).Seconds() + hist.Record(c.UserContext(), durationSeconds, metric.WithAttributes(attrs...)) +} + +// classifyHTTPErrorType returns the stable, low-cardinality error.type +// label for the http.server.request.duration metric per OpenTelemetry HTTP +// semantic conventions. Status-driven by design: a 503 surfaced via +// fiber.NewError(503) and a 503 surfaced via c.SendStatus(503) MUST produce +// the same time series so alert rules of the form error_type=~"5.." +// aggregate reliably. The originating Go type identity, when useful for +// debugging, is published separately on the span via errorTypeOriginal. +func classifyHTTPErrorType(statusCode int) string { + if statusCode >= 500 { + return strconv.Itoa(statusCode) + } + + return "" +} + // EndTracingSpans is a middleware that ends the tracing spans. func (tm *TelemetryMiddleware) EndTracingSpans(c *fiber.Ctx) error { if c == nil { @@ -230,6 +454,10 @@ func (tm *TelemetryMiddleware) WithTelemetryInterceptor(tl *tracing.Telemetry) g methodName = info.FullMethod } + if tenantID := ResolveTenantIDFromGRPC(ctx); tenantID != "" { + ctx = observability.ContextWithSpanAttributes(ctx, attribute.String(constant.AttrKeyTenantID, tenantID)) + } + ctx = observability.ContextWithSpanAttributes(ctx, attribute.String("app.request.request_id", requestID), attribute.String("grpc.method", methodName), diff --git a/middleware/telemetry_metrics_test.go b/middleware/telemetry_metrics_test.go new file mode 100644 index 0000000..0db5026 --- /dev/null +++ b/middleware/telemetry_metrics_test.go @@ -0,0 +1,982 @@ +//go:build unit + +package middleware + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "unicode/utf8" + + constant "github.com/LerianStudio/lib-observability/constants" + "github.com/LerianStudio/lib-observability/metrics" + "github.com/LerianStudio/lib-observability/tracing" + "github.com/gofiber/fiber/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" +) + +// newMetricsHarness wires a real OTel SDK ManualReader so tests can assert on +// the http.server.request.duration histogram exactly as it would appear to an +// exporter. Returns the configured Telemetry pointer plus a flush function. +func newMetricsHarness(t *testing.T) (*tracing.Telemetry, *sdkmetric.ManualReader) { + t.Helper() + + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + t.Cleanup(func() { _ = mp.Shutdown(context.Background()) }) + + factory, err := metrics.NewMetricsFactory(mp.Meter("test-library"), nil) + require.NoError(t, err) + + tel := &tracing.Telemetry{ + TelemetryConfig: tracing.TelemetryConfig{ + LibraryName: "test-library", + EnableTelemetry: true, + }, + MeterProvider: mp, + MetricsFactory: factory, + } + + return tel, reader +} + +// findDurationHistogram extracts the http.server.request.duration histogram +// data point from a ManualReader collection. Returns nil if the metric is +// absent (which the tests use to assert non-recording paths). +func findDurationHistogram( + t *testing.T, + reader *sdkmetric.ManualReader, +) *metricdata.HistogramDataPoint[float64] { + t.Helper() + + rm := &metricdata.ResourceMetrics{} + require.NoError(t, reader.Collect(context.Background(), rm)) + + for _, sm := range rm.ScopeMetrics { + for _, m := range sm.Metrics { + if m.Name != httpServerRequestDurationMetric { + continue + } + + h, ok := m.Data.(metricdata.Histogram[float64]) + require.True(t, ok, "expected Float64 histogram for %s, got %T", m.Name, m.Data) + require.NotEmpty(t, h.DataPoints, "histogram has no data points") + require.Equal(t, "s", m.Unit, "metric unit must be seconds") + + dp := h.DataPoints[0] + return &dp + } + } + + return nil +} + +func attrValue(set attribute.Set, key string) (string, bool) { + v, ok := set.Value(attribute.Key(key)) + if !ok { + return "", false + } + + return v.AsString(), true +} + +// newTelemetryHarness extends newMetricsHarness with a real TracerProvider +// backed by an InMemoryExporter so tests can assert on both the +// http.server.request.duration histogram and the span attributes produced +// by WithTelemetry in a single fixture. +func newTelemetryHarness( + t *testing.T, +) (*tracing.Telemetry, *sdkmetric.ManualReader, *tracetest.InMemoryExporter) { + t.Helper() + + tel, reader := newMetricsHarness(t) + + spanExp := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(spanExp)) + t.Cleanup(func() { _ = tp.Shutdown(context.Background()) }) + + tel.TracerProvider = tp + + return tel, reader, spanExp +} + +// getSpanAttr returns the string value for the named attribute on the given +// stub span, or "" if absent. Non-string attribute values are also returned +// stringified to keep call-site assertions uniform. +func getSpanAttr(span tracetest.SpanStub, key string) string { + for _, kv := range span.Attributes { + if string(kv.Key) == key { + return kv.Value.Emit() + } + } + + return "" +} + +// TestHTTPServerDurationBuckets_MatchOTelAdvisory locks the bucket layout +// against the current OTel HTTP semconv advisory. Any change to this slice +// is observable from dashboards, so it MUST be a deliberate spec-tracking +// update — never an accidental edit. +func TestHTTPServerDurationBuckets_MatchOTelAdvisory(t *testing.T) { + expected := []float64{ + 0.005, 0.01, 0.025, 0.05, 0.075, + 0.1, 0.25, 0.5, 0.75, + 1, 2.5, 5, 7.5, 10, + } + assert.Equal(t, expected, httpServerDurationBuckets) +} + +// TestWithTelemetry_RecordsDurationOnSuccess verifies that a successful 200 +// request emits the duration histogram with the route template (not the raw +// path) and no error.type attribute. +func TestWithTelemetry_RecordsDurationOnSuccess(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/users/:id", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/users/42", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp, "expected http.server.request.duration to be recorded") + assert.EqualValues(t, 1, dp.Count, "exactly one request should have been recorded") + assert.GreaterOrEqual(t, dp.Sum, 0.0, "duration sum must be non-negative seconds") + + method, ok := attrValue(dp.Attributes, "http.request.method") + require.True(t, ok) + assert.Equal(t, "GET", method) + + route, ok := attrValue(dp.Attributes, "http.route") + require.True(t, ok) + assert.Equal(t, "/api/users/:id", route, "must use route template, never raw path") + + statusVal, ok := dp.Attributes.Value(attribute.Key("http.response.status_code")) + require.True(t, ok) + assert.EqualValues(t, http.StatusOK, statusVal.AsInt64()) + + _, hasErr := dp.Attributes.Value(attribute.Key("error.type")) + assert.False(t, hasErr, "error.type must not be set on a 2xx response") +} + +// TestWithTelemetry_RecordsDurationOn4xx verifies a client-error response is +// recorded without an error.type attribute (4xx is not classified as an error +// per OTel HTTP server conventions). +func TestWithTelemetry_RecordsDurationOn4xx(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/items/:id", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusNotFound) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/items/missing", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusNotFound, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + assert.EqualValues(t, 1, dp.Count) + + statusVal, ok := dp.Attributes.Value(attribute.Key("http.response.status_code")) + require.True(t, ok) + assert.EqualValues(t, http.StatusNotFound, statusVal.AsInt64()) + + _, hasErr := dp.Attributes.Value(attribute.Key("error.type")) + assert.False(t, hasErr, "4xx must not set error.type") +} + +// TestWithTelemetry_RecordsDurationOn5xxStatus verifies that a 5xx response +// returned by the handler (without returning an error) is classified with a +// numeric error.type derived from the status code. +func TestWithTelemetry_RecordsDurationOn5xxStatus(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/health", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusServiceUnavailable) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/health", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + errType, ok := attrValue(dp.Attributes, "error.type") + require.True(t, ok, "5xx without handler error must still set error.type") + assert.Equal(t, "503", errType) + + // SendStatus(503) carries no handler error, so error.type_original must + // be absent everywhere (regression guard against re-introducing the + // Go-type label on the metric). + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrigOnMetric, + "error.type_original must NEVER appear on the metric") +} + +// TestWithTelemetry_RecordsDurationOnHandlerError verifies the Opção C +// hybrid for a generic handler-returned error reconciled to 500: +// - Metric error.type is the status code string ("500"), never the Go type +// name (which would balloon cardinality across application error types). +// - Metric does NOT carry error.type_original. +// - Span carries the same numeric error.type plus error.type_original with +// the originating Go type name for debugging. +func TestWithTelemetry_RecordsDurationOnHandlerError(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New(fiber.Config{ + ErrorHandler: func(c *fiber.Ctx, err error) error { + return c.Status(http.StatusInternalServerError).SendString(err.Error()) + }, + }) + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + sentinel := errors.New("boom") + app.Get("/api/explode", func(c *fiber.Ctx) error { + return sentinel + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/explode", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + + // Metric: numeric error.type, no error.type_original. + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + errType, ok := attrValue(dp.Attributes, "error.type") + require.True(t, ok) + assert.Equal(t, "500", errType, + "metric error.type must be status-driven for low cardinality") + + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrigOnMetric, + "error.type_original must NEVER appear on the metric") + + // Span: same numeric error.type + originating Go type name. + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Equal(t, "500", getSpanAttr(spans[0], "error.type")) + assert.Equal(t, "errors.errorString", + getSpanAttr(spans[0], "error.type_original"), + "span must surface the originating Go type name for debugging") +} + +// TestWithTelemetry_FiberError4xxOmitsErrorTypeOnMetric verifies that a +// handler returning fiber.NewError(4xx) records the effective HTTP status +// from the error but does NOT set error.type on the metric (4xx is not +// classified as an error per the status-driven contract). The originating +// *fiber.Error type is preserved on the span as error.type_original. +func TestWithTelemetry_FiberError4xxOmitsErrorTypeOnMetric(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/items/:id", func(c *fiber.Ctx) error { + return fiber.NewError(http.StatusNotFound, "not found") + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/items/missing", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusNotFound, resp.StatusCode, + "client must observe the 404 from Fiber's default error handler") + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + assert.EqualValues(t, 1, dp.Count) + + statusVal, ok := dp.Attributes.Value(attribute.Key("http.response.status_code")) + require.True(t, ok) + assert.EqualValues(t, http.StatusNotFound, statusVal.AsInt64(), + "status_code must reflect the *fiber.Error code, not the unwritten default") + + // Metric MUST omit error.type and error.type_original for 4xx. + _, hasErrTypeOnMetric := dp.Attributes.Value(attribute.Key("error.type")) + assert.False(t, hasErrTypeOnMetric, + "4xx must not set error.type on the metric per status-driven classification") + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrigOnMetric, + "error.type_original must NEVER appear on the metric") + + // Span MUST also omit error.type but carry error.type_original. + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Empty(t, getSpanAttr(spans[0], "error.type"), + "span error.type follows the same status-driven rule") + assert.Equal(t, "fiber.Error", + getSpanAttr(spans[0], "error.type_original"), + "span must preserve the originating *fiber.Error type") +} + +// TestWithTelemetry_FiberError400OmitsErrorTypeOnMetric asserts the same +// contract for 4xx bad-request errors raised via fiber.NewError, independently +// from the 404 case to catch regressions for either code path. +func TestWithTelemetry_FiberError400OmitsErrorTypeOnMetric(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Post("/api/validate", func(c *fiber.Ctx) error { + return fiber.NewError(http.StatusBadRequest, "invalid payload") + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodPost, "/api/validate", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + statusVal, ok := dp.Attributes.Value(attribute.Key("http.response.status_code")) + require.True(t, ok) + assert.EqualValues(t, http.StatusBadRequest, statusVal.AsInt64(), + "status_code must reflect fiber.NewError(400)") + + _, hasErrTypeOnMetric := dp.Attributes.Value(attribute.Key("error.type")) + assert.False(t, hasErrTypeOnMetric, + "400 must not set error.type on the metric") + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrigOnMetric) + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Empty(t, getSpanAttr(spans[0], "error.type")) + assert.Equal(t, "fiber.Error", + getSpanAttr(spans[0], "error.type_original")) +} + +// TestWithTelemetry_RecordsDurationOnFiberError5xx verifies that a 5xx +// fiber.NewError is reflected in status_code AND error.type on the duration +// metric using the status-driven numeric label, with the originating Go type +// preserved on the span as error.type_original (never on the metric). +func TestWithTelemetry_RecordsDurationOnFiberError5xx(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/down", func(c *fiber.Ctx) error { + return fiber.NewError(http.StatusBadGateway, "upstream gone") + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/down", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + statusVal, ok := dp.Attributes.Value(attribute.Key("http.response.status_code")) + require.True(t, ok) + assert.EqualValues(t, http.StatusBadGateway, statusVal.AsInt64()) + + errType, ok := attrValue(dp.Attributes, "error.type") + require.True(t, ok) + assert.Equal(t, "502", errType, + "metric error.type must be status-driven for low cardinality") + + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrigOnMetric, + "error.type_original must NEVER appear on the metric") + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Equal(t, "502", getSpanAttr(spans[0], "error.type")) + assert.Equal(t, "fiber.Error", getSpanAttr(spans[0], "error.type_original")) +} + +// TestWithTelemetry_FiberErrorAndSendStatusAreConsistent verifies the core +// motivation of the status-driven contract: a 503 raised via +// fiber.NewError(503) and a 503 written via c.SendStatus(503) MUST produce +// the same metric time series so alert rules of the form error_type=~"5.." +// aggregate reliably across both code paths. +func TestWithTelemetry_FiberErrorAndSendStatusAreConsistent(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/raise", func(c *fiber.Ctx) error { + return fiber.NewError(http.StatusServiceUnavailable, "down") + }) + app.Get("/api/send", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusServiceUnavailable) + }) + + r1, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/raise", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, r1.Body.Close()) }() + require.Equal(t, http.StatusServiceUnavailable, r1.StatusCode) + + r2, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/send", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, r2.Body.Close()) }() + require.Equal(t, http.StatusServiceUnavailable, r2.StatusCode) + + // Both requests share method+status+error.type but have different + // http.route values, so they remain two distinct time series, each with + // Count==1. Both MUST carry error.type="503" (status-driven) and no + // error.type_original on the metric. + rm := &metricdata.ResourceMetrics{} + require.NoError(t, reader.Collect(context.Background(), rm)) + + var points []metricdata.HistogramDataPoint[float64] + + for _, sm := range rm.ScopeMetrics { + for _, m := range sm.Metrics { + if m.Name != httpServerRequestDurationMetric { + continue + } + + h, ok := m.Data.(metricdata.Histogram[float64]) + require.True(t, ok) + points = append(points, h.DataPoints...) + } + } + + require.Len(t, points, 2, + "each route is a distinct series; both must record exactly one point") + + for _, dp := range points { + errType, ok := attrValue(dp.Attributes, "error.type") + require.True(t, ok, + "both fiber.NewError and SendStatus 5xx paths MUST set error.type") + assert.Equal(t, "503", errType, + "both paths MUST produce the same status-driven error.type label") + + _, hasOrig := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrig, + "error.type_original must NEVER appear on the metric") + + assert.EqualValues(t, 1, dp.Count) + } +} + +// TestWithTelemetry_GenericHandlerErrorStatusCodeIs500 verifies that when a +// handler returns a non-fiber error and no custom ErrorHandler has rewritten +// the status code by the time the metric is recorded, the metric reports +// status_code=500 with the status-driven error.type="500" (not the Go type +// name). The originating Go type identity is preserved on the span via +// error.type_original. +func TestWithTelemetry_GenericHandlerErrorStatusCodeIs500(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + // Use Fiber's default ErrorHandler (no override) so the response status + // is materialized AFTER the WithTelemetry middleware unwinds. + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/explode", func(c *fiber.Ctx) error { + return errors.New("boom") + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/explode", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode, + "Fiber's default error handler maps unknown errors to 500") + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + statusVal, ok := dp.Attributes.Value(attribute.Key("http.response.status_code")) + require.True(t, ok) + assert.EqualValues(t, http.StatusInternalServerError, statusVal.AsInt64(), + "generic handler error must record status_code=500 to match client view") + + errType, ok := attrValue(dp.Attributes, "error.type") + require.True(t, ok) + assert.Equal(t, "500", errType) + + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("error.type_original")) + assert.False(t, hasOrigOnMetric) + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Equal(t, "500", getSpanAttr(spans[0], "error.type")) + assert.Equal(t, "errors.errorString", + getSpanAttr(spans[0], "error.type_original")) +} + +// TestWithTelemetry_DoesNotRecordForExcludedRoute verifies that excluded +// routes bypass duration recording entirely. +func TestWithTelemetry_DoesNotRecordForExcludedRoute(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel, "/swagger")) + + app.Get("/swagger/index.html", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/swagger/index.html", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + assert.Nil(t, findDurationHistogram(t, reader), + "excluded route must not record http.server.request.duration") +} + +// TestWithTelemetry_NilTelemetryDoesNotRecord verifies the handler is safe and +// silent when no Telemetry is configured. +func TestWithTelemetry_NilTelemetryDoesNotRecord(t *testing.T) { + // Standalone reader without a real Telemetry attached - we still expect + // the metric to be absent because the middleware short-circuits. + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + t.Cleanup(func() { _ = mp.Shutdown(context.Background()) }) + + app := fiber.New() + mid := NewTelemetryMiddleware(nil) + app.Use(mid.WithTelemetry(nil)) + + app.Get("/ping", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/ping", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + rm := &metricdata.ResourceMetrics{} + require.NoError(t, reader.Collect(context.Background(), rm)) + for _, sm := range rm.ScopeMetrics { + for _, m := range sm.Metrics { + assert.NotEqual(t, httpServerRequestDurationMetric, m.Name, + "nil telemetry must not record duration") + } + } +} + +// TestWithTelemetry_NilMeterProviderDoesNotRecord verifies that recording is +// skipped when Telemetry is present but has no MeterProvider, while the +// request itself still completes successfully. +func TestWithTelemetry_NilMeterProviderDoesNotRecord(t *testing.T) { + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + t.Cleanup(func() { _ = mp.Shutdown(context.Background()) }) + + tel := &tracing.Telemetry{ + TelemetryConfig: tracing.TelemetryConfig{LibraryName: "test-library"}, + // MeterProvider intentionally nil. + MetricsFactory: metrics.NewNopFactory(), + } + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/no-mp", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/no-mp", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + assert.Nil(t, findDurationHistogram(t, reader), + "nil MeterProvider must not record duration") +} + +// TestWithTelemetry_NilMetricsFactoryDoesNotRecord verifies that recording is +// gated on MetricsFactory presence even when MeterProvider is configured. +// This matches the requirement that nil MetricsFactory must silently skip. +func TestWithTelemetry_NilMetricsFactoryDoesNotRecord(t *testing.T) { + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + t.Cleanup(func() { _ = mp.Shutdown(context.Background()) }) + + tel := &tracing.Telemetry{ + TelemetryConfig: tracing.TelemetryConfig{LibraryName: "test-library"}, + MeterProvider: mp, + // MetricsFactory intentionally nil. + } + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/no-factory", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/no-factory", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + assert.Nil(t, findDurationHistogram(t, reader), + "nil MetricsFactory must not record duration") +} + +// TestWithTelemetry_UnmatchedRouteOmitsHTTPRoute verifies the catch-all 404 +// guard: Fiber v2's default unmatched-path handler exposes Route().Path=="/", +// which would conflate scanner/404 traffic with the actual root handler in +// dashboards. The middleware MUST omit http.route entirely from both the span +// and the metric in that case, while still recording http.route="/" for a +// legitimately-registered root handler. +func TestWithTelemetry_UnmatchedRouteOmitsHTTPRoute(t *testing.T) { + t.Run("unmatched 404 omits http.route", func(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + // No routes registered: any request hits Fiber's catch-all 404. + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/not-registered", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusNotFound, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp, "duration must still be recorded for the 404") + assert.EqualValues(t, 1, dp.Count) + + _, hasRoute := dp.Attributes.Value(attribute.Key("http.route")) + assert.False(t, hasRoute, + "http.route must be absent on the metric for unmatched 404") + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Empty(t, getSpanAttr(spans[0], "http.route"), + "http.route must be absent on the span for unmatched 404") + }) + + t.Run("registered root handler retains http.route", func(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + app.Get("/", func(c *fiber.Ctx) error { return c.SendStatus(http.StatusOK) }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + route, ok := attrValue(dp.Attributes, "http.route") + require.True(t, ok, + "http.route must be present for a legitimately-registered root handler") + assert.Equal(t, "/", route) + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Equal(t, "/", getSpanAttr(spans[0], "http.route")) + }) +} + +// TestWithTelemetry_NormalizesUnknownMethodOnSpan verifies that an unknown +// HTTP verb is normalized to "_OTHER" on both the metric and the span, with +// the original verb preserved exclusively on the span's +// http.request.method_original attribute (never on the metric, to keep +// label cardinality bounded). +func TestWithTelemetry_NormalizesUnknownMethodOnSpan(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + // Fiber rejects unknown verbs at the framework boundary by default. + // Extend RequestMethods so the middleware actually sees PROPFIND. + app := fiber.New(fiber.Config{ + RequestMethods: append(fiber.DefaultMethods, "PROPFIND"), + }) + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Add("PROPFIND", "/dav/:resource", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest("PROPFIND", "/dav/foo", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + require.Equal(t, http.StatusOK, resp.StatusCode) + + // Metric assertions: normalized to "_OTHER", no method_original. + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + method, ok := attrValue(dp.Attributes, "http.request.method") + require.True(t, ok) + assert.Equal(t, "_OTHER", method) + + _, hasOrigOnMetric := dp.Attributes.Value(attribute.Key("http.request.method_original")) + assert.False(t, hasOrigOnMetric, "method_original must NEVER appear on the metric") + + // Span assertions: normalized method + original preserved. + spans := spanExp.GetSpans() + require.Len(t, spans, 1) + assert.Equal(t, "_OTHER", getSpanAttr(spans[0], "http.request.method")) + assert.Equal(t, "PROPFIND", getSpanAttr(spans[0], "http.request.method_original")) +} + +// TestWithTelemetry_KnownMethodHasNoOriginal verifies that a canonical method +// (GET) does not emit http.request.method_original anywhere. +func TestWithTelemetry_KnownMethodHasNoOriginal(t *testing.T) { + tel, reader, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/ping", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/ping", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + method, _ := attrValue(dp.Attributes, "http.request.method") + assert.Equal(t, "GET", method) + _, hasOrig := dp.Attributes.Value(attribute.Key("http.request.method_original")) + assert.False(t, hasOrig) + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + assert.Equal(t, "GET", getSpanAttr(spans[0], "http.request.method")) + assert.Empty(t, getSpanAttr(spans[0], "http.request.method_original")) +} + +// TestWithTelemetry_TruncatesLongUserAgent verifies the user_agent.original +// span attribute is capped at maxUserAgentAttrLen bytes regardless of input +// length, protecting trace storage from pathological clients. +func TestWithTelemetry_TruncatesLongUserAgent(t *testing.T) { + tel, _, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + app.Get("/x", func(c *fiber.Ctx) error { return c.SendStatus(http.StatusOK) }) + + longUA := strings.Repeat("a", 4000) + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.Header.Set("User-Agent", longUA) + + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + + ua := getSpanAttr(spans[0], "user_agent.original") + assert.Len(t, ua, maxUserAgentAttrLen) + assert.Equal(t, strings.Repeat("a", maxUserAgentAttrLen), ua) +} + +// TestWithTelemetry_TruncatesUserAgentAtRuneBoundary verifies that a multi-byte +// UTF-8 user-agent is truncated at a rune boundary, never mid-codepoint, so +// the resulting span attribute is always valid UTF-8 and never exceeds the +// byte cap. Uses a 3-byte rune ("€" = 0xE2 0x82 0xAC) repeated so the byte +// cap (256) falls strictly inside a codepoint; a naive byte slice would +// produce invalid UTF-8 at the boundary. +func TestWithTelemetry_TruncatesUserAgentAtRuneBoundary(t *testing.T) { + tel, _, spanExp := newTelemetryHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + app.Get("/x", func(c *fiber.Ctx) error { return c.SendStatus(http.StatusOK) }) + + longUA := strings.Repeat("€", 1000) // 3000 bytes + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.Header.Set("User-Agent", longUA) + + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + + spans := spanExp.GetSpans() + require.NotEmpty(t, spans) + + ua := getSpanAttr(spans[0], "user_agent.original") + assert.LessOrEqual(t, len(ua), maxUserAgentAttrLen, + "truncated user-agent must not exceed the byte cap") + assert.True(t, utf8.ValidString(ua), + "truncated user-agent must remain valid UTF-8 (never split a codepoint)") + // 256 / 3 = 85 complete "€" runes (255 bytes), which is the largest + // rune-aligned prefix that fits within the cap. + assert.Equal(t, strings.Repeat("€", 85), ua) +} + +// TestWithTelemetry_RecordsDurationWithTenantID verifies that the +// http.server.request.duration metric carries the tenant.id attribute when the +// request supplies the canonical X-Tenant-Id header. This is the contract that +// lets dashboards and alerts filter/group request volume and latency per +// tenant — replacing the spanmetrics calls_total{tenant_id} usage previously +// derived from the trace pipeline. +func TestWithTelemetry_RecordsDurationWithTenantID(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/orders", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, "/api/orders", nil) + req.Header.Set("X-Tenant-Id", "acme") + + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + assert.EqualValues(t, 1, dp.Count) + + tenantVal, ok := attrValue(dp.Attributes, "tenant.id") + require.True(t, ok, "tenant.id label must be present when X-Tenant-Id is supplied") + assert.Equal(t, "acme", tenantVal) +} + +// TestWithTelemetry_RecordsDurationWithBaggageTenantID is the regression guard +// for the metrics-only gap that survived PR #21: tenant.id propagated +// cross-service via OTel baggage (PR #20) reached spans and logs but NOT the +// http.server.request.duration metric, because the metric path read the +// AttrBag only. The request below carries NO X-Tenant-Id header on the local +// hop; the tenant is present solely in the inbound baggage, exactly as it +// arrives at a downstream midaz plugin. The duration metric must still carry +// tenant.id, matching the trace/log pipelines. +func TestWithTelemetry_RecordsDurationWithBaggageTenantID(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + + // Seed the request UserContext with baggage-propagated tenant.id before the + // telemetry middleware runs, mirroring an upstream service that injected + // tenant.id into the OTel baggage rather than the X-Tenant-Id header. + app.Use(func(c *fiber.Ctx) error { + c.SetUserContext(ctxWithBaggageTenant(t, "acme")) + return c.Next() + }) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/orders", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + // No X-Tenant-Id header: the only tenant source is the inbound baggage. + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/orders", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + assert.EqualValues(t, 1, dp.Count) + + tenantVal, ok := attrValue(dp.Attributes, "tenant.id") + require.True(t, ok, "tenant.id label must be present when tenant.id arrives via OTel baggage") + assert.Equal(t, "acme", tenantVal) +} + +// TestWithTelemetry_RecordsDurationWithoutTenantID verifies that the +// tenant.id attribute is omitted entirely when the request does not carry the +// canonical header. Series for non-tenant traffic (probes, internal callers) +// must not gain an empty label that would split the time series. +func TestWithTelemetry_RecordsDurationWithoutTenantID(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/orders", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/api/orders", nil)) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + _, ok := attrValue(dp.Attributes, "tenant.id") + assert.False(t, ok, "tenant.id must be absent when no X-Tenant-Id header was supplied") +} + +// TestWithTelemetry_RecordsDurationDropsOversizedTenantID verifies that a +// tenant value exceeding the 128-byte cap enforced by ResolveTenantIDFromHTTP +// is dropped silently and does NOT become a metric label. This is the +// cardinality safety guarantee: an attacker that floods X-Tenant-Id with +// random oversized values cannot inflate the Prometheus series set. +func TestWithTelemetry_RecordsDurationDropsOversizedTenantID(t *testing.T) { + tel, reader := newMetricsHarness(t) + + app := fiber.New() + mid := NewTelemetryMiddleware(tel) + app.Use(mid.WithTelemetry(tel)) + + app.Get("/api/orders", func(c *fiber.Ctx) error { + return c.SendStatus(http.StatusOK) + }) + + req := httptest.NewRequest(http.MethodGet, "/api/orders", nil) + req.Header.Set("X-Tenant-Id", strings.Repeat("a", constant.MaxTenantIDLen+1)) + + resp, err := app.Test(req) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + assert.Equal(t, http.StatusOK, resp.StatusCode) + + dp := findDurationHistogram(t, reader) + require.NotNil(t, dp) + + _, ok := attrValue(dp.Attributes, "tenant.id") + assert.False(t, ok, "tenant.id must be absent when the header value exceeds the cap") +} diff --git a/middleware/telemetry_test.go b/middleware/telemetry_test.go index 991f6ac..ccb39f0 100644 --- a/middleware/telemetry_test.go +++ b/middleware/telemetry_test.go @@ -155,8 +155,7 @@ func TestWithTelemetry(t *testing.T) { return tt.setupHandler(c) }) - req, err := http.NewRequest(tt.method, tt.path, nil) - require.NoError(t, err) + req := httptest.NewRequest(tt.method, tt.path, nil) if tt.traceparent != "" { req.Header.Set("traceparent", tt.traceparent) @@ -269,8 +268,7 @@ func TestWithTelemetryExcludedRoutes(t *testing.T) { return c.SendStatus(http.StatusOK) }) - req, err := http.NewRequest(tt.method, tt.path, nil) - require.NoError(t, err) + req := httptest.NewRequest(tt.method, tt.path, nil) resp, err := app.Test(req) require.NoError(t, err) @@ -590,8 +588,7 @@ func TestExtractHTTPContext(t *testing.T) { return c.SendStatus(http.StatusOK) }) - req1, err := http.NewRequest("GET", "/test", nil) - require.NoError(t, err) + req1 := httptest.NewRequest("GET", "/test", nil) req1.Header.Set("traceparent", traceparent) resp1, err := app.Test(req1) @@ -599,8 +596,7 @@ func TestExtractHTTPContext(t *testing.T) { defer func() { require.NoError(t, resp1.Body.Close()) }() assert.Equal(t, http.StatusOK, resp1.StatusCode) - req2, err := http.NewRequest("GET", "/test", nil) - require.NoError(t, err) + req2 := httptest.NewRequest("GET", "/test", nil) resp2, err := app.Test(req2) require.NoError(t, err) @@ -672,8 +668,7 @@ func TestWithTelemetryConditionalTracePropagation(t *testing.T) { return c.SendStatus(http.StatusOK) }) - req, err := http.NewRequest("GET", "/test", nil) - require.NoError(t, err) + req := httptest.NewRequest("GET", "/test", nil) if tt.userAgent != "" { req.Header.Set("User-Agent", tt.userAgent) diff --git a/middleware/tenant.go b/middleware/tenant.go new file mode 100644 index 0000000..6a6de26 --- /dev/null +++ b/middleware/tenant.go @@ -0,0 +1,122 @@ +package middleware + +import ( + "context" + + observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" + "github.com/gofiber/fiber/v2" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/baggage" + "google.golang.org/grpc/metadata" +) + +// ResolveTenantIDFromHTTP returns the tenant identifier carried by the +// canonical X-Tenant-Id header, normalized for safe inclusion in telemetry. +// Returns an empty string when the header is absent, empty after trimming, or +// longer than MaxTenantIDLen bytes. The header is trusted only as an +// observability hint: callers MUST authenticate the tenant separately and +// override via observability.ContextWithSpanAttributes when the real value +// differs from the header. +func ResolveTenantIDFromHTTP(c *fiber.Ctx) string { + if c == nil { + return "" + } + + return sanitizeTenantID(c.Get(constant.HeaderTenantID)) +} + +// ResolveTenantIDFromGRPC returns the tenant identifier carried by the +// canonical tenant-id gRPC metadata key, normalized for safe inclusion in +// telemetry. Returns an empty string when the metadata is absent, empty, or +// longer than MaxTenantIDLen bytes. Same trust caveat as the HTTP variant. +func ResolveTenantIDFromGRPC(ctx context.Context) string { + if ctx == nil { + return "" + } + + md, ok := metadata.FromIncomingContext(ctx) + if !ok || md == nil { + return "" + } + + vals := md.Get(constant.MetadataTenantID) + if len(vals) == 0 { + return "" + } + + return sanitizeTenantID(vals[0]) +} + +// sanitizeTenantID trims whitespace and control bytes from raw, then enforces +// the MaxTenantIDLen cap. Returns "" for any value that fails normalization or +// exceeds the cap, so callers can use it as a presence check. +func sanitizeTenantID(raw string) string { + value := normalizeRequestID(raw) + if isNilOrEmptyString(&value) { + return "" + } + + if len(value) > constant.MaxTenantIDLen { + return "" + } + + return value +} + +// tenantIDFromAttrBag returns the tenant.id stored in the request AttrBag, or +// "" when absent or when the stored value would breach the telemetry +// cardinality cap. Because ContextWithSpanAttributes deduplicates by key +// (last-wins), this never returns a stale value when a later layer overrode +// the tenant — the bag carries a single entry per key. +// +// The result is funneled through sanitizeTenantID so the constraints applied +// at ingestion (MaxTenantIDLen cap, control-byte stripping) also hold for +// values injected directly via observability.ContextWithSpanAttributes (for +// example, a handler that wants to override the header-supplied tenant with +// one resolved from a JWT). Without this defense-in-depth step a caller +// could silently bypass the cap and inflate log fields and metric label +// cardinality. +func tenantIDFromAttrBag(ctx context.Context) string { + for _, attr := range observability.AttributesFromContext(ctx) { + if attr.Key == attribute.Key(constant.AttrKeyTenantID) { + return sanitizeTenantID(attr.Value.AsString()) + } + } + + return "" +} + +// tenantIDFromBaggage returns the tenant.id carried by the standard OTel +// baggage (written upstream by lib-commons), normalized through +// sanitizeTenantID so the same cardinality guards as every other ingestion +// path apply. Returns "" when the member is absent. +func tenantIDFromBaggage(ctx context.Context) string { + if ctx == nil { + return "" + } + + return sanitizeTenantID(baggage.FromContext(ctx).Member(constant.AttrKeyTenantID).Value()) +} + +// resolveTenantIDForTelemetry resolves the tenant.id to attach to a request's +// telemetry (request logger AND the http.server.request.duration metric) using +// the same base→override precedence as the span processor: the standard OTel +// baggage is the base source, and the request AttrBag (header/JWT-resolved) +// overrides it when present. Returns "" when neither source carries a usable +// value. +// +// Sharing this resolver across logs, spans, and metrics is deliberate: tenant +// .id frequently arrives cross-service via OTel baggage (see PR #20) rather +// than the canonical X-Tenant-Id header on the local hop. The metric path +// previously read the AttrBag only (tenantIDFromAttrBag), so baggage-propagated +// tenants produced an empty tenant_id label on the duration histogram even +// though spans and logs carried the value. Routing the metric through this +// resolver keeps the tenant.id label consistent with the trace/log pipelines. +func resolveTenantIDForTelemetry(ctx context.Context) string { + if tenantID := tenantIDFromAttrBag(ctx); tenantID != "" { + return tenantID + } + + return tenantIDFromBaggage(ctx) +} diff --git a/middleware/tenant_test.go b/middleware/tenant_test.go new file mode 100644 index 0000000..9b1e712 --- /dev/null +++ b/middleware/tenant_test.go @@ -0,0 +1,214 @@ +//go:build unit + +package middleware + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" + "github.com/gofiber/fiber/v2" + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/baggage" + "google.golang.org/grpc/metadata" +) + +// ctxWithBaggageTenant returns a context carrying tenant.id in the standard +// OTel baggage, the same way lib-commons propagates it upstream. +func ctxWithBaggageTenant(t *testing.T, value string) context.Context { + t.Helper() + + m, err := baggage.NewMember(constant.AttrKeyTenantID, value) + assert.NoError(t, err) + + b, err := baggage.New(m) + assert.NoError(t, err) + + return baggage.ContextWithBaggage(context.Background(), b) +} + +func TestResolveTenantIDFromHTTP(t *testing.T) { + cases := []struct { + name string + header map[string]string + want string + }{ + {name: "canonical header present", header: map[string]string{"X-Tenant-Id": "acme"}, want: "acme"}, + {name: "header absent", header: nil, want: ""}, + {name: "header empty after trim", header: map[string]string{"X-Tenant-Id": " "}, want: ""}, + {name: "header literal null is dropped", header: map[string]string{"X-Tenant-Id": "null"}, want: ""}, + {name: "header at max length is kept", header: map[string]string{"X-Tenant-Id": strings.Repeat("a", constant.MaxTenantIDLen)}, want: strings.Repeat("a", constant.MaxTenantIDLen)}, + {name: "header above max length is dropped", header: map[string]string{"X-Tenant-Id": strings.Repeat("a", constant.MaxTenantIDLen+1)}, want: ""}, + {name: "control chars are stripped", header: map[string]string{"X-Tenant-Id": "acme\r\n"}, want: "acme"}, + {name: "non-canonical aliases are ignored", header: map[string]string{"tenant-id": "acme"}, want: ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + app := fiber.New() + + var got string + app.Get("/", func(c *fiber.Ctx) error { + got = ResolveTenantIDFromHTTP(c) + return nil + }) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + for k, v := range tc.header { + req.Header.Set(k, v) + } + + _, err := app.Test(req) + assert.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestResolveTenantIDFromHTTPNilCtx(t *testing.T) { + assert.Equal(t, "", ResolveTenantIDFromHTTP(nil)) +} + +func TestResolveTenantIDFromGRPC(t *testing.T) { + cases := []struct { + name string + md metadata.MD + want string + }{ + {name: "canonical metadata present", md: metadata.Pairs("tenant-id", "acme"), want: "acme"}, + {name: "metadata absent", md: nil, want: ""}, + {name: "metadata empty", md: metadata.Pairs("tenant-id", " "), want: ""}, + {name: "metadata above max length is dropped", md: metadata.Pairs("tenant-id", strings.Repeat("a", constant.MaxTenantIDLen+1)), want: ""}, + {name: "non-canonical aliases are ignored", md: metadata.Pairs("tenant_id", "acme"), want: ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + if tc.md != nil { + ctx = metadata.NewIncomingContext(ctx, tc.md) + } + + assert.Equal(t, tc.want, ResolveTenantIDFromGRPC(ctx)) + }) + } +} + +func TestResolveTenantIDFromGRPCNilCtx(t *testing.T) { + assert.Equal(t, "", ResolveTenantIDFromGRPC(nil)) +} + +func TestTenantIDFromAttrBag_OverrideWins(t *testing.T) { + // Simulates middleware writing the header tenant, then a downstream auth + // layer overriding with the JWT-validated tenant. Last-wins must surface + // the override, not the original. + ctx := observability.ContextWithSpanAttributes(context.Background(), + attribute.String(constant.AttrKeyTenantID, "from-header"), + ) + ctx = observability.ContextWithSpanAttributes(ctx, + attribute.String(constant.AttrKeyTenantID, "from-jwt"), + ) + + assert.Equal(t, "from-jwt", tenantIDFromAttrBag(ctx)) +} + +func TestTenantIDFromAttrBag_AbsentReturnsEmpty(t *testing.T) { + assert.Equal(t, "", tenantIDFromAttrBag(context.Background())) +} + +func TestTenantIDFromBaggage(t *testing.T) { + t.Run("member present", func(t *testing.T) { + ctx := ctxWithBaggageTenant(t, "acme") + assert.Equal(t, "acme", tenantIDFromBaggage(ctx)) + }) + + t.Run("member absent", func(t *testing.T) { + assert.Equal(t, "", tenantIDFromBaggage(context.Background())) + }) + + t.Run("nil ctx", func(t *testing.T) { + assert.Equal(t, "", tenantIDFromBaggage(nil)) + }) + + t.Run("oversized baggage value is dropped", func(t *testing.T) { + ctx := ctxWithBaggageTenant(t, strings.Repeat("a", constant.MaxTenantIDLen+1)) + assert.Equal(t, "", tenantIDFromBaggage(ctx)) + }) +} + +// TestResolveTenantIDForTelemetry exercises the base→override precedence shared +// by the logging middleware AND the http.server.request.duration metric: the +// OTel baggage is the base source and the request AttrBag (header/JWT-resolved) +// overrides it when present. +func TestResolveTenantIDForTelemetry(t *testing.T) { + t.Run("baggage only is used as base", func(t *testing.T) { + ctx := ctxWithBaggageTenant(t, "from-baggage") + assert.Equal(t, "from-baggage", resolveTenantIDForTelemetry(ctx)) + }) + + t.Run("attrbag overrides baggage", func(t *testing.T) { + ctx := ctxWithBaggageTenant(t, "from-baggage") + ctx = observability.ContextWithSpanAttributes(ctx, + attribute.String(constant.AttrKeyTenantID, "from-header"), + ) + assert.Equal(t, "from-header", resolveTenantIDForTelemetry(ctx)) + }) + + t.Run("attrbag only", func(t *testing.T) { + ctx := observability.ContextWithSpanAttributes(context.Background(), + attribute.String(constant.AttrKeyTenantID, "from-header"), + ) + assert.Equal(t, "from-header", resolveTenantIDForTelemetry(ctx)) + }) + + t.Run("neither source", func(t *testing.T) { + assert.Equal(t, "", resolveTenantIDForTelemetry(context.Background())) + }) +} + +func TestRequestAttributes_ReturnsCopy(t *testing.T) { + ctx := observability.ContextWithSpanAttributes(context.Background(), + attribute.String(constant.AttrKeyTenantID, "acme"), + attribute.String("app.request.request_id", "req-1"), + ) + + attrs := RequestAttributes(ctx) + assert.Len(t, attrs, 2) + + // Mutating the returned slice must not affect the bag in ctx. + attrs[0] = attribute.String("tampered", "x") + again := RequestAttributes(ctx) + assert.Equal(t, attribute.Key(constant.AttrKeyTenantID), again[0].Key) +} + +// TestTenantIDFromAttrBag_DropsOversizedFromContext guards the defense-in-depth +// step that re-applies sanitizeTenantID to values pulled from the AttrBag. +// 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 value injected this way would +// bypass the 128-byte cap enforced at HTTP/gRPC ingestion and inflate log +// fields and metric label cardinality downstream. +func TestTenantIDFromAttrBag_DropsOversizedFromContext(t *testing.T) { + ctx := observability.ContextWithSpanAttributes(context.Background(), + attribute.String(constant.AttrKeyTenantID, strings.Repeat("a", constant.MaxTenantIDLen+1)), + ) + + assert.Equal(t, "", tenantIDFromAttrBag(ctx)) +} + +// TestTenantIDFromAttrBag_StripsControlBytesFromContext mirrors the previous +// test for the other sanitization rule: control characters (CRLF, NUL) must +// never reach a log field or metric label even when the caller injected them +// directly into the bag. +func TestTenantIDFromAttrBag_StripsControlBytesFromContext(t *testing.T) { + ctx := observability.ContextWithSpanAttributes(context.Background(), + attribute.String(constant.AttrKeyTenantID, "acme\r\n"), + ) + + assert.Equal(t, "acme", tenantIDFromAttrBag(ctx)) +} diff --git a/observability.go b/observability.go index 7cf072c..7b3c5f3 100644 --- a/observability.go +++ b/observability.go @@ -236,8 +236,12 @@ func newDefaultTrackingComponents() TrackingComponents { // ---- Attribute Bag (request-wide span attributes) ---- -// ContextWithSpanAttributes appends one or more attributes to the request's AttrBag. -// Call this once at the ingress (HTTP/gRPC middleware) and avoid per-layer duplication. +// ContextWithSpanAttributes merges one or more attributes into the request's +// AttrBag using last-wins semantics: if a key is already present, its value is +// replaced in place; otherwise the attribute is appended. Call this at the +// ingress (HTTP/gRPC middleware) for shared identifiers and again from a +// downstream layer (e.g. after authentication resolves the real tenant) to +// override the value without producing duplicates in the bag. // Example keys: tenant.id, enduser.id, request.route, region, plan. func ContextWithSpanAttributes(ctx context.Context, kv ...attribute.KeyValue) context.Context { if ctx == nil { @@ -249,12 +253,37 @@ func ContextWithSpanAttributes(ctx context.Context, kv ...attribute.KeyValue) co } values := cloneContextValues(ctx) - // Append to the cloned (independent) slice. - values.AttrBag = append(values.AttrBag, kv...) + values.AttrBag = mergeAttrBagLastWins(values.AttrBag, kv) return context.WithValue(ctx, ContextKey, values) } +// mergeAttrBagLastWins returns a slice where every key from incoming overrides +// the matching key in bag (in place, preserving the original position), and +// keys not yet present are appended. The bag input is treated as already +// independent (cloneContextValues deep-copies it before calling), so it is +// safe to mutate. +func mergeAttrBagLastWins(bag, incoming []attribute.KeyValue) []attribute.KeyValue { + for _, attr := range incoming { + replaced := false + + for i := range bag { + if bag[i].Key == attr.Key { + bag[i] = attr + replaced = true + + break + } + } + + if !replaced { + bag = append(bag, attr) + } + } + + return bag +} + // AttributesFromContext returns a shallow copy of the AttrBag slice, safe to reuse by processors. func AttributesFromContext(ctx context.Context) []attribute.KeyValue { if ctx == nil { diff --git a/observability_test.go b/observability_test.go index 726c39b..04b365e 100644 --- a/observability_test.go +++ b/observability_test.go @@ -28,3 +28,36 @@ func TestReplaceAttributesNilContext(t *testing.T) { attrs := AttributesFromContext(ctx) assert.Equal(t, []attribute.KeyValue{attribute.String("service.name", "new")}, attrs) } + +func TestContextWithSpanAttributes_DedupesByKey(t *testing.T) { + // Writing the same key twice must collapse to a single entry with the + // most recent value (last-wins). This is the contract that lets a + // downstream layer (e.g. JWT auth) override a value provided by the + // ingress middleware without bloating the bag or producing stale reads. + ctx := ContextWithSpanAttributes(context.Background(), + attribute.String("tenant.id", "from-header"), + ) + ctx = ContextWithSpanAttributes(ctx, + attribute.String("tenant.id", "from-jwt"), + ) + + attrs := AttributesFromContext(ctx) + assert.Equal(t, []attribute.KeyValue{attribute.String("tenant.id", "from-jwt")}, attrs) +} + +func TestContextWithSpanAttributes_PreservesOrderForNewKeys(t *testing.T) { + ctx := ContextWithSpanAttributes(context.Background(), + attribute.String("app.request.request_id", "req-1"), + ) + ctx = ContextWithSpanAttributes(ctx, + attribute.String("tenant.id", "acme"), + ) + ctx = ContextWithSpanAttributes(ctx, + attribute.String("app.request.request_id", "req-1"), // duplicate, must not append + ) + + attrs := AttributesFromContext(ctx) + assert.Len(t, attrs, 2) + assert.Equal(t, attribute.Key("app.request.request_id"), attrs[0].Key) + assert.Equal(t, attribute.Key("tenant.id"), attrs[1].Key) +} diff --git a/tracing/otel.go b/tracing/otel.go index 2c443e9..d1231e5 100644 --- a/tracing/otel.go +++ b/tracing/otel.go @@ -100,6 +100,7 @@ func NewTelemetry(cfg TelemetryConfig) (*Telemetry, error) { } normalizeEndpoint(&cfg) + normalizeEndpointEnvVars(cfg.Logger) if cfg.EnableTelemetry && strings.TrimSpace(cfg.CollectorExporterEndpoint) == "" { return handleEmptyEndpoint(cfg) @@ -150,7 +151,35 @@ func normalizeEndpoint(cfg *TelemetryConfig) { case strings.HasPrefix(ep, "https://"): cfg.CollectorExporterEndpoint = strings.TrimPrefix(ep, "https://") default: + // No scheme — assume insecure (common in k8s internal comms). + // Persist the trimmed value back so leading/trailing whitespace is dropped. cfg.CollectorExporterEndpoint = ep + cfg.InsecureExporter = true + } +} + +// normalizeEndpointEnvVars ensures OTEL exporter endpoint environment variables +// contain a URL scheme. The OTEL SDK's envconfig reads these via url.Parse(), +// which fails on bare "host:port" values. Adding "http://" prevents noisy +// "parse url" errors from the SDK's internal logger. +func normalizeEndpointEnvVars(logger log.Logger) { + for _, key := range []string{ + "OTEL_EXPORTER_OTLP_ENDPOINT", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT", + } { + v := strings.TrimSpace(os.Getenv(key)) + if v == "" || strings.HasPrefix(v, "http://") || strings.HasPrefix(v, "https://") { + continue + } + + // Failure here means the SDK will later choke on the bare host:port value, + // so surface it rather than swallowing it silently. + if err := os.Setenv(key, "http://"+v); err != nil && logger != nil { + logger.Log(context.Background(), log.LevelWarn, + "failed to normalize OTEL endpoint env var", + log.String("key", key), log.Err(err)) + } } } diff --git a/tracing/otel_test.go b/tracing/otel_test.go index f96114a..2592f06 100644 --- a/tracing/otel_test.go +++ b/tracing/otel_test.go @@ -9,11 +9,13 @@ import ( "testing" observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" "github.com/LerianStudio/lib-observability/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" sdktrace "go.opentelemetry.io/otel/sdk/trace" @@ -208,10 +210,10 @@ func TestNewTelemetry_EndpointNormalization(t *testing.T) { wantInsecure: false, }, { - name: "no scheme preserves insecure default", + name: "no scheme infers insecure (k8s internal comms)", endpoint: "otel-collector:4317", wantEndpoint: "otel-collector:4317", - wantInsecure: false, + wantInsecure: true, }, { name: "https with explicit insecure override preserved", @@ -1130,6 +1132,89 @@ func TestAttrBagSpanProcessor_OnStart_WithContextAttributes(t *testing.T) { assert.True(t, found, "span must contain app.request.id=r1 from context bag") } +// ctxWithBaggageTenant returns a context carrying tenant.id in the standard +// OTel baggage, mirroring how lib-commons propagates it across services. +func ctxWithBaggageTenant(t *testing.T, value string) context.Context { + t.Helper() + + m, err := baggage.NewMember(constant.AttrKeyTenantID, value) + require.NoError(t, err) + + b, err := baggage.New(m) + require.NoError(t, err) + + return baggage.ContextWithBaggage(context.Background(), b) +} + +func TestAttrBagSpanProcessor_OnStart_TenantFromBaggage(t *testing.T) { + t.Parallel() + + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(AttrBagSpanProcessor{}), + sdktrace.WithSyncer(exporter), + ) + t.Cleanup(func() { _ = tp.Shutdown(context.Background()) }) + + ctx := ctxWithBaggageTenant(t, "acme") + _, span := tp.Tracer("test").Start(ctx, "op") + span.End() + + spans := exporter.GetSpans() + require.Len(t, spans, 1) + assert.Equal(t, "acme", attrsToMap(spans[0].Attributes)[constant.AttrKeyTenantID], + "span must inherit tenant.id from the standard OTel baggage") +} + +func TestAttrBagSpanProcessor_OnStart_AttrBagOverridesBaggage(t *testing.T) { + t.Parallel() + + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(AttrBagSpanProcessor{}), + sdktrace.WithSyncer(exporter), + ) + t.Cleanup(func() { _ = tp.Shutdown(context.Background()) }) + + // Baggage carries the propagated base value; the request AttrBag carries the + // authoritative (e.g. JWT-resolved) value and must win via last-wins. + ctx := ctxWithBaggageTenant(t, "from-baggage") + ctx = observability.ContextWithSpanAttributes(ctx, + attribute.String(constant.AttrKeyTenantID, "from-attrbag")) + + _, span := tp.Tracer("test").Start(ctx, "op") + span.End() + + spans := exporter.GetSpans() + require.Len(t, spans, 1) + assert.Equal(t, "from-attrbag", attrsToMap(spans[0].Attributes)[constant.AttrKeyTenantID], + "request AttrBag must override the baggage-derived tenant.id") +} + +func TestRedactingAttrBagSpanProcessor_OnStart_TenantFromBaggage(t *testing.T) { + t.Parallel() + + p := RedactingAttrBagSpanProcessor{Redactor: NewDefaultRedactor()} + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(p), + sdktrace.WithSyncer(exporter), + ) + t.Cleanup(func() { _ = tp.Shutdown(context.Background()) }) + + ctx := ctxWithBaggageTenant(t, "from-baggage") + ctx = observability.ContextWithSpanAttributes(ctx, + attribute.String(constant.AttrKeyTenantID, "from-attrbag")) + + _, span := tp.Tracer("test").Start(ctx, "op") + span.End() + + spans := exporter.GetSpans() + require.Len(t, spans, 1) + assert.Equal(t, "from-attrbag", attrsToMap(spans[0].Attributes)[constant.AttrKeyTenantID], + "request AttrBag must override baggage even through the redacting processor") +} + func TestRedactingAttrBagSpanProcessor_OnStart_WithContextAttributes(t *testing.T) { t.Parallel() diff --git a/tracing/processor.go b/tracing/processor.go index 826acf7..e5bceac 100644 --- a/tracing/processor.go +++ b/tracing/processor.go @@ -5,7 +5,9 @@ import ( "strings" observability "github.com/LerianStudio/lib-observability" + constant "github.com/LerianStudio/lib-observability/constants" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/baggage" sdktrace "go.opentelemetry.io/otel/sdk/trace" ) @@ -20,24 +22,82 @@ type RedactingAttrBagSpanProcessor struct { } // OnStart applies request-scoped context attributes to newly started spans. +// +// Sources are applied base-first, override-last so SetAttributes' last-wins +// semantics keep the precedence correct per key: +// 1. standard OTel baggage (e.g. tenant.id propagated by lib-commons) — base; +// 2. the request AttrBag (header/JWT-resolved values) — override. func (AttrBagSpanProcessor) OnStart(ctx context.Context, s sdktrace.ReadWriteSpan) { + if base := baggageBaseAttributes(ctx); len(base) > 0 { + s.SetAttributes(base...) + } + if kv := observability.AttributesFromContext(ctx); len(kv) > 0 { s.SetAttributes(kv...) } } // OnStart applies request-scoped attributes and redacts sensitive values before writing to span. +// +// Like AttrBagSpanProcessor.OnStart it layers the standard OTel baggage as the +// base source and the request AttrBag as the override, redacting each source by +// key before writing so the last-wins precedence survives redaction. func (p RedactingAttrBagSpanProcessor) OnStart(ctx context.Context, s sdktrace.ReadWriteSpan) { + base := baggageBaseAttributes(ctx) kv := observability.AttributesFromContext(ctx) - if len(kv) == 0 { + + if len(base) == 0 && len(kv) == 0 { return } if p.Redactor != nil { + base = redactAttributesByKey(base, p.Redactor) kv = redactAttributesByKey(kv, p.Redactor) } - s.SetAttributes(kv...) + if len(base) > 0 { + s.SetAttributes(base...) + } + + if len(kv) > 0 { + s.SetAttributes(kv...) + } +} + +// baggageBaseAttributes extracts the shared identifiers carried by the standard +// OTel baggage (currently tenant.id, written upstream by lib-commons) so they +// can seed a span before the request AttrBag overrides them. Returns nil when +// no recognized member is present. +func baggageBaseAttributes(ctx context.Context) []attribute.KeyValue { + if v := sanitizeTenantFromBaggage(baggage.FromContext(ctx).Member(constant.AttrKeyTenantID).Value()); v != "" { + return []attribute.KeyValue{attribute.String(constant.AttrKeyTenantID, v)} + } + + return nil +} + +// sanitizeTenantFromBaggage applies the same cardinality guards used on the +// logging path (middleware.sanitizeTenantID) to raw baggage values: strip the +// log-injection control bytes (\r \n \x00), trim surrounding whitespace, and +// drop values exceeding MaxTenantIDLen. Mirroring those rules here keeps span +// and log tenant.id identical, preventing cardinality drift between the two. +// It is duplicated rather than imported because middleware depends on tracing, +// so importing middleware here would create a cycle. +func sanitizeTenantFromBaggage(raw string) string { + replacer := strings.NewReplacer("\r", "", "\n", "", "\x00", "") + v := strings.TrimSpace(replacer.Replace(raw)) + + // Treat literal "null"/"nil" (case-insensitive) as empty, mirroring the + // middleware path, to handle JSON null serialization artifacts. + if strings.EqualFold(v, "null") || strings.EqualFold(v, "nil") { + return "" + } + + if v == "" || len(v) > constant.MaxTenantIDLen { + return "" + } + + return v } // OnEnd is a no-op for this processor. @@ -82,7 +142,7 @@ func redactAttributesByKey(attrs []attribute.KeyValue, redactor *Redactor) []att case RedactionDrop: continue case RedactionHash: - redacted = append(redacted, attribute.String(string(attr.Key), redactor.hashString(attr.Value.Emit()))) + redacted = append(redacted, attribute.String(string(attr.Key), redactor.hashString(attr.Value.String()))) default: redacted = append(redacted, attribute.String(string(attr.Key), redactor.maskValue)) }