kaxil commented on code in PR #68284:
URL: https://github.com/apache/airflow/pull/68284#discussion_r3384350636
##########
shared/observability/src/airflow_shared/observability/metrics/otel_logger.py:
##########
@@ -299,7 +303,12 @@ def timer(
**kwargs,
) -> Timer:
"""Timer context manager returns the duration and can be cancelled."""
- return _OtelTimer(self, stat, tags)
+ safe_stat = (
+ stat
+ if stat is not None and self.metrics_validator.test(stat) and
name_is_otel_safe(self.prefix, stat)
Review Comment:
The `metrics_validator.test(stat) and name_is_otel_safe(self.prefix, stat)`
predicate now appears six times across `incr`, `decr`, `gauge` (twice),
`timing`, and `timer`, and this bug happened because two methods missed it.
Maybe pull it into a helper so the next recording method can't forget it:
```python
def _is_recordable(self, stat: str | None) -> bool:
return bool(stat) and self.metrics_validator.test(stat) and
name_is_otel_safe(self.prefix, stat)
```
##########
shared/observability/src/airflow_shared/observability/metrics/otel_logger.py:
##########
@@ -270,12 +270,16 @@ def gauge(
if _skip_due_to_rate(rate):
return
- if back_compat_name and self.metrics_validator.test(back_compat_name):
+ if (
+ back_compat_name
+ and self.metrics_validator.test(back_compat_name)
+ and name_is_otel_safe(self.prefix, back_compat_name)
+ ):
self.metrics_map.set_gauge_value(
full_name(prefix=self.prefix, name=back_compat_name), value,
delta, tags
)
- if self.metrics_validator.test(stat):
+ if self.metrics_validator.test(stat) and
name_is_otel_safe(self.prefix, stat):
Review Comment:
FYI this will spam the logs for the reported case: `name_is_otel_safe()`
logs `Invalid stat name: ...` every time the DagProcessor emits this gauge, so
`PBI_SKU_Performance copy.py` produces a recurring warning for as long as the
file exists. `incr()` already does the same for invalid names, so not blocking.
If the noise comes up later, a log-once-per-name dedupe in `name_is_otel_safe`
would handle it.
--
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]
