docs: fix sampling defaults, Log API refs, and deprecation table#52
docs: fix sampling defaults, Log API refs, and deprecation table#52
Conversation
Shows how to use mux.HandlePath for webhooks, static files, OAuth callbacks, and path parameters alongside grpc-gateway routes.
- Fix NEW_RELIC_OPENTELEMETRY_SAMPLE and OTLP_SAMPLING_RATIO defaults from 0.2 to 0.1 to match code (changed in OTEL sampling reduction) - Replace loggers.AddToLogContext with log.AddToContext (public API) throughout Log.md - Add missing deprecated entries: DISABLE_PORMETHEUS, OTLP_USE_OPENTRACING_BRIDGE, GRPCClientInterceptor() - Add slog migration note for deprecated gokit/logrus backends
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocumentation-only updates: default trace sampling ratios lowered from 0.2 to 0.1; added deprecation entry for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the documentation to align with recent observability/logging API changes, clarifies deprecations, and adds guidance for registering custom (non-gRPC) HTTP routes on the grpc-gateway mux.
Changes:
- Update documented default sampling ratios for
NEW_RELIC_OPENTELEMETRY_SAMPLEandOTLP_SAMPLING_RATIOto0.1and extend the deprecated settings table. - Replace
loggers.AddToLogContextreferences with the publiclog.AddToContextAPI and add a slog migration note. - Add a new “Custom HTTP Routes” section (with examples) and an FAQ entry pointing to it.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| howto/Log.md | Updates context-aware logging examples to use log.AddToContext and adds slog migration guidance. |
| howto/APIs.md | Documents registering custom HTTP routes via runtime.ServeMux.HandlePath with several examples. |
| FAQ.md | Adds an FAQ entry linking to the new custom HTTP routes section. |
| config-reference.md | Adjusts sampling defaults and expands the deprecated settings table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 (2)
howto/Log.md (2)
68-83:⚠️ Potential issue | 🟡 MinorFix missing imports and variable declaration.
The code example has compilability issues:
- Missing
net/httpimport forhttp.ResponseWriterandhttp.Request- Missing
contextimport forcontext.Context- Line 73 should use
:=to declarectxon first assignment🔧 Proposed fix
import ( + "context" + "net/http" + "github.com/go-coldbrew/log" ) func handler(w http.ResponseWriter, r *http.Request) { - ctx = r.Context() + ctx := r.Context() ctx = log.AddToContext(ctx, "request-id", "1234")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/Log.md` around lines 68 - 83, The code sample fails to compile because handler uses types from net/http and context but doesn't import them and it reassigns an undeclared ctx variable; update the imports to include "net/http" and "context", and in handler declare the context variable on first use (ctx := r.Context()) before calling log.AddToContext; keep the rest of the flow (log.AddToContext calls and helloWorld(ctx) and helloWorld's signature using context.Context and log.Info) unchanged.
147-178:⚠️ Potential issue | 🟡 MinorFix missing imports and variable declaration.
The code example has the same compilability issues as the earlier example:
- Missing
net/httpimport forhttp.ResponseWriterandhttp.Request- Missing
contextimport forcontext.Context- Line 160 should use
:=to declarectxon first assignment🔧 Proposed fix
import ( + "context" + "net/http" + "github.com/go-coldbrew/log" "github.com/go-coldbrew/log/loggers" ) func init() { // set global log level to info // this is typically set by the ColdBrew cookiecutter using the LOG_LEVEL environment variable log.SetLevel(loggers.InfoLevel) } func handler(w http.ResponseWriter, r *http.Request) { - ctx = r.Context() + ctx := r.Context() ctx = log.AddToContext(ctx, "request-id", "1234")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/Log.md` around lines 147 - 178, The example is missing required imports and mis-declares ctx: add imports for "net/http" and "context" to the import block alongside "github.com/go-coldbrew/log" and "github.com/go-coldbrew/log/loggers", and in handler change the first assignment to ctx so it is declared (use ctx := r.Context()) before using log.AddToContext and log.OverrideLogLevel; update references to log.AddToContext, log.OverrideLogLevel, handler, and helloWorld accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@howto/Log.md`:
- Line 61: Change the section header "Context aware logs" to the hyphenated form
"Context-aware logs" in howto/Log.md; locate the heading text "Context aware
logs" and update it so the compound adjective is hyphenated when modifying the
noun.
---
Outside diff comments:
In `@howto/Log.md`:
- Around line 68-83: The code sample fails to compile because handler uses types
from net/http and context but doesn't import them and it reassigns an undeclared
ctx variable; update the imports to include "net/http" and "context", and in
handler declare the context variable on first use (ctx := r.Context()) before
calling log.AddToContext; keep the rest of the flow (log.AddToContext calls and
helloWorld(ctx) and helloWorld's signature using context.Context and log.Info)
unchanged.
- Around line 147-178: The example is missing required imports and mis-declares
ctx: add imports for "net/http" and "context" to the import block alongside
"github.com/go-coldbrew/log" and "github.com/go-coldbrew/log/loggers", and in
handler change the first assignment to ctx so it is declared (use ctx :=
r.Context()) before using log.AddToContext and log.OverrideLogLevel; update
references to log.AddToContext, log.OverrideLogLevel, handler, and helloWorld
accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9082fab-3551-49bd-a653-e2c4be5e72d0
📒 Files selected for processing (2)
config-reference.mdhowto/Log.md
- Fix slog migration note wording (no-code-change contradiction) - Fix grammar: "there a set" → "there is a set" - Hyphenate "Context-aware logs" heading - Fix ctx := declaration in code examples - Remove GRPCClientInterceptor() from env var deprecated table - Re-apply: sampling defaults 0.2 → 0.1, log.AddToContext, deprecated table entries
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
NEW_RELIC_OPENTELEMETRY_SAMPLEandOTLP_SAMPLING_RATIOdefaults from0.2to0.1in config-reference.md (code changed in OTEL sampling reduction)loggers.AddToLogContextwithlog.AddToContext(public API) throughout Log.mdDISABLE_PORMETHEUS,OTLP_USE_OPENTRACING_BRIDGE,GRPCClientInterceptor()Test plan
0.2defaults remain in config-reference.mdloggers.AddToLogContextreferences remain in Log.mdSummary by CodeRabbit
Documentation