From 9952a17feddada00dcc7657a6c8bd7e12891721b Mon Sep 17 00:00:00 2001 From: GareArc Date: Sun, 1 Mar 2026 02:24:25 -0800 Subject: [PATCH] fix(telemetry): use URL scheme instead of API key for gRPC TLS detection - Change insecure parameter from API key-based to URL scheme-based detection - https:// endpoints now correctly use TLS (insecure=False) - All other endpoints (http://, no scheme) use insecure=True - Update tests to reflect URL scheme-based logic - Remove incorrect documentation claiming API key controls TLS --- api/configs/enterprise/__init__.py | 3 +- api/enterprise/telemetry/exporter.py | 7 ++- api/extensions/ext_otel.py | 16 +++-- .../enterprise/telemetry/test_exporter.py | 62 ++++++++++++++++--- 4 files changed, 69 insertions(+), 19 deletions(-) diff --git a/api/configs/enterprise/__init__.py b/api/configs/enterprise/__init__.py index 2db9de6195..11a71e1537 100644 --- a/api/configs/enterprise/__init__.py +++ b/api/configs/enterprise/__init__.py @@ -46,8 +46,7 @@ class EnterpriseTelemetryConfig(BaseSettings): ) ENTERPRISE_OTLP_API_KEY: str = Field( - description="Bearer token for enterprise OTLP export authentication. " - "When set, gRPC exporters automatically use TLS (insecure=False).", + description="Bearer token for enterprise OTLP export authentication.", default="", ) diff --git a/api/enterprise/telemetry/exporter.py b/api/enterprise/telemetry/exporter.py index 247e03691f..2dbf3f9f23 100644 --- a/api/enterprise/telemetry/exporter.py +++ b/api/enterprise/telemetry/exporter.py @@ -109,9 +109,10 @@ class EnterpriseExporter: sampling_rate: float = getattr(config, "ENTERPRISE_OTEL_SAMPLING_RATE", 1.0) self.include_content: bool = getattr(config, "ENTERPRISE_INCLUDE_CONTENT", True) api_key: str = getattr(config, "ENTERPRISE_OTLP_API_KEY", "") - # Auto-detect TLS: when bearer token is configured, use secure channel - insecure: bool = not bool(api_key) - + + # Auto-detect TLS: https:// uses secure, everything else is insecure + insecure = not endpoint.startswith("https://") + resource = Resource( attributes={ ResourceAttributes.SERVICE_NAME: service_name, diff --git a/api/extensions/ext_otel.py b/api/extensions/ext_otel.py index 40a915e68c..fce5736032 100644 --- a/api/extensions/ext_otel.py +++ b/api/extensions/ext_otel.py @@ -59,16 +59,20 @@ def init_app(app: DifyApp): protocol = (dify_config.OTEL_EXPORTER_OTLP_PROTOCOL or "").lower() if dify_config.OTEL_EXPORTER_TYPE == "otlp": if protocol == "grpc": + # Auto-detect TLS: https:// uses secure, everything else is insecure + endpoint = dify_config.OTLP_BASE_ENDPOINT + insecure = not endpoint.startswith("https://") + exporter = GRPCSpanExporter( - endpoint=dify_config.OTLP_BASE_ENDPOINT, + endpoint=endpoint, # Header field names must consist of lowercase letters, check RFC7540 - headers=(("authorization", f"Bearer {dify_config.OTLP_API_KEY}"),), - insecure=True, + headers=((("authorization", f"Bearer {dify_config.OTLP_API_KEY}"),) if dify_config.OTLP_API_KEY else None), + insecure=insecure, ) metric_exporter = GRPCMetricExporter( - endpoint=dify_config.OTLP_BASE_ENDPOINT, - headers=(("authorization", f"Bearer {dify_config.OTLP_API_KEY}"),), - insecure=True, + endpoint=endpoint, + headers=((("authorization", f"Bearer {dify_config.OTLP_API_KEY}"),) if dify_config.OTLP_API_KEY else None), + insecure=insecure, ) else: headers = {"Authorization": f"Bearer {dify_config.OTLP_API_KEY}"} if dify_config.OTLP_API_KEY else None diff --git a/api/tests/unit_tests/enterprise/telemetry/test_exporter.py b/api/tests/unit_tests/enterprise/telemetry/test_exporter.py index 48fdd308f8..8f052516e1 100644 --- a/api/tests/unit_tests/enterprise/telemetry/test_exporter.py +++ b/api/tests/unit_tests/enterprise/telemetry/test_exporter.py @@ -121,8 +121,8 @@ def test_api_key_overrides_conflicting_header( @patch("enterprise.telemetry.exporter.GRPCSpanExporter") @patch("enterprise.telemetry.exporter.GRPCMetricExporter") -def test_api_key_set_uses_secure_grpc(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: - """Test that API key presence enables TLS (insecure=False) for gRPC.""" +def test_https_endpoint_uses_secure_grpc(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: + """Test that https:// endpoint enables TLS (insecure=False) for gRPC.""" mock_config = SimpleNamespace( ENTERPRISE_OTLP_ENDPOINT="https://collector.example.com", ENTERPRISE_OTLP_HEADERS="", @@ -135,18 +135,17 @@ def test_api_key_set_uses_secure_grpc(mock_metric_exporter: MagicMock, mock_span EnterpriseExporter(mock_config) - # Verify insecure=False for both exporters + # Verify insecure=False for both exporters (https:// scheme) assert mock_span_exporter.call_args is not None assert mock_span_exporter.call_args.kwargs["insecure"] is False assert mock_metric_exporter.call_args is not None assert mock_metric_exporter.call_args.kwargs["insecure"] is False - @patch("enterprise.telemetry.exporter.GRPCSpanExporter") @patch("enterprise.telemetry.exporter.GRPCMetricExporter") -def test_no_api_key_uses_insecure_grpc(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: - """Test that empty API key uses insecure gRPC (backward compat).""" +def test_http_endpoint_uses_insecure_grpc(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: + """Test that http:// endpoint uses insecure gRPC (insecure=True).""" mock_config = SimpleNamespace( ENTERPRISE_OTLP_ENDPOINT="http://collector.example.com", ENTERPRISE_OTLP_HEADERS="", @@ -159,14 +158,13 @@ def test_no_api_key_uses_insecure_grpc(mock_metric_exporter: MagicMock, mock_spa EnterpriseExporter(mock_config) - # Verify insecure=True for both exporters + # Verify insecure=True for both exporters (http:// scheme) assert mock_span_exporter.call_args is not None assert mock_span_exporter.call_args.kwargs["insecure"] is True assert mock_metric_exporter.call_args is not None assert mock_metric_exporter.call_args.kwargs["insecure"] is True - @patch("enterprise.telemetry.exporter.HTTPSpanExporter") @patch("enterprise.telemetry.exporter.HTTPMetricExporter") def test_insecure_not_passed_to_http_exporters(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: @@ -213,3 +211,51 @@ def test_api_key_with_special_chars_preserved(mock_metric_exporter: MagicMock, m headers = mock_span_exporter.call_args.kwargs.get("headers") assert headers is not None assert ("authorization", f"Bearer {special_key}") in headers + + +@patch("enterprise.telemetry.exporter.GRPCSpanExporter") +@patch("enterprise.telemetry.exporter.GRPCMetricExporter") +def test_no_scheme_localhost_uses_insecure(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: + """Test that endpoint without scheme defaults to insecure for localhost.""" + mock_config = SimpleNamespace( + ENTERPRISE_OTLP_ENDPOINT="localhost:4317", + ENTERPRISE_OTLP_HEADERS="", + ENTERPRISE_OTLP_PROTOCOL="grpc", + ENTERPRISE_SERVICE_NAME="dify", + ENTERPRISE_OTEL_SAMPLING_RATE=1.0, + ENTERPRISE_INCLUDE_CONTENT=True, + ENTERPRISE_OTLP_API_KEY="", + ) + + EnterpriseExporter(mock_config) + + # Verify insecure=True for localhost without scheme + assert mock_span_exporter.call_args is not None + assert mock_span_exporter.call_args.kwargs["insecure"] is True + + assert mock_metric_exporter.call_args is not None + assert mock_metric_exporter.call_args.kwargs["insecure"] is True + + +@patch("enterprise.telemetry.exporter.GRPCSpanExporter") +@patch("enterprise.telemetry.exporter.GRPCMetricExporter") +def test_no_scheme_production_uses_insecure(mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock) -> None: + """Test that endpoint without scheme defaults to insecure (not https://).""" + mock_config = SimpleNamespace( + ENTERPRISE_OTLP_ENDPOINT="collector.example.com:4317", + ENTERPRISE_OTLP_HEADERS="", + ENTERPRISE_OTLP_PROTOCOL="grpc", + ENTERPRISE_SERVICE_NAME="dify", + ENTERPRISE_OTEL_SAMPLING_RATE=1.0, + ENTERPRISE_INCLUDE_CONTENT=True, + ENTERPRISE_OTLP_API_KEY="", + ) + + EnterpriseExporter(mock_config) + + # Verify insecure=True for any endpoint without https:// scheme + assert mock_span_exporter.call_args is not None + assert mock_span_exporter.call_args.kwargs["insecure"] is True + + assert mock_metric_exporter.call_args is not None + assert mock_metric_exporter.call_args.kwargs["insecure"] is True \ No newline at end of file