docs(core): instrumentation guide for lib-observability v2#38
Merged
Conversation
Prescriptive usage guide (docs/USAGE.md) for instrumenting a service with lib-observability v2: bootstrap (NewTelemetry + ApplyGlobals pitfall), HTTP (Fiber v3), gRPC (grpcmiddleware), SQL (sqlobs, incl. dbresolver + separate-pool caveat), cache (redisobs Redis/Valkey), messaging (messagingobs RabbitMQ), business metrics, an adoption checklist, and the cardinality/PII rules. All snippets validated against the real v2 API. Meant to be handed to an AI/engineer to perform the per-service instrumentation (Fase 3). Mongo + HTTP-on-Fiber-v2 explicitly listed as not-yet-available. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> X-Lerian-Ref: 0x1
Contributor
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughO guia ChangesGuia de instrumentação
✨ Finishing Touches✨ Simplify code
Comment |
gauchito91
changed the title
lerian-studio
added
size/M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/USAGE.md`:
- Line 229: Update the HTTP checklist item in the Fiber v3 section to reference
tm.WithTelemetry(tel) instead of middleware.WithTelemetry, matching the
documented HTTP example while preserving the Fiber version guidance.
- Around line 121-123: Update the usage examples around InstrumentDB,
RegisterDBStatsMetrics, and f.Counter to handle each returned error before
proceeding. Only construct the resolver after both InstrumentDB calls succeed,
defer reg.Unregister() after successful registration, and check the f.Counter
result before calling WithAttributes().AddOne(...), returning or reporting
errors consistently with the examples.
- Around line 31-42: Update the NewTelemetry example to handle initialization
errors by returning or exiting before registering the shutdown defer, preventing
use of a nil tel handle. Define a shutdown context for
ShutdownTelemetryWithContext, or use an existing documented context variable, so
ctx is initialized before the defer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- bootstrap: return/exit on NewTelemetry error before the shutdown defer (nil handle would panic); define the shutdown ctx. - sqlobs.RegisterDBStatsMetrics / metrics.Counter: check returned error before defer Unregister / chaining. - adoption checklist: use tm.WithTelemetry(tel) to match the HTTP example. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/USAGE.md`:
- Around line 41-45: Update the production telemetry example around NewTelemetry
to use an HTTPS collector endpoint and set InsecureExporter to false;
alternatively, clearly relabel the configuration as development-only. Ensure the
documented production path does not require ALLOW_INSECURE_OTEL or terminate at
startup.
- Around line 41-46: Add the standard library imports for context and log to the
import block used by the telemetry bootstrap example, so context.Background and
log.Fatalf resolve when the snippet is copied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…eRabbit) - bootstrap example used DeploymentEnv=production + InsecureExporter=true, which makes NewTelemetry return an error (service won't start) unless ALLOW_INSECURE_OTEL is set. Switch to https:// endpoint + InsecureExporter false, and add a security note explaining the policy + when insecure is allowed (development/local only). - add missing imports (context, log) used by the bootstrap snippet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> X-Lerian-Ref: 0x1
…oint Bootstrap example now reads all config from environment variables (the Helm-injected pattern already used by Lerian services) instead of a literal URL in code: OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_RESOURCE_SERVICE_NAME/_VERSION/ _DEPLOYMENT_ENVIRONMENT, OTEL_LIBRARY_NAME, ENABLE_TELEMETRY. Endpoint/insecure are decided per-environment by Helm; the code only calls os.Getenv. Security note reworded accordingly. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/USAGE.md`:
- Line 26: Update the usage documentation and corresponding configuration
snippet to either implement an ENV_NAME fallback when
OTEL_RESOURCE_DEPLOYMENT_ENVIRONMENT is unset, or remove ENV_NAME from the
documented canonical variables; ensure the documented behavior matches the
configuration actually read for DeploymentEnv.
- Around line 38-39: Update the ParseBool handling for ENABLE_TELEMETRY and
OTEL_EXPORTER_OTLP_INSECURE to distinguish unset variables from malformed
non-empty values: retain false only when the environment variable is unset, and
surface or return ParseBool errors for invalid input instead of ignoring them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- DeploymentEnv: fall back to ENV_NAME when OTEL_RESOURCE_DEPLOYMENT_ENVIRONMENT is unset (both are used across the services' .env), instead of only mentioning ENV_NAME in a comment while reading a single var. - ParseBool: distinguish unset (default false) from a malformed value (return error) via a parseBoolEnv helper, so a typo in ENABLE_TELEMETRY / OTEL_EXPORTER_OTLP_INSECURE no longer silently disables telemetry. 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/USAGE.md (1)
70-76: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle
ErrEmptyEndpointseparately in this example.NewTelemetryreturns a non-nil noop telemetry handle and installs noop globals when the collector endpoint is empty, so thislog.Fatalfaborts the service instead of letting the documented fallback run.🤖 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/USAGE.md` around lines 70 - 76, Update the error handling around NewTelemetry to detect ErrEmptyEndpoint separately, allowing the non-nil noop telemetry handle and installed noop globals to continue through the documented fallback path; retain log.Fatalf for other initialization errors, and keep the ShutdownTelemetryWithContext defer safe for the resulting handle.
🤖 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/USAGE.md`:
- Around line 41-45: Update the boolean environment parsing flow using
os.LookupEnv so only !ok returns false without an error. Remove the v == ""
shortcut, allowing explicitly empty values to reach strconv.ParseBool and return
its parsing error.
---
Outside diff comments:
In `@docs/USAGE.md`:
- Around line 70-76: Update the error handling around NewTelemetry to detect
ErrEmptyEndpoint separately, allowing the non-nil noop telemetry handle and
installed noop globals to continue through the documented fallback path; retain
log.Fatalf for other initialization errors, and keep the
ShutdownTelemetryWithContext defer safe for the resulting handle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
parseBoolEnv now defaults false ONLY for unset vars (!ok). An explicitly set but empty value (e.g. an empty Helm value) goes to strconv.ParseBool and fails visibly, so it can't silently disable telemetry or flip exporter security. 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.
Adiciona
docs/USAGE.md— guia prescritivo de como instrumentar um serviço com a lib-observability v2 (as features do #36).Escrito para ser entregue a uma IA/engenheiro executar a instrumentação por serviço (Fase 3 do plano).
Conteúdo
NewTelemetry) + a pegadinha doApplyGlobals(bug nº1 de "não emite nada").grpcmiddleware, sem Fiber).sqlobs): caso simples, dbresolver (instrumentar cada*sql.DBantes do resolver), e o caveat do pool separado.redisobs— Redis + Valkey).messagingobs— RabbitMQ, produce/consume comfinish(err)).tenant.idexplícito).Qualidade
Todas as assinaturas/snippets foram validados contra a API real da v2 (mergeada no #36). Sem código que não compila.
🤖 Generated with Claude Code