SameerMesiah97 commented on code in PR #67029:
URL: https://github.com/apache/airflow/pull/67029#discussion_r3253420039
##########
airflow-core/src/airflow/dag_processing/manager.py:
##########
@@ -969,7 +969,7 @@ def _log_file_processing_stats(self, known_files: dict[str,
set[DagFileInfo]]):
proc = self._processors.get(file)
num_dags = stat.num_dags
num_errors = stat.import_errors
- file_name = Path(file.rel_path).stem
+ file_name = normalize_name_for_stats(Path(file.rel_path).stem)
Review Comment:
Is` _log_file_processing_stats()` the only place where Dag file names are
incorporated into metric names?
Since the bug is fundamentally about unsanitized file-derived metric
segments, it’d be good to verify there aren’t other DAG-processing stats paths
still using `Path(...).stem` or `rel_path `directly without
`normalize_name_for_stats()`.
##########
airflow-core/tests/unit/dag_processing/test_manager.py:
##########
@@ -1359,6 +1359,25 @@ def test_send_file_processing_statsd_timing(
tags={"bundle_name": bundle_name, "file_name": dag_filename[:-3]},
)
+ @mock.patch("airflow.dag_processing.manager.stats.gauge")
+ def
test_log_file_processing_stats_sanitizes_last_run_seconds_ago_metric_name(self,
statsd_gauge_mock):
Review Comment:
```suggestion
def test_log_file_processing_stats_normalizes_metric_name(self,
statsd_gauge_mock):
```
--
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]
