Repository navigation
Conversation
Issue apache#42492 asked for a new Airflow config key (otel_path) because the Airflow-config-based OTel setup (otel_host/otel_port) hardcodes the /v1/metrics suffix, which breaks collectors that expect a different path (e.g. VictoriaMetrics' /opentelemetry/api/v1/push). Investigating get_otel_data_exporter() showed that path is only appended in that legacy Airflow-config fallback. As soon as any standard OTel endpoint env var is set (OTEL_EXPORTER_OTLP_ENDPOINT or the type-specific OTEL_EXPORTER_OTLP_METRICS_ENDPOINT), the code hands off directly to OTLPMetricExporter() with no endpoint argument and lets the SDK read the env vars itself — so a full custom-path URL in OTEL_EXPORTER_OTLP_METRICS_ENDPOINT already passes through untouched. This was simply never documented, and adding another Airflow config key would cut against the project's existing move to deprecate the Airflow-config OTel keys in favor of the standard env vars. The existing test covering type-specific endpoint precedence used the gRPC protocol, whose targets are always host:port with no path component, so it could not have caught a regression in path preservation even though its name suggested it was general-purpose precedence coverage. Switched it to the HTTP protocol with a path in the endpoint so it actually backs the documented behavior.
potiuk
added
the
closed because of open PR limit
|
Hello @ColtenOuO - thank you for your contributions to Apache Airflow! The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 19 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:
These pull requests stay open because maintainers are already engaged in them - they count towards your limit:
This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are. What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack. Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst. Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting |
2 participants
Summary
Issue #42492 reports that Airflow's OTel metrics exporter can't send metrics to a non-default path (e.g. VictoriaMetrics'
/opentelemetry/api/v1/push), and asks for a newotel_pathAirflow config option to fix it — mirroring howotel_host/otel_portalready work.Looking at
get_otel_data_exporter()inshared/observability/src/airflow_shared/observability/common.py, the/v1/metricssuffix is only ever appended in the legacy fallback branch, when neitherOTEL_EXPORTER_OTLP_ENDPOINTnorOTEL_EXPORTER_OTLP_METRICS_ENDPOINTis set and the code falls back to the deprecatedotel_host/otel_portAirflow config keys. As soon as either of those standard OTel endpoint env vars is set, the code callsOTLPMetricExporter()with no endpoint argument at all and lets the OpenTelemetry SDK read the env var itself — which means a full custom-path URL inOTEL_EXPORTER_OTLP_METRICS_ENDPOINTalready passes through completely untouched today. Verified this against the realopentelemetry-exporter-otlp-proto-httppackage.So the capability the issue asks for already exists -- it's just undocumented.
While writing the doc, found that the one existing test that covered "type-specific endpoint env var takes precedence over the base one" (
test_config_priorities[type_specific_vars_take_precedence]) used the gRPC protocol for its type-specific case. gRPC targets are alwayshost:port— there's no path component in a gRPC target — so that test could not have caught a regression in path handling no matter what URL was fed into it; it only proved precedence, not path preservation, despite the surrounding docs claim it would be backing. Updated it to use the HTTP protocol with a path-bearing endpoint, which is the same shape as the example in the new docs section, so the test actually guards the behavior being documented.Changes
airflow-core/docs/administration-and-deployment/logging-monitoring/metrics.rst: added a note under the deprecatedotel_*config keys explaining thatOTEL_EXPORTER_OTLP_METRICS_ENDPOINTcan be set to a complete custom-path endpoint, that it takes precedence overOTEL_EXPORTER_OTLP_ENDPOINT, and that doing so avoids the default/v1/metricssuffix — with a runnable example matching a VictoriaMetrics-style push path.shared/observability/tests/observability/metrics/test_otel_logger.py: replaced the gRPC-protocol precedence case with an HTTP-protocol case whose endpoint carries a custom path, so the test actually exercises and locks in the path-preservation behavior described in the new docs.closes: #42492
Was generative AI tooling used to co-author this PR?