docs(observability/metrics): document the metrics package by xytan0056 · Pull Request #158 · uber/tango

xytan0056 · 2026-07-07T23:24:38Z

add Tango Metrics Inventory RFC into the repo. Covers the Emitter contract and its usage in callsites

xytan0056 · 2026-07-07T23:40:02Z

+- shared metric-name, op-name, and tag-key constants
+- default histogram buckets sized for RPC latency and large target counts
+- an in-flight gauge helper that owns the atomic counter callers otherwise reinvent
+- a failure classifier that reuses the errors package


@justinwon777 place holder for errors package design RFC

why metrics should own a classifier?

just discussed that we will just emit classified error code instead. Will remove

sywhang · 2026-07-08T16:50:36Z

+
+Options are variadic and compose. `WithTags` merges into the base tag set with later-wins semantics on key collision. `WithDurationBuckets` / `WithValueBuckets` override the emitter's default histogram buckets on a single call.
+
+## Op-as-subscope


what does this mean??

Op-as-subscope means passed op will be a subscope, called like

func (e *defaultEmitter) Inc(op, name string, opts ...Option) { ... e.Subscope(op).Counter(name).Inc(1) }

so the metric name is controller.get_target_graph.TreehashCacheLookup

sywhang · 2026-07-08T17:32:27Z

+
+Every RPC calls one of:
+
+- `e.Inc(op, Requests, WithTags({result: success}))` on the happy path


Inc and RecordFailure aren't intuitive names for counterparts of each other. can we rename?

Also, not sure why is Inc and failure/success is a thing. Do we need this?

good point. added symmetrical helper and documented generic purpose of Inc

sbalabanov

Missing pieces:

error classification integration
working with histograms for both status and duration and an example on custom buckets

sbalabanov · 2026-07-08T22:58:36Z

+
+Tango is a library. Consumers wire it into their own `fx` graph with their own `tally.Scope`, and often deploy multiple flavors (long-running server, batch job, one-shot CLI) side-by-side. Without a shared convention every deployment invents its own metric names and tag layout, and dashboards devolve into per-name merges.
+
+This package owns:


should this file be README.md under observability/metrics then?

following https://github.com/uber-go/fx/tree/d5da5b04ac906bfbad8b400baeee9b970c1be6f3/docs/src
this is meant to be usage doc

sbalabanov · 2026-07-08T23:00:36Z

+- shared metric-name, op-name, and tag-key constants
+- default histogram buckets sized for RPC latency and large target counts
+- an in-flight gauge helper that owns the atomic counter callers otherwise reinvent
+- a failure classifier that reuses the errors package


why metrics should own a classifier?

sbalabanov · 2026-07-08T23:02:58Z

+- default histogram buckets sized for RPC latency and large target counts
+- an in-flight gauge helper that owns the atomic counter callers otherwise reinvent
+- a failure classifier that reuses the errors package
+- context-based emitter propagation so downstream helpers do not need the emitter threaded through every signature


they still need to thread context object though.
For the record, I do not necessarily agree that this is better approach than explicit signature-based, but not going to make it blocking.

Good point. Yes, if a function needs to send metrics, it should explictly extract from context, but that should be the standard idiom (context has to be there, no context is worse that no metrics). I'll add this bit in the doc
This is the same for all other request scoped things like logger, tracings, signals etc. Emitter is what all downstream functions need but not required for business logic.
Signature based will just require the arg in every callsite -- like passing scope into hashing even though it's not needed in the actual logic, plus we'll have to create a subscope and be careful not to mix up with the one from parent

sbalabanov · 2026-07-08T23:26:26Z

+
+## TrackInFlight
+
+Gauges are update-only, so an in-flight counter needs an owning atomic somewhere. Rather than force every call site to allocate its own `atomic.Int64` and remember to update the gauge, the Emitter.TrackInFlight owns a per-op `*int64`:


what is the practical example where gauges need to be used for Tango?
Gauge is a pretty nasty metric type when it comes to distributed systems with multiple running instances, and should be in general avoided.

