dkranchii commented on PR #68152:
URL: https://github.com/apache/airflow/pull/68152#issuecomment-6069913203
Thanks for the detailed review and for the context on the earlier change
request.
Rebased onto current main and applied the `prune_dict` tags so the events
timer matches the heartbeat timer:
```python
tags=prune_dict(
{
"executor": type(executor).__name__,
"team_name": executor.team_name,
}
),
```
`test_process_executor_events_emits_timer` now follows the same shape as
`test_executor_heartbeat_emits_timer` — one executor with `team_name` set, one
without, building `expected_tags` conditionally. I checked it is a real
regression test: reverting the tag change makes it fail on exactly the missing
`team_name: team_a` tag, which is the multi-team case you described.
One thing I left alone deliberately. The registry description for
`scheduler.executor_events_duration` still reads "tagged by executor class
name", but so does the `scheduler.executor_heartbeat_duration` entry directly
above it, even though that metric already carries the `team_name` tag on main.
Updating only mine would leave the two siblings inconsistent, so I kept them
symmetric. Happy to update both here, or leave it for a follow-up — whichever
you prefer.
The rebase also means CI runs fresh rather than against the July state.
@ashb — when you have a moment, could you take another look? The
commit-history concern from your June review is resolved (single commit, no
merge commit), so this is just waiting on that change request being lifted.
---
Drafted-by: Cursor (Claude Opus 5);
reviewed by @dkranchii before posting
--
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]
