Skip to content

Bring back edge worker metric compatibility with Airflow 3.2#67328

Merged
potiuk merged 10 commits into
apache:mainfrom
boschglobal:bugfix/edge-worker-export-metrics-with-dual-stats-manager
Jul 19, 2026
Merged

Bring back edge worker metric compatibility with Airflow 3.2#67328
potiuk merged 10 commits into
apache:mainfrom
boschglobal:bugfix/edge-worker-export-metrics-with-dual-stats-manager

Conversation

@AutomationDev85

@AutomationDev85 AutomationDev85 commented May 22, 2026

Copy link
Copy Markdown
Contributor

Overview

After switching to edge3 provider version 3.6.0, we observed that edge metrics were exposed without worker_name tags. During root-cause analysis, we found that metrics were no longer exported through DualStatsManager. As a result, StatsD mappings broke because the worker name was no longer included in the metric name.

In Airflow version 3.2.1, this behavior was handled by DualStatsManager. As we understand it, the new implementation that replaces DualStatsManager will address this, but it is planned for Airflow 3.3. Therefore, this change reintroduces the DualStatsManager check to keep edge worker metrics compatible with Airflow 3.2.

Details of change:

  • Add DualStatsManager check for Airflow 3.2 compatibility

Important

🛠️ Maintainer triage note for @AutomationDev85 · by @potiuk · 2026-07-02 17:46 UTC

Some review feedback from @xBis7 is waiting on you:

  • 1 unresolved review thread(s) from @xBis7 need a reply or a fix.

The ball is in your court — you've been assigned to this PR. Reply or push a fix in each thread, then mark them resolved.

Automated triage — may be imperfect; a maintainer takes the next look.

@boring-cyborg boring-cyborg Bot added area:providers provider:edge Edge Executor / Worker (AIP-69) / edge3 labels May 22, 2026
@jscheffl jscheffl changed the title [DT Flow] Bring back edge worker metric compatibility with Airflow 3.2 Bring back edge worker metric compatibility with Airflow 3.2 May 22, 2026
@jscheffl

Copy link
Copy Markdown
Contributor

@xBis7 this PR was made in relation to the cleanup in #63932 which it seems broke monitoring. Aif Airflow 3.2.x is used and Common-Compat is upgraded (which PR was this?) what is the best strategy to fix it back? Brnging back DualStasMgr into Edge seems to be counter productive to what you cleaned, obviously there is some drift. Does tag handling need to be added in Common-Compat provider somewhere?

@xBis7 xBis7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes adding back the DualStatsManager is counter productive. In addition, we added a Stats shim so that imports won't get affected but only the latest code has the dual stat emission logic in the Stats.X() calls.

Because the imports have stayed the same, I think in this case, it makes more sense to use a version check. The code should look like

if AIRFLOW_V_3_3_PLUS:
      Stats.gauge(
        "edge_worker.status",
        sysinfo.get("status", logging.NOTSET),  # type: ignore
        tags={"worker_name": worker_name},
    )
    Stats.gauge("edge_worker.connected", int(connected), tags={"worker_name": worker_name})
    Stats.gauge("edge_worker.maintenance", int(maintenance), tags={"worker_name": worker_name})
    Stats.gauge("edge_worker.jobs_active", jobs_active, tags={"worker_name": worker_name})
    Stats.gauge("edge_worker.concurrency", concurrency, tags={"worker_name": worker_name})
    Stats.gauge("edge_worker.free_concurrency", free_concurrency, tags={"worker_name": worker_name})
    Stats.gauge(
        "edge_worker.num_queues",
        len(queues),
        tags={"worker_name": worker_name, "queues": ",".join(queues)},
    )

    for key in additional_keys:
        value = sysinfo.get(key)
        if isinstance(value, (int, float)):
            Stats.gauge(f"edge_worker.{key}", value, tags={"worker_name": worker_name})
else:
    Stats.gauge(f"edge_worker.status.{worker_name}", sysinfo.get("status", logging.NOTSET))  # type: ignore
    Stats.gauge(
        "edge_worker.status",
        sysinfo.get("status", logging.NOTSET),  # type: ignore
        tags={"worker_name": worker_name},
    )

    Stats.gauge(f"edge_worker.connected.{worker_name}", int(connected))
    Stats.gauge("edge_worker.connected", int(connected), tags={"worker_name": worker_name})

    Stats.gauge(f"edge_worker.maintenance.{worker_name}", int(maintenance))
    Stats.gauge("edge_worker.maintenance", int(maintenance), tags={"worker_name": worker_name})

    Stats.gauge(f"edge_worker.jobs_active.{worker_name}", jobs_active)
    Stats.gauge("edge_worker.jobs_active", jobs_active, tags={"worker_name": worker_name})

    Stats.gauge(f"edge_worker.concurrency.{worker_name}", concurrency)
    Stats.gauge("edge_worker.concurrency", concurrency, tags={"worker_name": worker_name})

    Stats.gauge(f"edge_worker.free_concurrency.{worker_name}", free_concurrency)
    Stats.gauge("edge_worker.free_concurrency", free_concurrency, tags={"worker_name": worker_name})

    Stats.gauge(f"edge_worker.num_queues.{worker_name}", len(queues))
    Stats.gauge(
        "edge_worker.num_queues",
        len(queues),
        tags={"worker_name": worker_name, "queues": ",".join(queues)},
    )

    for key in additional_keys:
        value = sysinfo.get(key)
        if isinstance(value, (int, float)):
            Stats.gauge(f"edge_worker.{key}.{worker_name}", value)
            Stats.gauge(f"edge_worker.{key}", value, tags={"worker_name": worker_name})

Since this issue wasn't caught by the back compat CI steps, either the unit tests don't check for both new and legacy metric names or the assertions don't run. Can you extend some of the unit tests? It will help us identify similar issues in the future.

Comment thread providers/edge3/src/airflow/providers/edge3/models/edge_worker.py Outdated
@potiuk

potiuk commented May 24, 2026

Copy link
Copy Markdown
Member

@AutomationDev85 A few things need addressing before review — see our Pull Request quality criteria.

  • Static checks fail: MyPy providers checks — type errors in the provider changes. See the failing job.
  • Compat tests fail: provider distributions tests / Compat 3.2.1:P3.10: — backward-compat check against Airflow 3.2.1 is red. See the failing job.

No rush.


Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@diogosilva30

diogosilva30 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@AutomationDev85 you saw metrics in your metrics backend without the worker label or metrics completely gone?

Because I'm on Airflow 3.2.2, edge3 provider 3.7.0, statsd_exporter v0.28.0 and we did saw all edge worker metrics completely gone, but due to bug I've described in #68077. After implementing a fix I do see all the metrics back including the worker label, using the statsd mapping:

-  match: airflow.edge_worker.concurrency.*
    name: "airflow_edge_worker_concurrency"
    labels:
       worker_name: "$1"
image

@AutomationDev85

Copy link
Copy Markdown
Contributor Author

@AutomationDev85 you saw metrics in your metrics backend without the worker label or metrics completely gone?

Because I'm on Airflow 3.2.2, edge3 provider 3.7.0, statsd_exporter v0.28.0 and we did saw all edge worker metrics completely gone, but due to bug I've described in #68077. After implementing a fix I do see all the metrics back including the worker label, using the statsd mapping:

-  match: airflow.edge_worker.concurrency.*
    name: "airflow_edge_worker_concurrency"
    labels:
       worker_name: "$1"
image

I noticed the issue with metrics missing the worker label. In our Airflow deployment, we had already patched this locally to address the same problem you encountered. Since the fix is already on main, we took the fastest path to run Airflow 3.2 with the latest Edge Worker package. We are now waiting for the Airflow 3.3 release so we can remove our local workaround.

@AutomationDev85
AutomationDev85 force-pushed the bugfix/edge-worker-export-metrics-with-dual-stats-manager branch from d79b934 to 01ca3ca Compare June 8, 2026 14:15
@jscheffl

jscheffl commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@AutomationDev85 you saw metrics in your metrics backend without the worker label or metrics completely gone?
Because I'm on Airflow 3.2.2, edge3 provider 3.7.0, statsd_exporter v0.28.0 and we did saw all edge worker metrics completely gone, but due to bug I've described in #68077. After implementing a fix I do see all the metrics back including the worker label, using the statsd mapping:

-  match: airflow.edge_worker.concurrency.*
    name: "airflow_edge_worker_concurrency"
    labels:
       worker_name: "$1"

I noticed the issue with metrics missing the worker label. In our Airflow deployment, we had already patched this locally to address the same problem you encountered. Since the fix is already on main, we took the fastest path to run Airflow 3.2 with the latest Edge Worker package. We are now waiting for the Airflow 3.3 release so we can remove our local workaround.

@diogosilva30 @AutomationDev85 do you think in regards of this metrics "trouble" and changes, do we need to add some sectiont o docs to describe what metrics template to adjust to make it working in different Airflow versions such that others do not need to reverse-engineer the same problems? Some small note in RST docs?

@AutomationDev85

Copy link
Copy Markdown
Contributor Author

@jscheffl @diogosilva30 I tried to sum up the current situation of the metrics export in a compatibility table. What do you think about this?

I think it is also important to check this PR #68078 for the Airflow 3.3 release.

@AutomationDev85
AutomationDev85 force-pushed the bugfix/edge-worker-export-metrics-with-dual-stats-manager branch from 36652ea to ede104f Compare June 9, 2026 07:53
@jscheffl
jscheffl requested a review from xBis7 June 9, 2026 09:24
@jscheffl

jscheffl commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@xBis7 OK for you like it is now?

Comment thread providers/edge3/docs/edge_executor.rst Outdated
Comment thread providers/edge3/docs/edge_executor.rst Outdated

@xBis7 xBis7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AutomationDev85 Thanks for the changes! The code looks good and it should work! But the doc changes are inaccurate and need updating. I've added 2 comments.

I'm still thinking that we should have tests that catch scenarios of metrics going missing.

@AutomationDev85

AutomationDev85 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@AutomationDev85 Thanks for the changes! The code looks good and it should work! But the doc changes are inaccurate and need updating. I've added 2 comments.

I'm still thinking that we should have tests that catch scenarios of metrics going missing.

@xBis7 I this is a good idea but I have no idea at the moment how to implement this in an easy way. Do you have an idea? Looks for me like a new PR?

Comment thread providers/edge3/docs/edge_executor.rst Outdated
Comment thread providers/edge3/docs/edge_executor.rst Outdated
@xBis7

xBis7 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Do you have an idea? Looks for me like a new PR?

I would suggest adding them as part of this PR so that we can also verify that your fix is the right one.

I would add a new test file providers/edge3/tests/unit/edge3/models/test_edge_worker.py and then call set_metrics() and test that both new and legacy metrics have been exported. You don't have to assert on all the metrics or their tags. Just pick 1 metric, e.g. if the worker name is test_worker1, then check that you can find both edge_worker.status and edge_worker.status.test_worker1.

If we are in AIRFLOW_V_3_3_PLUS then Stats will be delegating to the module-level functions of stats which will then do the dual emmission. For example,

@mock.patch("airflow.sdk._shared.observability.metrics.stats._get_backend")
def test_set_metrics(mock_get_backend):
    mock_backend = mock.MagicMock(spec=StatsLogger)
    mock_get_backend.return_value = mock_backend
    set_metrics(
        worker_name="test_worker1",
        state=EdgeWorkerState.IDLE,
        jobs_active=0,
        concurrency=1,
        free_concurrency=1,
        queues=None,
        sysinfo={"status": 1},
    )
    metric_names = [call.args[0] for call in mock_backend.gauge.call_args_list]
    print(f"captured metrics: {metric_names}")
    assert "edge_worker.status" in metric_names
    assert "edge_worker.status.test_worker1" in metric_names

and for older versions you should patch Stats itself.

Here are some examples of handling back-compat in providers

https://github.com/xBis7/airflow/blob/main/providers/openlineage/tests/unit/openlineage/plugins/test_adapter.py#L70

https://github.com/xBis7/airflow/blob/main/providers/openlineage/tests/unit/openlineage/plugins/test_adapter.py#L307-L308

https://github.com/xBis7/airflow/blob/main/providers/celery/tests/unit/celery/executors/test_celery_executor.py#L57-L65

@AutomationDev85

Copy link
Copy Markdown
Contributor Author

@xBis7 Sorry really busy days at the moment. I added the unitest. Could you take a look at these changes when you have a moment?

@xBis7 xBis7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AutomationDev85 Thank you for addressing all my comments. Now that we have a unit test, we will avoid similar issues in the future. LGTM!

@xBis7

xBis7 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

@jscheffl From my point of view, this is good for a merge.

@potiuk

potiuk commented Jun 29, 2026

Copy link
Copy Markdown
Member

@AutomationDev85 - WDYT? - I am happy to merge if you are OK.

@AutomationDev85

Copy link
Copy Markdown
Contributor Author

@potiuk Thanks for picking this up! Yes from my point of view this is ready to be merged.

@AutomationDev85
AutomationDev85 force-pushed the bugfix/edge-worker-export-metrics-with-dual-stats-manager branch 2 times, most recently from bad3d30 to a073770 Compare July 14, 2026 07:29
@AutomationDev85
AutomationDev85 force-pushed the bugfix/edge-worker-export-metrics-with-dual-stats-manager branch from a073770 to 8893052 Compare July 14, 2026 12:47
@AutomationDev85

Copy link
Copy Markdown
Contributor Author

@potiuk I fixed the latest merge conflicts. Can you check again?

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jul 18, 2026
@potiuk
potiuk merged commit 64f9cbd into apache:main Jul 19, 2026
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:edge Edge Executor / Worker (AIP-69) / edge3 ready for maintainer review Set after triaging when all criteria pass.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants