Skip to content

Fix labelled heartbeat metrics reporting 0 with a buffered meter - #1593

Open
Mahnoor-Zaffar wants to merge 2 commits into
temporalio:mainfrom
Mahnoor-Zaffar:bmcqueen/heartbeat-metricbuffer-labels
Open

Fix labelled heartbeat metrics reporting 0 with a buffered meter#1593
Mahnoor-Zaffar wants to merge 2 commits into
temporalio:mainfrom
Mahnoor-Zaffar:bmcqueen/heartbeat-metricbuffer-labels

Conversation

@Mahnoor-Zaffar

Copy link
Copy Markdown

What & Why

When a worker is configured with a buffered metric meter (e.g. the Python SDK's MetricBuffer), every labelled worker-heartbeat metric — all slot counts and all poller counts — is reported to the server as 0, even though the worker is demonstrably serving work. Unlabelled heartbeat metrics (total_processed_tasks, etc.) are unaffected.

Root cause: Worker heartbeats do not query slot suppliers/pollers directly. They read AtomicU64s that the metrics pipeline writes as a side effect via gauge_with_in_memoryHeartbeatMetricType::WithLabel. The label lookup in label_value_from_attributes only handled Prometheus, OTel, and NoOp; MetricAttributes::Buffer fell through to None, so the atomic was never written and kept its #[derive(Default)] value of 0.

The buffered handle is an opaque LazyRef<Arc<dyn CustomMetricAttributes>> (a Python-held PyDict in the Python bridge), which core cannot read without taking a GIL — exactly what the buffered design avoids. So core retains the Vec<MetricKeyValue> it already has in hand at attribute-creation time instead.

Changes

  • temporalio-common: MetricAttributes::Buffer now also carries a core-side kvs: Arc<Vec<MetricKeyValue>> snapshot; label_value_from_attributes resolves labels from it.
  • temporalio-sdk-core: MetricsCallBuffer::new_attributes/extend_attributes capture the kvs (merging last-wins on extend); BufferInstrument::send destructures the new shape. The MetricEvent protocol is unchanged, so language bridges compile untouched.
  • Tests: unit regression covering buffered label resolution at both the HeartbeatMetricType and MetricsContext/in_memory_meter levels, plus a buffered backing added to the worker-heartbeat integration test.
  • Changelog entry under Unreleased.

Validation

  • cargo test -p temporalio-common --features core-telemetry-bridge --lib in_memory_attributes_provide_label_values — pass
  • cargo test -p temporalio-sdk-core --lib telemetry::metrics — pass (incl. new regression; confirmed it fails without the fix)
  • Integration test target compiles; clippy clean for these changes; cargo +nightly fmt --check clean.

Fixes the Python SDK issue reported in temporalio/sdk-python#1817.

@Mahnoor-Zaffar
Mahnoor-Zaffar requested a review from a team as a code owner September 5, 2026 13:42
@CLAassistant

CLAassistant commented Sep 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Sushisource Sushisource left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think this makes sense but we shouldn't need to be pulling in any updates from api_upstream in this PR

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants