ashb commented on a change in pull request #12158:
URL: https://github.com/apache/airflow/pull/12158#discussion_r529514696
##########
File path: airflow/stats.py
##########
@@ -191,93 +443,121 @@ def test(self, stat):
return True # default is all metrics allowed
+def _format_safe_stats_logger_args(func):
+ @functools.wraps(func)
+ def func_wrap(_self, stat, *args, labels=None, **kwargs):
+ if labels:
+ # Remove {} from stat key e.g. {class_name} => class_name
+ stat = stat.format(**labels)
+ return validate_stat(func)(_self, stat, *args, **kwargs)
+
+ return func_wrap
+
+
class SafeStatsdLogger:
"""Statsd Logger"""
def __init__(self, statsd_client,
allow_list_validator=AllowListValidator()):
self.statsd = statsd_client
self.allow_list_validator = allow_list_validator
- @validate_stat
- def incr(self, stat, count=1, rate=1):
+ @_format_safe_stats_logger_args
+ def incr(self, stat, count=1, rate=1, labels=None):
"""Increment stat"""
+ del labels
Review comment:
Is this actually needed? Because of the _format_safe_stats_logger_args
decorator, this is never called with any value:
```
def func_wrap(_self, stat, *args, labels=None, **kwargs):
if labels:
# Remove {} from stat key e.g. {class_name} => class_name
stat = stat.format(**labels)
return validate_stat(func)(_self, stat, *args, **kwargs)
```
`labels` is an explicit parameter, so won't be in the kwargs passed to this
function (`func` in that snippet)
----------------------------------------------------------------
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]
