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, metrics invetnory, context propagation design and other utils and their usage.

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:07:09Z

+
+## Emitter
+
+`Emitter` is an interface. `New(scope)` returns the default tally-backed implementation; `New(nil)` returns one backed by `tally.NoopScope`, so callers do not need to special-case metrics using alternative metrics library than tally. This also serves as a no-op fallback which could be useful for tests.


it is unlikely that clients could use anything other than Tally; tally is used for all of the internal functions that clients won't override.
The extension point is typically inverted: clients use Tally library but can provide a custom backend.

good point, reworded

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

+  Inc(op, name string, opts ...Option)
+  Gauge(op, name string, v float64, opts ...Option)
+  RecordDur(op, name string, d time.Duration, opts ...Option)
+  RecordCount(op, name string, v int64, opts ...Option)


what is a difference between this and Inc ?

Inc is counter for counting calls, RecordCount is histogram for recording # of targets. Added explanation to make it clear

sbalabanov · 2026-07-08T23:19:59Z

+
+// RecordFailure emits a requests counter tagged with result=fail plus
+// failure information derived from err. No-op on nil.
+func RecordFailure(e Emitter, op string, err error)


need a cross reference to error classification.
what is "additional failure information"?

RecordFailure needs to call classifier to get errorCode to count reasons for failure. The err needs to be passed in. But fold this in RecordRequest as suggested below

sbalabanov · 2026-07-08T23:21:27Z

+These are helper functions wrapping `Inc` calls for top-level request recording.
+
+```go
+// RecordSuccess emits a requests counter tagged with result=success.


is the number of reported error types is limited, it is more efficient for the metrics backend to store the outcome in the same tag, i.e. result=success|infra_error|infra_retryable_error|user_error

yep that's the intention, RecordSuccess RecordFailure will both use result= tag.
I folded these in RecordRequest as suggested below

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.

sbalabanov · 2026-07-08T23:28:06Z

+}
+func FromContext(ctx context.Context) *Emitter { // should never return nil
+   e, ok := ctx.Value(emitterKey{}).(*Emitter)
+   if !ok { return noopEmitter }


panic or error ? :)

at least, let RFC discuss tradeoffs

sure I'll add some explanations.
imo,

observability is not busisness logic, not having metrics emission should not crash the server or fail the request.

in practice, a missing emitter means someone forgot to call WithEmitter in the handler -> lose metrics -> notice in dashboards -> fix the settitng, it

if someone don't want metrics, don't set it. Rather than having to explicitly set it to noop in context

other wise every callsite has to handle this error, if err := FromContext(ctx); err != nil {} but there's nothing meaninful they can do for it

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


		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:


		## Emitter

		`Emitter` is an interface. `New(scope)` returns the default tally-backed implementation; `New(nil)` returns one backed by `tally.NoopScope`, so callers do not need to special-case metrics using alternative metrics library than tally. This also serves as a no-op fallback which could be useful for tests.


		## 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!

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!

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!

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!

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!

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!

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

xytan0056 Jul 9, 2026 •

edited

Loading

xytan0056 Jul 9, 2026 •

edited

Loading

xytan0056 Jul 9, 2026 •

edited

Loading

xytan0056 Jul 9, 2026 •

edited

Loading