potiuk commented on code in PR #68906:
URL: https://github.com/apache/airflow/pull/68906#discussion_r3683039882
##########
airflow-core/src/airflow/models/serialized_dag.py:
##########
@@ -762,6 +763,7 @@ def write_dag(
session.merge(dag_version)
# Update the latest DagCode
DagCode.update_source_code(dag_id=dag.dag_id, fileloc=dag.fileloc,
session=session)
+ stats.incr("dag.serialization_writes", tags={"dag_id": dag.dag_id,
"bundle_name": bundle_name})
Review Comment:
This is the earlier of the two emission sites, and it fires on the path
where the serialized Dag **did not change** — only `dag_version.bundle_name` /
`bundle_version` / `version_data` were merged and `DagCode.update_source_code`
refreshed. No new `SerializedDagModel` row is written here.
The metric description says "Number of times a Dag was serialized and
written to the metadata DB", which doesn't match that path. Anyone using
`dag.serialization_writes` to measure how often Dags actually re-serialize —
the obvious use, and what the name suggests — will over-count every time a
bundle version changes without the Dag changing.
Two options, either is fine:
1. Emit only from the second site (the real write), and drop this one.
2. Keep both but distinguish them — either a `reason`/`kind` tag
(`"metadata_refresh"` vs `"new_version"`), or reword the description to say it
counts write operations including version-metadata refreshes.
I'd lean towards (1) unless you specifically want visibility into the
refresh path.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
