airflow icon indicating copy to clipboard operation
airflow copied to clipboard

Refactor `SafeDogStatsdLogger` to use `get_validator` to enable pattern matching

Open rawwar opened this issue 1 year ago • 2 comments

closes #39368

By reusing the get_validators function, we can incorporate pattern search to include allow/block lists.

rawwar avatar May 02 '24 11:05 rawwar

@pankajkoti, Yes, I have tested them. Allow/Block list without pattern worked perfectly. However, I noticed the following behaviour when using pattern matching, which I am not sure if it's right.

With pattern matching enabled:

  1. AIRFLOW__METRICS__METRICS_ALLOW_LIST = .*queued_duration.* : Worked perfectly
image
  1. AIRFLOW__METRICS__METRICS_ALLOW_LIST = .*get_astronauts.* : Not sure if this is correct
image

In the 2nd screenshot, I still see several metrics that don't have get_astronauts anywhere in their metrics names. Is this correct behaviour? One of them is airflow.ti.finish

rawwar avatar May 03 '24 06:05 rawwar

Block lists seem to be working as intended.

  1. AIRFLOW__METRICS__METRICS_BLOCK_LIST = .*duration.* : works perfectly
image

FYI: I have been using different tags for different settings to differentiate between searches.

rawwar avatar May 03 '24 06:05 rawwar

Env variables set:

AIRFLOW__METRICS__STATSD_DATADOG_ENABLED true
AIRFLOW__METRICS__STATSD_DATADOG_TAGS testNo:C2
DATADOG_API_KEY ● ● ● ● ● ● ●
AIRFLOW__METRICS__METRICS_USE_PATTERN_MATCH true
AIRFLOW__METRICS__METRICS_ALLOW_LIST .*print_astronaut_craft.*

rawwar avatar May 03 '24 11:05 rawwar

Code change LGTM, but I'm not sure that second allow-list example (the one you seemed unsure about) is working as expected. Either way, that is not in scope for this PR if that is broken, but you may want to look into that more if you have the time.

I will look into it. I will create an issue( #39400) and post my findings there.

rawwar avatar May 04 '24 00:05 rawwar