Skip to content

fix(observability): adopt the ratified block — coarse enabled:true has no sampling ceiling - #298

Open
chitcommit wants to merge 1 commit into
mainfrom
fix/observability-ratified-off
Open

chitcommit wants to merge 1 commit into
mainfrom
fix/observability-ratified-off

Conversation

@chitcommit

@chitcommit chitcommit commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Why now

wrangler.jsonc:30 was "observability": { "enabled": true } — the coarse form, which carries no head_sampling_rate and therefore logs at 100%.

It went live in today's deploy: the first successful deploy of this Worker since 2026-08-08. So it had been dormant in config rather than costing anything, and is now active on a Worker taking production traffic. This is the same shape as the incident behind the ~$10k Cloudflare bill.

The change

-  "observability": { "enabled": true },
+  "observability": {
+    "logs": { "enabled": false, "head_sampling_rate": 0.05, "invocation_logs": true, "persist": true },
+    "traces": { "enabled": false, "head_sampling_rate": 1, "persist": true }
+  },

The ceiling is the point, not the on/off. Both are disabled, but the preset head_sampling_rate means that when observability is switched on later it samples at 5% rather than everything. enabled: true has no such bound.

Ecosystem context

count
wrangler configs declaring observability 96
coarse enabled: true (drift) 57
already granular 78

This moves chittyconnect onto the majority pattern rather than inventing a convention.

Two things flagged rather than silently decided

Sampling rate divergence. The three repos already on the granular shape — chittyevidence-db, chittycommand, chittystorage — use head_sampling_rate: 0. This uses 0.05 / 1 per the operator-supplied block. Both are defensible; ecosystem-wide reconciliation is worth doing, but not by quietly picking one inside an unrelated PR.

Inheritance is unverified. Placement is top level only, matching those same three. But tail_consumers is known not to inherit into service environments, and this Worker deploys with --env production. Whether top-level observability reaches the production environment will be verified against the live worker after deploy, not assumed. If it doesn't inherit, the block needs repeating per env and I'll follow up.

chittyconnect declares no tail_consumers at all — the deliberate post-incident state, left untouched.

Validation

wrangler deploy --env production --dry-run accepts the config. 601 tests pass, 1 skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX

Summary by CodeRabbit

  • Configuration
    • Updated observability settings to separately configure logs and traces.
    • Disabled log and trace collection while retaining persistence, sampling, and invocation logging settings.

…s no sampling ceiling

`"observability": { "enabled": true }` was live on a worker taking production
traffic. The coarse form carries no head_sampling_rate, so it logs at 100%. This
is the shape that produced the ~$10k Cloudflare bill previously attributed to a
tail-consumer spike, and it went live in today's deploy — the first successful
deploy of this Worker since 2026-08-08, so it had been dormant in config rather
than costing anything until now.

Replaced with the ratified granular block: logs and traces both disabled, and —
the part that matters — head_sampling_rate preset to 0.05 for logs and persist
set, so that switching observability ON later samples at 5% instead of
everything. The ceiling is the point; on/off is not.

Ecosystem context: 96 wrangler configs declare observability, 57 use the coarse
enabled:true form, 78 already use the granular one. This moves chittyconnect to
the majority pattern rather than inventing a convention.

Divergence flagged rather than silently resolved: the three repos already on the
granular shape (chittyevidence-db, chittycommand, chittystorage) use
head_sampling_rate 0. This uses 0.05 / 1 per the operator-supplied block. Worth
reconciling ecosystem-wide, but not by quietly picking one here.

Placement follows those same three: top level only. chittyconnect declares no
tail_consumers at all, which is the deliberate post-incident state and is left
untouched. Whether top-level observability reaches the production service
environment is verified post-deploy against the live worker, not assumed —
tail_consumers is known not to inherit and observability has not been proven to.

wrangler --dry-run accepts the config; 601 tests pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZbwet4A5CENYvuS1KAbSX
Copilot AI lite review requested due to automatic review settings September 8, 2026 19:17
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T19:22:03.736056Z 5e9fa61 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

traces.head_sampling_rate is set to 1 (100%), which undermines the “preset safe ceiling” intent if traces are enabled later.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates the Cloudflare Worker configuration to use the granular observability block in wrangler.jsonc, ensuring a predefined sampling ceiling exists (avoiding accidental 100% logging if observability is later enabled).

Changes:

  • Replaced coarse "observability": { "enabled": true } with explicit logs and traces sub-blocks.
  • Pre-set head_sampling_rate (notably logs: 0.05) while keeping logs/traces disabled by default.
File summaries
File Description
wrangler.jsonc Switches to granular observability configuration with explicit log/trace settings and sampling rates.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread wrangler.jsonc
"observability": { "enabled": true },
"observability": {
"logs": { "enabled": false, "head_sampling_rate": 0.05, "invocation_logs": true, "persist": true },
"traces": { "enabled": false, "head_sampling_rate": 1, "persist": true }
@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: eaac3473-cd93-4658-9f5f-9860aa7d1cb8

📥 Commits

Reviewing files that changed from the base of the PR and between f02a0da and 5e9fa61.

📒 Files selected for processing (1)
  • wrangler.jsonc

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Wrangler observability configuration now uses separate logs and traces settings. Both remain disabled with persistence and explicit sampling rates. Invocation logging remains enabled.

Changes

Observability configuration

Layer / File(s) Summary
Separate logs and traces settings
wrangler.jsonc
Replaces the single observability setting with separate logs and traces configuration. Both are disabled, persisted, and assigned sampling rates. Invocation logging remains enabled.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 5e9fa

This updates observability settings to granular disabled log and trace configuration with preset sampling ceilings. No merge-blocking production or user-impact risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the purpose, configuration change, ecosystem context, open decisions, and validation results. It does not follow the repository template because it omits the required Summary,… Add the required template sections. Document the impacted service or data layer and ontology classification. Complete the Security & Access and Docs checklists, including label status. Keep the existing validation details under Validation a…
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the observability configuration change and its primary purpose: adding a sampling ceiling to the coarse enabled setting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the purpose, configuration change, ecosystem context, open decisions, and validation results. It does not follow the repository template because it omits the required Summary, Security & Access, and Docs sections, plus explicit ontology classification and checklist status.

Resolution

Add the required template sections. Document the impacted service or data layer and ontology classification. Complete the Security & Access and Docs checklists, including label status. Keep the existing validation details under Validation and state the CI, lint, and test results explicitly.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/observability-ratified-off

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants