andreahlert commented on code in PR #56150:
URL: https://github.com/apache/airflow/pull/56150#discussion_r2775020336
##########
shared/observability/src/airflow_shared/observability/traces/otel_tracer.py:
##########
@@ -339,18 +343,70 @@ def get_otel_tracer(
otel_service: str | None = None,
debug: bool = False,
) -> OtelTrace:
- """Get OTEL tracer from airflow configuration."""
+ """Get OTEL tracer from the regular OTel env variables or the airflow
configuration."""
+ otel_env_config = load_traces_env_config()
+
tag_string = cls.get_constant_tags()
protocol = "https" if ssl_active else "http"
- # Allow transparent support for standard OpenTelemetry SDK environment
variables.
- #
https://opentelemetry.io/docs/specs/otel/protocol/exporter/#configuration-options
- endpoint = os.environ.get("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
f"{protocol}://{host}:{port}/v1/traces")
+ # According to the OpenTelemetry Spec, specific config options like
'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'
+ # take precedence over generic ones like 'OTEL_EXPORTER_OTLP_ENDPOINT'.
+ env_exporter_protocol = (
+ otel_env_config.type_specific_exporter_protocol or
otel_env_config.exporter_protocol
+ )
+ env_endpoint = otel_env_config.type_specific_endpoint or
otel_env_config.base_endpoint
+
+ # If the protocol env var isn't set, then it will be None,
+ # and it will default to an http/protobuf exporter.
+ if env_endpoint and env_exporter_protocol == "grpc":
+ from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import
OTLPSpanExporter
+ else:
+ from opentelemetry.exporter.otlp.proto.http.trace_exporter import
OTLPSpanExporter
+
+ if env_endpoint:
+ if host is not None and port is not None:
+ log.warning(
+ "Both the standard OpenTelemetry environment variables and "
+ "the Airflow OpenTelemetry configs have been provided. "
+ "Using the OpenTelemetry environment variables. "
+ "The Airflow configs have been deprecated and will be removed
in the future."
+ )
+
+ endpoint = env_endpoint
+ # The SDK will pick up all the values from the environment.
+ exporter = OTLPSpanExporter()
+ else:
+ if host is not None and port is not None:
+ # Since the configs have been deprecated, host and port could be
None.
+ # Log a warning to steer the user towards configuring the
environment variables
+ # and deliberately let it fail here without providing fallbacks.
+ log.warning(
+ "OpenTelemetry has unset config settings. "
+ "The Airflow configs have been deprecated and will be removed
in the future. "
+ "Configure the standard OpenTelemetry environment variables
instead. "
+ "For more info, check the docs."
+ )
+ else:
+ log.warning(
+ "The Airflow OpenTelemetry configs have been deprecated and
will be removed in the future. "
+ "OpenTelemetry is advised to be configured using the standard
environment variables. "
+ "For more info, check the docs."
+ )
+ # If the environment endpoint isn't set, then assume that the airflow
config is used
+ # where protocol isn't specified, and it's always http/protobuf.
+ # In that case it should default to the full 'url_path' and set it
directly.
+ endpoint = f"{protocol}://{host}:{port}/v1/traces"
Review Comment:
Thanks for the context on the module boundaries and the warning approach. I
see that #61289 also addresses the missing defaults upstream, so this should be
well covered now.
--
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]
