Skip to content

fix(telemetry): simplify Sentry configuration metadata - #5854

Merged
NathanFlurry merged 1 commit into
mainfrom
workspace/silver-forest-859f
Oct 10, 2026
Merged

NathanFlurry merged 1 commit into
mainfrom
workspace/silver-forest-859f

Conversation

@NathanFlurry

@NathanFlurry NathanFlurry commented Oct 9, 2026 •

Copy link
Copy Markdown
Member
  • Replace full configuration dumps in Sentry tags with concise database, pubsub, and cache backend labels.
  • Remove topology metadata from the Sentry scope while preserving configuration loading and the telemetry opt-out.
  • Validate with cargo check -p rivet-telemetry.

@railway-app

railway-app Bot commented Oct 9, 2026

Copy link
Copy Markdown

🚅 Deployed to the rivet-pr-5854 environment in rivet-frontend

8 services not affected by this PR
  • frontend-cloud
  • frontend-inspector
  • website
  • ladle
  • sentry-tunnel
  • kitchen-sink
  • mcp-hub
  • frontend-cloud-ee

@NathanFlurry
NathanFlurry force-pushed the workspace/silver-forest-859f branch from c16fa83 to 85b3657 Compare October 9, 2026 23:58
@NathanFlurry NathanFlurry changed the title fix(telemetry): exclude configuration secrets from Sentry tags fix(telemetry): simplify Sentry configuration metadata Oct 9, 2026
@claude

claude Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review: fix(telemetry): simplify Sentry configuration metadata

Summary: This removes a real leak. Serializing the full Database, PubSub, and Cache configs into Sentry tags could send credentials such as Postgres or NATS URLs with passwords. Tagging only fixed labels fixes that. I found no blocking issues.

Correctness

  • The enum matches are exhaustive, with no _ => arm. This follows CLAUDE.md, and a new Database, PubSub, or CacheDriver variant will fail to compile instead of silently going untagged.
  • The import paths resolve. rivet_config::config re-exports Database, PubSub, and CacheDriver, and Cache::driver() and enabled exist.
  • Dropping serde_json from Cargo.toml and Cargo.lock is consistent, since nothing else in the crate uses it.
  • The tag values are bounded and low-cardinality, which suits Sentry tags.

Minor notes

  • The topology tag is removed entirely. That fits the description, but anyone filtering Sentry by topology will lose that dimension.
  • The comment on the allowlist is clear. If you reword it, use periods instead of any dashes, per the CLAUDE.md comment rules.
  • Tests: there are none, and a pure tag mapping makes that acceptable. If you want one, a test under tests/ could check that the tags never contain URL-like content. I did not run cargo check -p rivet-telemetry myself.

LGTM.

@NathanFlurry
NathanFlurry marked this pull request as ready for review October 9, 2026 23:59
@NathanFlurry
NathanFlurry merged commit 8614063 into main Oct 10, 2026
1 of 5 checks passed
@NathanFlurry
NathanFlurry deleted the workspace/silver-forest-859f branch October 10, 2026 00:00
@railway-app
railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5854 October 10, 2026 00:00 Destroyed

This branch was successfully deployed

No deployments
rivet-frontend / rivet-pr-5854 — 85b36571 Deployed Oct 10, 2026 by railway-app[bot]
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.

1 participant