mik-laj commented on a change in pull request #12158:
URL: https://github.com/apache/airflow/pull/12158#discussion_r519178143
##########
File path: airflow/models/dagbag.py
##########
@@ -471,7 +471,7 @@ def collect_dags(
# file_stat.file similar format: /subdir/dag_name.py
# TODO: Remove for Airflow 2.0
filename = file_stat.file.split('/')[-1].replace('.py', '')
- Stats.timing(f'dag.loading-duration.{filename}',
file_stat.duration)
+ Stats.timing('dag.loading-duration.{filename}',
file_stat.duration, labels={"filename": filename})
Review comment:
The only problem is that not every client supports tags, and some
clients support tags in a different way than the official Datadog. For this
reason, I preferred not to add more keys, but leave these decisions to the
statsd client.
If the client supports the tags, it will keep metrics in the best form for
itself.
We currently have two clients:
- Classic statsd client (without tags support), which in this case will get
a metric - `dag.loading-duration.my-awesome-file.py` without tags
- Datadog statsd (with tags support), which in this in this case will get a
metric - `dag.loading-duration.filename` with tag `filename:my-awesome-file.py`
So you can aggregate this data if you need it.
This key name is not ideal when the client supports tags, but we don't need
to generate the metrics key twice. On the other hand, writing the available
parameters in the name of the metrics is not a stupid idea. This can help us
find the tags more easily.
It is also worth adding that Stackdriver stores labels differently. Instead
of tags, it uses a dictionary that has a normal key and value
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
