xBis7 commented on code in PR #62019:
URL: https://github.com/apache/airflow/pull/62019#discussion_r2812822658
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1173,6 +1173,11 @@ def _on_term(signum, frame):
state: TaskInstanceState
error: BaseException | None = None
+ stats_tags = {"dag_id": ti.dag_id, "task_id": ti.task_id}
+ Stats.incr(f"ti.start.{ti.dag_id}.{ti.task_id}", tags=stats_tags)
+ # Same metric with tagging
+ Stats.incr("ti.start", tags=stats_tags)
Review Comment:
You should use the `DualStatsManager` instead of `Stats` because of the
legacy metric.
`ti.start` is the regular metric name and the
`ti.start.{ti.dag_id}.{ti.task_id}` is the legacy name.
```diff
- Stats.incr(f"ti.start.{ti.dag_id}.{ti.task_id}", tags=stats_tags)
- # Same metric with tagging
- Stats.incr("ti.start", tags=stats_tags)
+ DualStatsManager.incr("ti.start", extra_tags=stats_tags)
```
There is a new config option to disable legacy metrics. The
`DualStatsManager` will check the option and skip the legacy name if enabled.
Also it will read the `metrics_tamplate.yaml` and get the legacy name and the
name tags from there
https://github.com/astronomer/airflow/blob/ff0c4bd334d215a1f2d717f1f0f2e1c25985c257/shared/observability/src/airflow_shared/observability/metrics/metrics_template.yaml#L167-L179
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1283,6 +1288,13 @@ def _on_term(signum, frame):
msg, state = _handle_current_task_failed(ti)
error = e
finally:
+ Stats.incr(
+ f"ti.finish.{ti.dag_id}.{ti.task_id}.{state.value}",
+ tags=stats_tags,
+ )
+ # Same metric with tagging
+ Stats.incr("ti.finish", tags={**stats_tags, "state": state.value})
Review Comment:
Similarly here
```diff
- Stats.incr(
- f"ti.finish.{ti.dag_id}.{ti.task_id}.{state.value}",
- tags=stats_tags,
- )
- # Same metric with tagging
- Stats.incr("ti.finish", tags={**stats_tags, "state": state.value})
+ DualStatsManager.incr("ti.finish", tags={"state": state.value},
extra_tags=stats_tags)
```
`tags` are used for both new and legacy metrics. `extra_tags` are referring
to the tags on the legacy metric name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
