Repository navigation
Add metrics for task and asset state store access - #75729
Conversation
|
Thanks for the review @ashb! Merging. |
Backport successfully created: v3-4-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
FrankYang0529
left a comment
There was a problem hiding this comment.
Thanks for the PR. Leave some comments. I can create a follow-up PR to handle it.
| def clear(self) -> None: | ||
| """Delete all state keys for this asset.""" | ||
| from airflow.sdk.execution_time.task_runner import SUPERVISOR_COMMS | ||
|
|
||
| # DB ref first, same ordering rationale as delete(). | ||
| SUPERVISOR_COMMS.send(self._build_clear_message()) | ||
| backend = _get_worker_state_store_backend() | ||
| if backend is not None: | ||
| backend.clear(AssetScope(name=self._name, uri=self._uri)) | ||
|
|
||
| async def aclear(self) -> None: | ||
| """Async version of `clear` that awaits instead of blocking the event loop.""" | ||
| from airflow.sdk.execution_time.task_runner import SUPERVISOR_COMMS | ||
|
|
||
| await SUPERVISOR_COMMS.asend(self._build_clear_message()) | ||
| backend = _get_worker_state_store_backend() | ||
| if backend is not None: | ||
| await backend.aclear(AssetScope(name=self._name, uri=self._uri)) |
There was a problem hiding this comment.
The PR description mentioned "There is no scope-level clear on the asset side, hence eight plus one."
AssetStateStoreAccessor.clear() and AssetStateStoreAccessor.aclear() delete every key stored for an asset, the same way TaskStateStoreAccessor.clear() deletes every key for a task instance. Should the two asset methods also emit a metric, like task_state_store.clear on the task side? If so, I can open a follow-up PR.
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize( | ||
| ("call", "expected"), | ||
| [ | ||
| pytest.param(lambda s: s.aget("job_id"), "task_state_store.read_found", id="read"), | ||
| pytest.param(lambda s: s.aset("job_id", "v"), "task_state_store.write", id="write"), | ||
| pytest.param(lambda s: s.adelete("job_id"), "task_state_store.delete", id="delete"), | ||
| pytest.param(lambda s: s.aclear(), "task_state_store.clear", id="clear"), | ||
| ], | ||
| ) | ||
| async def test_task_store_async(self, mock_supervisor_comms, call, expected): | ||
| mock_supervisor_comms.asend.return_value = TaskStateStoreResult(value="job-001") | ||
| with patch(self.PATCH) as incr: | ||
| await call(self.task_store()) | ||
| incr.assert_called_once_with(expected, tags=self.TAGS) | ||
|
|
||
| def test_no_tags_when_not_supplied(self, mock_supervisor_comms): | ||
| mock_supervisor_comms.send.return_value = TaskStateStoreResult(value="job-001") | ||
| with patch(self.PATCH) as incr: | ||
| TaskStateStoreAccessor(ti_id=self.TI_ID, scope=self.SCOPE).get("job_id") | ||
| incr.assert_called_once_with("task_state_store.read_found", tags={}) |
There was a problem hiding this comment.
This test only covers the async methods on TaskStateStoreAccessor. No test covers the counters in AssetStateStoreAccessor.aset and AssetStateStoreAccessor.adelete.
Was generative AI tooling used to co-author this PR?
What
There is no way measure what / who is using the state store. Debugging / Adoption questions like "which Dags touch it", "is it read heavy or write heavy", "is anyone using
delete" cannot be answered from Airflow today at all.The
resumable_job.*counters do not fill this gap today. They are emitted byResumableJobMixin, so they only see operators that use the mixin. The two other ways to get durable execution are invisible to them:KubernetesPodOperator, which reads and writespod_identifierthrough a class attribute rather than the mixin, and user code callingtask_state_storedirectly from a@taskorPythonOperator.Current behaviour
The state store accessors emit nothing. Every call through
task_state_storeandasset_state_storeis unmeasured, whatever/whoever issued it.Proposed change
Nine counters at the SDK accessors, which every caller passes through regardless of operator:
task_state_store.read_foundasset_state_store.read_foundtask_state_store.read_absentasset_state_store.read_absenttask_state_store.writeasset_state_store.writetask_state_store.deleteasset_state_store.deletetask_state_store.clearTagged with
dag_id,task_id,run_type,operator, andteam_nameon multi-team deployments. There is no scope-level clear on the asset side, hence eight plus one.Changes of Note
These count calls, not saves. A read that finds a value is not proof that work was avoided. The accessor never learns whether the operator managed to reconnect to the external job, because that lives outside Airflow and only the operator can
ask. So these counters answer "who uses the store";
resumable_job.*continues to answer "how often did it save work".Task store and asset store get separate names rather than one counter with a
scopetag.TaskScopecarriesrun_id, so a task store read that finds something was written by an earlier attempt of the same Dag run.AssetScopedoes not, so an asset read that finds something is an ordinary watermark read across runs.The granularity is per call, not per attempt. One
AgentOperatortask emits roughly 21 counts where aKubernetesPodOperatortask emits 1, so these cannot be compared across operators by raw volume without normalising.No
state_keytag.AgentOperatorwrites one key per agent step, so key cardinality is unbounded. Key-level detail belongs in logs, not metrics.Where the emits sit. Reads go in
_extract_get_response, whichgetandagetalready share on both accessors and which already distinguishes found from absent, so one emit point covers sync and async. Writes, deletes and clears emit at their call sites. Tags are built once inRuntimeTaskInstance.get_template_contextasti.stats_tagsplusoperator, andpassed into the accessor constructors, so nothing has to be looked up at call time.
User implications / backcompat
No breaking change. The tags only materialise on backends that carry them: OTel and dogstatsd natively, StatsD only with
[metrics] statsd_influxdb_enabled.stats_tagsdefaults to empty when an accessor is constructed directly, so existing callers and tests keep working.New series are added, bounded by
dag_idxtask_idxoperator.Testing
I validated certain cases manually with breeze and otel integration -
breeze start-airflow --integration otel, reading the collector atlocalhost:28889/metrics.These cases matter to me:
1. Plain
@task, both read outcomes.Reads a missing key, writes it, reads it back.
Both read outcomes distinguished, tags present on a task with no operator or mixin involved.
2. Asset store alongside task store.
One task touching both.
Ran it two times to show the difference:
3. Delete and clear.
writeat 2 for twosetcalls confirms the per-call grain.deleteandclearstay separate, which matters becauseAgentOperatordeletes every key it wrote on success.4.
ResumableJobMixinoperator, both families side by side.A mixin operator submitting a mock job, then cleared and re-run.
Two separate counter families at play here, no shared names, nothing double counted. The mixin keeps reporting the resume decision; the accessor independently reports the calls underneath it.
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.