Skip to content

Conversation

@gregbarasch
Copy link
Contributor

@gregbarasch gregbarasch commented Jan 6, 2026

💸 TL;DR

Our thrift server metrics assume that there will only be one server running within a single runtime. This is not always true. If a second server is running, the thrift server metrics will clash with one another. This PR adds a serverName label to our serverside metrics.

This version of the PR updates our first attempt by creating duplicate metrics with the v0 suffix. These v0 metrics are decoupled from the label expectations of baseplate v2, and therefore can be updated independently

📜 Details

This update is backwards compatible, and notably should not increase cardinality in any significant way. Preexisting metric usage will emit "unknown" as the serverName, which will retain metric cardinality and will add a negligible amount of data for metric DBs.

The API itself does not enable backward compatible attribute changes. We have a choice. We can:

  1. inject the serverName into the ctx on each request
  2. Always call GetThriftServiceName(cfg.Processor) -- needs testing
  3. (The option chosen) Provide an alternative WithArgs function

Commonly, we might make use of the optionals or builder pattern for forward looking, backwards compatible changes. Instead though we've opted to use a Cfg struct, as this matches patterns found elsewhere in the repo.

🧪 Testing Steps / Validation

Unit tests

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant