feat(metrics): native RED/runtime metrics + DB/cache/queue instrumentation helpers#36
Merged
Merged
Conversation
…ation helpers Standardize transversal metrics emission and add thin auto-instrumentation helpers so services stop hand-writing infra spans and stop inventing per-app metric names. Native metrics (automatic when middleware registered + telemetry on): - rpc.server.duration / rpc.client.duration (gRPC RED; client interceptor is new) - http.server.active_requests (in-flight UpDownCounter, saturation signal) - Go runtime metrics (go.*) via contrib/runtime, opt-in EnableRuntimeMetrics Auto-instrumentation helpers (lib exposes helper; app owns the connection): - sqlobs: db.client.operation.duration for PostgreSQL + MySQL/MariaDB (otelsql). Query text disabled (DisableQuery); instrument each *sql.DB before dbresolver. - redisobs: same metric for Redis + Valkey (redisotel; WithDBStatement(false)). - messagingobs: messaging.client.operation.duration / messaging.process.duration for RabbitMQ, hand-rolled over existing queue trace-context helpers (no amqp dep in the lib; destination.template only, never routing key / message id). All instruments built once, no-op safe when telemetry disabled, unit=seconds, low-cardinality labels only (no query text / keys / IDs / PII — covered by tests). Metric contract documented in docs/metrics-contract.md. Mongo (otelmongo) deferred: v2 has no tagged release (would drag otel core off v1.44.0), v1 is deprecated. Tracked in docs/CHANGELOG-native-metrics.md backlog. Deps: XSAM/otelsql v0.43.0, redisotel/v9 v9.17.2, contrib/instrumentation/runtime v0.69.0 — all resolve to otel core v1.44.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> X-Lerian-Ref: 0x1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesObservability instrumentation
Sequence Diagram(s)sequenceDiagram
participant Publisher
participant RabbitMQHeaders
participant Consumer
Publisher->>Publisher: Start producer span
Publisher->>RabbitMQHeaders: Inject trace context
RabbitMQHeaders->>Consumer: Deliver inbound headers
Consumer->>Consumer: Extract trace context and start consumer span
Consumer->>Consumer: Record processing duration and end span
✨ Finishing Touches✨ Simplify code
Comment |
lerian-studio
added
size/XL
Contributor
🔒 Security Scan Results —
|
| Stage | Status | Blocking? |
|---|---|---|
| Filesystem Scan | ✅ Clean | — |
| Docker Image Scan | ➖ Skipped | — |
| Docker Hub Health Score | ➖ Skipped | — |
| Pre-release Version Check | ✅ Clean | — |
Trivy
Filesystem Scan
✅ No vulnerabilities or secrets found.
Pre-release Version Check
✅ No unstable version pins found.
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
Contributor
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 84.8% ✅ PASS |
| Threshold | 80% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/lib-observability/v2/assert |
97.9% |
github.com/LerianStudio/lib-observability/v2/constants |
83.3% |
github.com/LerianStudio/lib-observability/v2/grpcmiddleware |
79.0% |
github.com/LerianStudio/lib-observability/v2/log |
94.9% |
github.com/LerianStudio/lib-observability/v2/messagingobs |
89.4% |
github.com/LerianStudio/lib-observability/v2/metrics |
91.4% |
github.com/LerianStudio/lib-observability/v2/middleware |
72.6% |
github.com/LerianStudio/lib-observability/v2/redaction |
95.8% |
github.com/LerianStudio/lib-observability/v2/redisobs |
63.1% |
github.com/LerianStudio/lib-observability/v2/runtime |
80.4% |
github.com/LerianStudio/lib-observability/v2/sqlobs |
64.2% |
github.com/LerianStudio/lib-observability/v2/telemetrycore |
84.2% |
github.com/LerianStudio/lib-observability/v2/tracing |
87.2% |
github.com/LerianStudio/lib-observability/v2/zap |
96.0% |
github.com/LerianStudio/lib-observability/v2 |
91.5% |
Generated by Go PR Analysis workflow
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
middleware/telemetry.go (1)
247-250: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated telemetry-bootstrap resolution into a helper.
The
bootstrapTelemetry := tl; if bootstrapTelemetry == nil && tm != nil { bootstrapTelemetry = tm.Telemetry }snippet is duplicated identically acrossWithTelemetry,WithTelemetryInterceptor, andUnaryClientInterceptor, all touched by this PR. A small shared helper would remove the triplication and keep the three construction-time gates from silently drifting apart.♻️ Proposed refactor
+func resolveBootstrapTelemetry(tl *tracing.Telemetry, tm *TelemetryMiddleware) *tracing.Telemetry { + if tl != nil { + return tl + } + if tm != nil { + return tm.Telemetry + } + return nil +} + func (tm *TelemetryMiddleware) WithTelemetry(tl *tracing.Telemetry, excludedRoutes ...string) fiber.Handler { - bootstrapTelemetry := tl - if bootstrapTelemetry == nil && tm != nil { - bootstrapTelemetry = tm.Telemetry - } + bootstrapTelemetry := resolveBootstrapTelemetry(tl, tm)(apply the same substitution in
WithTelemetryInterceptorandUnaryClientInterceptor)Also applies to: 606-609, 809-812
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@middleware/telemetry.go` around lines 247 - 250, Extract the duplicated bootstrap telemetry resolution into a shared helper, preserving the precedence of the explicit `tl` value and falling back to `tm.Telemetry` only when needed and `tm` is non-nil. Replace the inline logic in `WithTelemetry`, `WithTelemetryInterceptor`, and `UnaryClientInterceptor` with this helper so all three construction-time gates use the same behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/metrics-contract.md`:
- Line 38: Update the gRPC naming footnote in docs/metrics-contract.md to
reference the function or method containing the rpc.grpc.status_code span
attribute assignment instead of the stale telemetry.go:549 line reference. Keep
the existing metric-name guidance unchanged.
In `@sqlobs/sqlobs.go`:
- Around line 180-218: Update the documentation for InstrumentDB to explicitly
state that it returns a fresh *sql.DB with a separate connection pool, and
instruct callers to close the original handle and reapply SetMaxOpenConns,
SetMaxIdleConns, and SetConnMaxLifetime settings to the returned handle.
---
Outside diff comments:
In `@middleware/telemetry.go`:
- Around line 247-250: Extract the duplicated bootstrap telemetry resolution
into a shared helper, preserving the precedence of the explicit `tl` value and
falling back to `tm.Telemetry` only when needed and `tm` is non-nil. Replace the
inline logic in `WithTelemetry`, `WithTelemetryInterceptor`, and
`UnaryClientInterceptor` with this helper so all three construction-time gates
use the same behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aecfec07-776c-4652-999e-f81d560364d1
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
docs/CHANGELOG-native-metrics.mddocs/metrics-contract.mdgo.modmessagingobs/doc.gomessagingobs/messagingobs.gomessagingobs/messagingobs_test.gomiddleware/telemetry.gomiddleware/telemetry_active_requests_test.gomiddleware/telemetry_grpc_metrics_test.goredisobs/doc.goredisobs/redisobs.goredisobs/redisobs_test.gosqlobs/doc.gosqlobs/sqlobs.gosqlobs/sqlobs_test.gotracing/otel.gotracing/runtime_metrics_test.go
… collector Move the Fiber-dependent code out of the library core so apps still on Fiber v2 can consume everything except the HTTP middleware (core NewTelemetry, runtime metrics, messaging, gRPC interceptors, DB/cache helpers). Pure code move — no metric/telemetry logic changed. Root cause: tracing/otel.go imported gofiber/fiber/v3 only for 2 HTTP helpers, which transitively dragged fiber into tracing, messagingobs, runtime and the gRPC interceptors (go list -deps ./messagingobs showed 13 fiber deps). Changes: - tracing is now fiber-free (removed fiber/v3 import). - SetSpanAttributeForParam / ExtractHTTPContext moved tracing -> middleware (new middleware/http_trace.go). - gRPC interceptors moved to new fiber-free package grpcmiddleware/ (WithTelemetryInterceptor, EndTracingSpansInterceptor, UnaryClientInterceptor). - System-metrics collector moved to new fiber-free package telemetrycore/ (single shared singleton for HTTP + gRPC). - HTTP middleware (WithTelemetry, EndTracingSpans) stays in middleware, still Fiber v3 — the only package that legitimately needs it. go list -deps gofiber count: tracing 13->0, messagingobs 13->0, grpcmiddleware 0, telemetrycore 0; middleware stays 13 (correct). BREAKING CHANGE: SetSpanAttributeForParam and ExtractHTTPContext move from package tracing to package middleware; the gRPC interceptors move from middleware.TelemetryMiddleware to grpcmiddleware.TelemetryMiddleware (grpcmiddleware.NewTelemetryMiddleware). Consumers must update import paths (midaz fix tracked in docs/CHANGELOG-native-metrics.md, separate repo/PR). All tests pass (go test -tags=unit ./...); golangci-lint 0 issues; OTel core unchanged at v1.44.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> X-Lerian-Ref: 0x1
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/CHANGELOG-native-metrics.md (1)
69-80: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the Phase 2 status to match the shipped implementation.
This section says SQL, Redis, and RabbitMQ instrumentation is “a implementar,” but the PR objectives and stack scope state these helpers are added in this PR. Marking them as pending makes the changelog misleading; either document them as implemented or explicitly exclude them from this release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/CHANGELOG-native-metrics.md` around lines 69 - 80, Update the Phase 2 section in docs/CHANGELOG-native-metrics.md to reflect that the SQL, Cache, and Mensageria helpers are implemented and shipped in this release, rather than “a implementar”; keep the deferred Mongo status unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@grpcmiddleware/telemetry.go`:
- Line 30: Update the google.golang.org/grpc dependency from v1.81.1 to v1.82.1
or newer in go.mod, then regenerate go.sum to match the resolved module version
and checksums.
---
Outside diff comments:
In `@docs/CHANGELOG-native-metrics.md`:
- Around line 69-80: Update the Phase 2 section in
docs/CHANGELOG-native-metrics.md to reflect that the SQL, Cache, and Mensageria
helpers are implemented and shipped in this release, rather than “a
implementar”; keep the deferred Mongo status unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 248e9ad8-9f01-4cf2-ae3e-5f63ceb5fe24
📒 Files selected for processing (13)
docs/CHANGELOG-native-metrics.mdgrpcmiddleware/harness_test.gogrpcmiddleware/telemetry.gogrpcmiddleware/telemetry_grpc_metrics_test.gogrpcmiddleware/telemetry_test.gomiddleware/helpers.gomiddleware/http_trace.gomiddleware/metrics.gomiddleware/telemetry.gomiddleware/telemetry_test.gotelemetrycore/metrics_collector.gotelemetrycore/metrics_collector_test.gotracing/otel.go
💤 Files with no reviewable changes (2)
- middleware/helpers.go
- tracing/otel.go
Address CodeRabbit review on PR #36: - security(CRITICAL): bump google.golang.org/grpc v1.81.1 -> v1.82.1 (GHSA-hrxh-6v49-42gf, affects <1.82.1). otel core unchanged at v1.44.0. - docs(sqlobs): document that InstrumentDB returns a FRESH *sql.DB with a SEPARATE connection pool — caller must Close the original handle and re-apply pool limits (SetMaxOpenConns/SetMaxIdleConns/SetConnMaxLifetime) on the returned one, else two live pools hit the same DB. - docs(metrics-contract): replace stale telemetry.go:549 line reference with the function name (grpcmiddleware.WithTelemetryInterceptor / recordRPCDuration) after the Fiber decoupling moved the gRPC code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> X-Lerian-Ref: 0x1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objetivo
Padronizar a emissão de métricas transversais (a lib emite por default, com nome/unidade/labels iguais em todo serviço) e oferecer helpers de auto-instrumentação de DB/cache/fila — para acabar com a bagunça de métricas por-app e aposentar spans manuais de infra (origem de ~27k séries de alta cardinalidade identificadas na migração pro Grafana Cloud).
Retrocompatível: nada muda até a app adotar. Runtime metrics é opt-in.
Métricas nativas (automáticas)
rpc.server.duration/rpc.client.durationhttp.server.active_requestsgo.*(runtime)EnableRuntimeMetrics)Helpers de auto-instrumentação (lib expõe helper; app cria o client)
sqlobsdb.client.operation.durationredisobsdb.client.operation.durationmessagingobsmessaging.client.operation.duration/messaging.process.durationPara SQL com dbresolver: instrumentar cada
*sql.DBantes do resolver (documentado).Guardrails
docs/metrics-contract.md.Fora deste PR
otelmongo v2sem release oficial (arrastaria otel core acima de v1.44.0); v1 deprecated. Backlog emdocs/CHANGELOG-native-metrics.md.Testes
34 testes novos, todos passando (
go test -tags=unit ./...— 13 pacotes OK), incluindo anti-PII e no-op.golangci-lint0 issues. OTel core mantido em v1.44.0.Deps
XSAM/otelsql v0.43.0,redis/go-redis/extra/redisotel/v9 v9.17.2,contrib/instrumentation/runtime v0.69.0— todas resolvem p/ otel core v1.44.0.🤖 Generated with Claude Code