o-nikolas commented on code in PR #36250:
URL: https://github.com/apache/airflow/pull/36250#discussion_r1450750079
##########
tests/core/test_stats.py:
##########
@@ -265,40 +271,128 @@ def
test_does_not_send_stats_using_statsd_when_statsd_and_dogstatsd_both_on(self
importlib.reload(airflow.stats)
-class TestStatsWithAllowList:
- def setup_method(self):
- self.statsd_client = Mock(spec=statsd.StatsClient)
- self.stats = SafeStatsdLogger(self.statsd_client,
AllowListValidator("stats_one, stats_two"))
+class TestStatsAllowAndBlockLists:
+ @pytest.mark.parametrize(
+ "validator, stat_name, expect_incr",
+ [
+ (PatternAllowListValidator, "stats_one", True),
+ (PatternAllowListValidator, "stats_two.bla", True),
+ (PatternAllowListValidator, "stats_three.foo", True),
+ (PatternAllowListValidator, "stats_foo_three", True),
+ (PatternAllowListValidator, "stats_three", False),
+ (AllowListValidator, "stats_one", True),
+ (AllowListValidator, "stats_two.bla", True),
+ (AllowListValidator, "stats_three.foo", False),
+ (AllowListValidator, "stats_foo_three", False),
+ (AllowListValidator, "stats_three", False),
+ (PatternBlockListValidator, "stats_one", False),
+ (PatternBlockListValidator, "stats_two.bla", False),
+ (PatternBlockListValidator, "stats_three.foo", False),
+ (PatternBlockListValidator, "stats_foo_three", False),
+ (PatternBlockListValidator, "stats_foo", False),
+ (PatternBlockListValidator, "stats_three", True),
+ (BlockListValidator, "stats_one", False),
+ (BlockListValidator, "stats_two.bla", False),
+ (BlockListValidator, "stats_three.foo", True),
+ (BlockListValidator, "stats_foo_three", True),
+ (BlockListValidator, "stats_three", True),
+ ],
+ )
+ def test_allow_and_block_list(self, validator, stat_name, expect_incr):
+ statsd_client = Mock(spec=statsd.StatsClient)
+ stats = SafeStatsdLogger(statsd_client, validator("stats_one,
stats_two, foo"))
+
+ stats.incr(stat_name)
+
+ if expect_incr:
+ statsd_client.incr.assert_called_once_with(stat_name, 1, 1)
+ else:
+ statsd_client.assert_not_called()
+
+ @pytest.mark.parametrize(
+ "match_pattern, expect_incr",
+ [
+ ("^stat", True),
+ ("s.{6}o", True),
+ ("^banana", False),
Review Comment:
Slight nit: all these patterns match from the beginning of the string either
by forcing it with ^ or just by chance (with the middle one). Since we want to
ensure this new feature can match in the middle, do you mind adding a regex
that does so? I think you could actually just make the 6 a 5 and it would match
in the middle.
##########
airflow/config_templates/config.yml:
##########
@@ -944,6 +944,14 @@ metrics:
description: |
StatsD (https://github.com/etsy/statsd) integration settings.
options:
+ metrics_use_pattern_match:
+ description: |
+ If true, metrics_allow_list and metrics_block_list will use regex
pattern matching in the metric
+ name and not just start of the name.
Review Comment:
A suggestion to make this even more concrete and spelled out. Seems obvious
to us working on it, but it's a slightly tricky one if you aren't familiar.
```suggestion
If true, metrics_allow_list and metrics_block_list will use regex
pattern matching anywhere within the metric
name instead of only prefix matching at the start of the 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]