Considering how heavy graph computation could be, I think it's beneficial to track inflight requests/leased workspaces per instance. Gauge is the natural fit here, this way users can have visibility into how much scale the service needs (e.g. how many workers exhausted workspace pool). Especially for native_orchestrator, where the computation happen on hosts.

In practice, when you track per-instance metrics, cardinality starts to become a problem.
It is almost always better to report two counter types of metrics instead: first when the request started, and second when the request finished (as a histogram, with latency and an outcome).
Rely on logging/log aggregation for high-cardinality tracking/debugging.

…local metrics Rework metrics.md around the revised proposal: a concrete Tally-backed Emitter that binds and returns instruments, explicit dependency injection instead of context propagation, domain-local operation/metric names and bucket policies, controller-boundary request classification, per-series in-flight counters, and explicit no-op construction.

…ing-method errors Refine the revised metrics design: remove the gauge/in-flight tracking, drop the central shared-conventions vocabulary, and return instruments directly from the binding methods (only New surfaces a nil-scope wiring error). Add a Request-specific tags example.

xytan0056 · 2026-07-15T04:49:52Z

updated based on discussion offline.

xytan0056 force-pushed the pr0-docs branch 5 times, most recently from ea9e0ef to d01c936 Compare July 7, 2026 23:37

xytan0056 marked this pull request as ready for review July 7, 2026 23:39

xytan0056 requested review from a team as code owners July 7, 2026 23:39

xytan0056 commented Jul 7, 2026

View reviewed changes

xytan0056 force-pushed the pr0-docs branch 4 times, most recently from a1249f6 to cf7326b Compare July 7, 2026 23:58

sywhang reviewed Jul 8, 2026

View reviewed changes

xytan0056 force-pushed the pr0-docs branch 6 times, most recently from 4f4387b to 54a049f Compare July 8, 2026 21:47

sbalabanov requested changes Jul 8, 2026

View reviewed changes

sbalabanov reviewed Jul 8, 2026

View reviewed changes

Comment thread docs/observability/metrics.md Outdated

xytan0056 force-pushed the pr0-docs branch from 54a049f to 763f930 Compare July 9, 2026 01:06

xytan0056 assigned gh-haniloza Jul 9, 2026

docs(observability/metrics): document the metrics package

59d8c93

xytan0056 force-pushed the pr0-docs branch from 763f930 to 59d8c93 Compare July 9, 2026 03:59

xytan0056 unassigned gh-haniloza Jul 9, 2026

Update metrics.md

4293ff1

xytan0056 mentioned this pull request Jul 11, 2026

feat(observability/metrics): add Emitter, options, buckets, and label vocabulary #181

Open

Update metrics.md

1b85a19

xytan0056 force-pushed the pr0-docs branch from 2a4d0f3 to 1b85a19 Compare July 14, 2026 17:24

xytan0056 added 2 commits July 14, 2026 13:42

Merge branch 'main' into pr0-docs

4371e62


		Options are variadic and compose. `WithTags` merges into the base tag set with later-wins semantics on key collision. `WithDurationBuckets` / `WithValueBuckets` override the emitter's default histogram buckets on a single call.

		## Op-as-subscope


		Every RPC calls one of:

		- `e.Inc(op, Requests, WithTags({result: success}))` on the happy path


		Tango is a library. Consumers wire it into their own `fx` graph with their own `tally.Scope`, and often deploy multiple flavors (long-running server, batch job, one-shot CLI) side-by-side. Without a shared convention every deployment invents its own metric names and tag layout, and dashboards devolve into per-name merges.

		This package owns:


		## TrackInFlight

		Gauges are update-only, so an in-flight counter needs an owning atomic somewhere. Rather than force every call site to allocate its own `atomic.Int64` and remember to update the gauge, the Emitter.TrackInFlight owns a per-op `*int64`:

Uh oh!

Conversation

xytan0056 commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbalabanov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xytan0056 Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xytan0056 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xytan0056 commented Jul 7, 2026 •

edited

Loading

xytan0056 Jul 8, 2026 •

edited

Loading

xytan0056 Jul 9, 2026 •

edited

Loading

xytan0056 Jul 9, 2026 •

edited

Loading