[yuechao-qin]

What

Four annotation keys were read from three different modules, one of which imported a
Kubernetes launcher purely to borrow its constants. This puts every read in one place
and adds the lookup that turns whatever a task's provider annotation says into a
provider and a cluster.

No behaviour changes. Every value emitted today is still emitted, identically.

Why

The metric emitter is about to become a fourth reader, and it runs inside
before_commit:

  TODAY                                      WHAT A FOURTH READER WOULD ADD
  ─────────────────────────────────────      ──────────────────────────────────
  execution_tracing   declares cloud_provider    metrics.py declares it a 3rd time
  contextual_logging  declares it again          + re-parses the accelerators string
  execution_tracing   imports kubernetes_        + needs cluster, which only the
                      launchers for 3 strings      launcher config knows

That last line is the problem. The cluster a task ran on is not in its annotations —
the annotation holds a key or an alias, and only the deployment's launcher config says
which cluster each resolves to. Reading that config per status change would put a file
read inside a database commit.

So the mapping is registered once at startup and looked up in memory.

Changes

  instrumentation/execution_annotations.py   new, the only module that knows a key
  instrumentation/execution_tracing.py       -1 declaration, -1 launcher import
  instrumentation/contextual_logging.py      -1 declaration

CLOUD_PROVIDER_KEY	declared here — no launcher owns it, routing sets it
the three resource keys	aliased from kubernetes_launchers, not respelled, so a rename there cannot silently empty these readers
cloud_provider()	the annotation as written
target()	resolved to Target(provider, cluster) through the registered map
accelerators()	{"X":"1"} and {"X": "1"} parse to one request
register_targets()	called once at startup by the deployment

Target.cluster_from_annotation covers a provider that declares no clusters of its own
and lets each task name one. The deployment registers that annotation key, so no
deployment-specific key appears in this repository.

Every reader is pure — task_spec is a loaded column, never a relationship — so all of
them are safe inside before_commit, where a lazy load would put a SELECT in the
commit path.

The caller arrives separately: targeting/routing.py in Shopify's oasis-backend builds
the map from its launcher config at create_launcher(). Until something calls
register_targets, the map is empty and target() returns None, which is the same
answer it gives for an unrecognised value.

Tests

• 16 cases over the three readers: absent, empty, malformed, two models, whitespace
• alias and canonical key resolve to the same target
• a provider naming its own cluster, and one that names none
• the existing tracing and logging tests are unchanged and still pass

[morgan-wowk]

🤖 AI-assisted review at 179ab82.

This review looks at two questions: does the new telemetry add slow queries or slow work to hot paths, and could it break anything?

Performance: fine. Every reader in execution_annotations is a dict lookup on the already-loaded task_spec column. Nothing lazy-loads or queries the database.

Robustness: two small points are inline. Neither causes a failure on its own in this PR. They matter once #409 calls these readers from before_commit outside the try block. That review covers the crash path.

The full suite passes at this head (599 passed).

_{Posted as a comment, not an approval. Any approval decision is separate from this review.}

[morgan-wowk]

accelerators() documents only ValueError. Two inputs raise TypeError instead:

• json.loads(raw) when raw is not a string. TaskSpec.annotations is Dict[str, Any], so a client can send the accelerators annotation as a JSON object instead of a JSON-encoded string.
• int(quantity) on line 141 when the count is null or a list, for example '{"nvidia-l4": null}'.

The same applies to _TARGETS.get(selected) on line 110: a list-valued cloud_provider is unhashable.

Today the only caller catches ValueError (_accelerator in #409), so these escape. The simplest fix is to have the readers catch (TypeError, ValueError), or to have the metric caller catch Exception.

[morgan-wowk]

Minor, import weight. contextual_logging used to be almost dependency-free. Through execution_annotations it now imports backend_types_sql, SQLAlchemy and launchers.kubernetes_launchers, which pulls in the kubernetes client. In a fresh subprocess, the cumulative import time went from about 4 ms to about 249 ms. There's no import cycle; the structured-logging, bugsnag and api-tracing import orders all still work.

This module is imported early by oasis-backend's logging setup. If startup time matters there, a lazy import inside execution_logging_context, or keeping just the one key constant here, would avoid it.