1fanwang opened a new pull request, #71529: URL: https://github.com/apache/airflow/pull/71529
A deployment that runs Airflow under `opentelemetry-instrument` with `OTEL_CONFIG_FILE` set loses its entire metrics configuration. Every view, reader and exporter declared in that file is thrown away, silently, and the operator gets Airflow's defaults instead. The OpenTelemetry [declarative configuration spec](https://opentelemetry.io/docs/specs/otel/configuration/sdk/#declarative-configuration) makes the file the sole source of SDK construction, so this is a spec violation rather than a preference. `get_otel_logger()` force-resets the SDK's `Once` guard and calls `set_meter_provider()` unconditionally, so whatever the agent bootstrap installed is discarded before Airflow's own provider goes in. The reset was added deliberately for #64690, where a forked child inherits the parent's `_done = True` and ends up on a provider whose reader thread is dead. That case is untouched: the reset still runs on every path where Airflow owns the provider. The change only declines to replace a provider that came from `OTEL_CONFIG_FILE`. Scoped to `OTEL_CONFIG_FILE` on purpose. The env-var bootstrap path has no equivalent "sole source" rule and Airflow's own `[metrics] otel_*` settings legitimately compete with it there, so leaving that behaviour alone keeps this a bug fix. related: #64690 Textual overlap with https://github.com/apache/airflow/pull/68393, which restructures this module but leaves the clobber in place. Happy to rebase on top of it if that lands first. ### Testing Done A script mirroring what `opentelemetry-instrument airflow scheduler` does: run `_OTelSDKConfigurator()._configure()` with `OTEL_CONFIG_FILE` pointing at a config declaring a `*_duration` view, then call `get_otel_logger()` and inspect the installed provider. ```yaml # otel-config.yaml file_format: "1.0-rc.1" meter_provider: readers: - periodic: interval: 60000 exporter: console: {} views: - selector: instrument_name: "*_duration" stream: aggregation: explicit_bucket_histogram: boundaries: [0.5, 1, 2, 4, 8] ``` ```python # repro.py import os os.environ["OTEL_CONFIG_FILE"] = "otel-config.yaml" from opentelemetry import metrics from opentelemetry.sdk._configuration import _OTelSDKConfigurator def views_of(provider): sdk_config = getattr(provider, "_sdk_config", None) return [] if sdk_config is None else [str(v._instrument_name) for v in sdk_config.views] _OTelSDKConfigurator()._configure() # what opentelemetry-instrument runs before = metrics.get_meter_provider() print("bootstrap views:", views_of(before)) from airflow_shared.observability.metrics.otel_logger import get_otel_logger get_otel_logger(host="localhost", port=4318) after = metrics.get_meter_provider() print("after get_otel_logger:", views_of(after)) print("same provider object?", before is after) ``` Before, on `main` — the declarative view is gone and the provider has been swapped for Airflow's, whose only view is the instrument-type baseline: <details><summary>Raw output</summary> ``` $ pip install opentelemetry-configuration $ python repro.py after opentelemetry-instrument bootstrap: MeterProvider declarative views: ['*_duration'] after get_otel_logger(): MeterProvider declarative views: ['None'] same provider object? False RESULT: declarative view was DISCARDED ``` </details> After, with this change — the provider and its view survive: <details><summary>Raw output</summary> ``` $ python repro.py after opentelemetry-instrument bootstrap: MeterProvider declarative views: ['*_duration'] after get_otel_logger(): MeterProvider declarative views: ['*_duration'] same provider object? True RESULT: declarative view survived ``` </details> `test_declaratively_configured_provider_is_not_replaced` covers this through the real `get_otel_logger()` against a real SDK `MeterProvider`, and fails on unpatched sources: <details><summary>Raw output</summary> ``` # revert just the fixed file to the released behaviour $ git checkout origin/main -- shared/observability/src/airflow_shared/observability/metrics/otel_logger.py $ uv run --project shared/observability pytest \ shared/observability/tests/observability/metrics/test_otel_logger.py -q -k declarative FAILED ...::test_declaratively_configured_provider_is_not_replaced assert logger.otel is configured_provider E assert <...MeterProvider object at 0x10a252ec0> is <...MeterProvider object at 0x109fb0190> 1 failed, 1 passed, 51 deselected # restore the fix $ git checkout HEAD -- shared/observability/src/airflow_shared/observability/metrics/otel_logger.py $ uv run --project shared/observability pytest \ shared/observability/tests/observability/metrics/test_otel_logger.py -q -k declarative 2 passed, 51 deselected ``` </details> `test_provider_is_replaced_without_declarative_config` pins the other side, so the fix can't quietly widen into "never replace any provider". The #64690 re-init path still exports after a second `get_otel_logger()` call: <details><summary>Raw output</summary> ``` $ python -c " from airflow_shared.observability.metrics.otel_logger import get_otel_logger, flush_otel_metrics get_otel_logger(debug=True) logger = get_otel_logger(debug=True) logger.incr('post_fork_stat') flush_otel_metrics()" | grep -c post_fork_stat 3 ``` </details> Full suite is green apart from `test_reinit_after_fork_exports_metrics`, a pre-existing local-environment problem — a `tests` package in site-packages shadows the repo's, so the subprocess `import tests.observability...` fails identically on unmodified `main`. <details><summary>Raw output</summary> ``` $ uv run --project shared/observability pytest shared/observability/tests/observability/ -q FAILED ...::test_reinit_after_fork_exports_metrics 1 failed, 183 passed, 2 warnings in 2.16s $ python -c "import tests; print(tests.__path__)" ['<repo>/.venv/lib/python3.10/site-packages/tests'] ``` </details> -- 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]
