Skip to content

Remove the DualStatsManager and the Stats interfaces#63932

Merged
ashb merged 52 commits into
apache:mainfrom
xBis7:simplify-stats
Apr 30, 2026
Merged

Remove the DualStatsManager and the Stats interfaces#63932
ashb merged 52 commits into
apache:mainfrom
xBis7:simplify-stats

Conversation

@xBis7

@xBis7 xBis7 commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

This patch is removing the DualStatsManager and the Stats interfaces.

Action items

  • DualStatsManager
    • Remove the DualStatsManager
    • Move all the DualStatsManager logic under _shared/stats.py
    • Rename the extra_tags parameters to legacy_name_tags
    • Use more intuitive function names for the methods copied from the DualStatsManager
  • Stats
    • Remove the metaclass
    • Replace all metrics calls using the Stats class by calls using the stats module
    • Convert all stats methods, incr, decr, gauge, etc. to module-level functions
    • Use a module-level variable for holding the backend that will be used
      • We used to store the backend implementation to the Stats.instance class attribute
    • Use a PID check to handle process forks
      • It used to check if the Stats.instance is None and then reinitialize
  • providers - back compat
    • Add a Stats shim, to continue handling imports using sdk.py
    • If Airflow >= 3.3
      • Use the new module-level functions stats.incr, stats.gauge, etc. through the Stats shim
      • The DualStatsManager logic exists under the new stats functions
      • Stats now has extra parameters for the DualStatsManager logic
    • For previous version
      • Import and use the old Stats interface

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
    Claude Sonnet 4.6 Extended

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@xBis7

xBis7 commented Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

cc @dstandish @ashb @potiuk

@xBis7 xBis7 force-pushed the simplify-stats branch 2 times, most recently from 0e5c7f8 to 91f408e Compare March 19, 2026 14:42
Comment thread airflow-core/src/airflow/observability/stats.py Outdated
Comment thread providers/edge3/src/airflow/providers/edge3/models/edge_worker.py Outdated
Comment thread scripts/ci/prek/check_metrics_synced_with_the_registry.py

@dstandish dstandish 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.

looks ok to me. can you summarize the potential backcompat issues?

@xBis7

xBis7 commented Mar 20, 2026

Copy link
Copy Markdown
Contributor Author

can you summarize the potential backcompat issues?

@dstandish Any potential backcompat issues are with the providers. Because they have different versioning, the latest version could run against some version of airflow-core or task-sdk prior to 3.2, in which case they will need to import and use the old Stats... class.

@xBis7 xBis7 force-pushed the simplify-stats branch 5 times, most recently from 0c670de to 80c947b Compare March 30, 2026 07:33
@xBis7 xBis7 marked this pull request as ready for review March 30, 2026 16:14
@ashb

ashb commented Apr 30, 2026

Copy link
Copy Markdown
Member

🤞🏻

@ashb ashb merged commit 67bab4a into apache:main Apr 30, 2026
143 checks passed
@xBis7 xBis7 deleted the simplify-stats branch April 30, 2026 18:55
@xBis7

xBis7 commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

@ashb @jscheffl Thank you!

@jscheffl

Copy link
Copy Markdown
Contributor

Wohoo! landed!

#protm

@kacpermuda

Copy link
Copy Markdown
Collaborator

Just FYI, the OL provider needed to be adjusted (#66351 ) after this change, we were using get_stats_factory, and the removal of arg that this function expected was a breaking change. I think it was the only place changed in this PR, where get_stats_factory is used in a provider, so hopefully no other places will be impacted.

@xBis7

xBis7 commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

@kacpermuda Sorry for the inconvenience. I didn't put much thought into it because the back compat CI step for 3.2.1 was green.

https://github.com/apache/airflow/actions/runs/25169748334/job/73789176552?pr=63932

@kacpermuda

Copy link
Copy Markdown
Collaborator

No problem, we caught that early, we should have had tests for that and now we'll add them, sounds like a win to me.

jscheffl added a commit that referenced this pull request Jun 12, 2026
…g; backport fix for 3.2.x) (#68078)

* Initialize Task SDK Stats in the API server so Edge Worker metrics are emitted

The API server serves the Edge Worker REST API (/edge_worker/v1/...) whose
heartbeat handler records edge_worker.* metrics through the Task SDK Stats
singleton (resolved by the Edge provider via airflow.providers.common.compat).

Unlike the scheduler, triggerer, dag-processor, executors and task runner, the
API server never called Stats.initialize(...). After the auto-initializing Stats
was removed in #63932, that singleton stays a NoStatsLogger in the API server
process and every Edge Worker metric is silently dropped.

Initialize the Task SDK Stats singleton from the FastAPI lifespan (runs once per
worker, post-fork), mirroring the existing init in serde/task_runner. The call is
guarded so a metrics misconfiguration can never block API server startup.

Closes: #68077
Signed-off-by: Diogo Silva <diogo.silva@five9.com>

* Add newsfragment for #68078

Signed-off-by: Diogo Silva <diogo.silva@five9.com>

* fix: fix ruff errors

* Remove newsfragment — bug fix does not require a user-facing changelog entry

* Make API server Stats init log and docstring metric-agnostic

* Update airflow-core/src/airflow/api_fastapi/app.py

Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>

* Update airflow-core/tests/unit/api_fastapi/test_app.py

Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>

---------

Signed-off-by: Diogo Silva <diogo.silva@five9.com>
Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
imrichardwu pushed a commit to imrichardwu/airflow that referenced this pull request Jun 16, 2026
…g; backport fix for 3.2.x) (apache#68078)

* Initialize Task SDK Stats in the API server so Edge Worker metrics are emitted

The API server serves the Edge Worker REST API (/edge_worker/v1/...) whose
heartbeat handler records edge_worker.* metrics through the Task SDK Stats
singleton (resolved by the Edge provider via airflow.providers.common.compat).

Unlike the scheduler, triggerer, dag-processor, executors and task runner, the
API server never called Stats.initialize(...). After the auto-initializing Stats
was removed in apache#63932, that singleton stays a NoStatsLogger in the API server
process and every Edge Worker metric is silently dropped.

Initialize the Task SDK Stats singleton from the FastAPI lifespan (runs once per
worker, post-fork), mirroring the existing init in serde/task_runner. The call is
guarded so a metrics misconfiguration can never block API server startup.

Closes: apache#68077
Signed-off-by: Diogo Silva <diogo.silva@five9.com>

* Add newsfragment for apache#68078

Signed-off-by: Diogo Silva <diogo.silva@five9.com>

* fix: fix ruff errors

* Remove newsfragment — bug fix does not require a user-facing changelog entry

* Make API server Stats init log and docstring metric-agnostic

* Update airflow-core/src/airflow/api_fastapi/app.py

Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>

* Update airflow-core/tests/unit/api_fastapi/test_app.py

Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>

---------

Signed-off-by: Diogo Silva <diogo.silva@five9.com>
Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
dingo4dev pushed a commit to dingo4dev/airflow that referenced this pull request Jun 16, 2026
…g; backport fix for 3.2.x) (apache#68078)

* Initialize Task SDK Stats in the API server so Edge Worker metrics are emitted

The API server serves the Edge Worker REST API (/edge_worker/v1/...) whose
heartbeat handler records edge_worker.* metrics through the Task SDK Stats
singleton (resolved by the Edge provider via airflow.providers.common.compat).

Unlike the scheduler, triggerer, dag-processor, executors and task runner, the
API server never called Stats.initialize(...). After the auto-initializing Stats
was removed in apache#63932, that singleton stays a NoStatsLogger in the API server
process and every Edge Worker metric is silently dropped.

Initialize the Task SDK Stats singleton from the FastAPI lifespan (runs once per
worker, post-fork), mirroring the existing init in serde/task_runner. The call is
guarded so a metrics misconfiguration can never block API server startup.

Closes: apache#68077
Signed-off-by: Diogo Silva <diogo.silva@five9.com>

* Add newsfragment for apache#68078

Signed-off-by: Diogo Silva <diogo.silva@five9.com>

* fix: fix ruff errors

* Remove newsfragment — bug fix does not require a user-facing changelog entry

* Make API server Stats init log and docstring metric-agnostic

* Update airflow-core/src/airflow/api_fastapi/app.py

Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>

* Update airflow-core/tests/unit/api_fastapi/test_app.py

Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>

---------

Signed-off-by: Diogo Silva <diogo.silva@five9.com>
Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants