Skip to content

internal/observability: derive metricProcedure from transport, not encoding - #2503

Open
elliotchance wants to merge 1 commit into
yarpc:mainfrom
elliotchance:OBSRVAPP-16832-metric-procedure-fix
Open

internal/observability: derive metricProcedure from transport, not encoding#2503
elliotchance wants to merge 1 commit into
yarpc:mainfrom
elliotchance:OBSRVAPP-16832-metric-procedure-fix

Conversation

@elliotchance

Copy link
Copy Markdown
Contributor

Problem

metricProcedure (added in #2359) is emitted on logs so an M3 alert can be
mapped back to the matching logs — its value is meant to equal the M3
procedure tag. It was derived from the encoding, but the M3 tag is shaped
by the transport, so the two diverge when a proto service is called over
HTTP:

Transport M3 procedure tag logged metricProcedure (before) match
gRPC uownservice/searchassets uownservice/searchassets
HTTP uber.infra.uown.uownservice--searchassets uownservice/searchassets

As a result, availability alerts on HTTP-invoked endpoints linked to log
queries that returned no results, making them hard to debug.

Fix

Derive metricProcedure from req.Transport instead of the encoding:

  • gRPC → short, slash-delimited form with the package path dropped
    (uownservice/searchassets)
  • every other transport (HTTP, TChannel) → fully-qualified name,
    lowercased, with ::-- (uber.infra.uown.uownservice--searchassets)

The M3 procedure tag itself is unchanged; only the log field is corrected to
match it. Encoding is still used to skip raw/unknown encodings.

Test plan

  • Updated TestMiddlewareLoggingWithMetricProcedure to be transport-aware,
    including the gRPC-vs-HTTP regression pair described above.
  • go test ./internal/observability/... passes; gofmt and go build clean.

🤖 Generated with Claude Code

…coding

metricProcedure mirrors the M3 "procedure" tag so logs can be correlated
with alert queries. It was derived from the encoding (proto/json -> "/",
thrift -> "--", package always stripped), but the M3 tag is shaped by the
transport. For a proto service called over HTTP the two diverge:

  gRPC  M3 procedure: uownservice/searchassets                  (log matched)
  HTTP  M3 procedure: uber.infra.uown.uownservice--searchassets (log did NOT)

So availability alerts on HTTP-invoked endpoints linked to log queries that
returned nothing. Derive metricProcedure from req.Transport instead: gRPC
keeps the short, slash-delimited form with the package path dropped; every
other transport (HTTP, TChannel) keeps the fully-qualified name and turns
"::" into "--". The M3 "procedure" tag itself is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants