From b6f07afddb08913732d5f1d0d707cf46840b4362 Mon Sep 17 00:00:00 2001 From: areveny Date: Sun, 2 Jan 2022 12:26:18 -0600 Subject: [PATCH] Default otlp-proto-grpc exporter to secure, let https endpoint supercede insecure --- .../exporter/otlp/proto/grpc/exporter.py | 9 ++++--- .../proto/grpc/trace_exporter/__init__.py | 5 +--- .../tests/test_otlp_trace_exporter.py | 24 +++++++++++++++++-- .../sdk/environment_variables.py | 10 ++++---- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py index 691c7c2dc1b..236472bdd41 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/exporter.py @@ -216,11 +216,10 @@ def __init__( parsed_url = urlparse(endpoint) - if insecure is None: - if parsed_url.scheme == "https": - insecure = False - else: - insecure = True + if insecure and parsed_url.scheme != "https": + insecure = True + else: + insecure = False if parsed_url.netloc: endpoint = parsed_url.netloc diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py index 5f18de1ac01..893c62fd941 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py @@ -84,10 +84,7 @@ def __init__( timeout: Optional[int] = None, compression: Optional[Compression] = None, ): - if ( - not insecure - and environ.get(OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE) is not None - ): + if environ.get(OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE) is not None: credentials = _get_credentials( credentials, OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py index 198ff442718..cb1fb4271c5 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py @@ -292,15 +292,35 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "http://localhost:4317", None, - mock_insecure, + mock_secure, # Default secure ), ( "localhost:4317", None, + mock_secure, # Default secure + ), + ( + "http://localhost:4317", + True, mock_insecure, ), ( "localhost:4317", + True, + mock_insecure, + ), + ( + "http://localhost:4317", + False, + mock_secure, + ), + ( + "localhost:4317", + False, + mock_secure, + ), + ( + "https://localhost:4317", False, mock_secure, ), @@ -312,7 +332,7 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "https://localhost:4317", True, - mock_insecure, + mock_secure, # https scheme overrides explicit insecure ), ] for endpoint, insecure, mock_method in endpoints: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py index e9d35092a61..ca22e5435c7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py @@ -281,9 +281,9 @@ .. envvar:: OTEL_EXPORTER_OTLP_ENDPOINT The :envvar:`OTEL_EXPORTER_OTLP_ENDPOINT` target to which the exporter is going to send spans or metrics. -The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. -A scheme of https indicates a secure connection. -Default: "https://localhost:4317" +The endpoint MUST be a valid URL host, and MAY contain a scheme (http or https), port and path. +A scheme of https indicates a secure connection and takes precedence over the `insecure` configuration setting. +Default: "http://localhost:4317" """ OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT" @@ -291,8 +291,8 @@ .. envvar:: OTEL_EXPORTER_OTLP_TRACES_ENDPOINT The :envvar:`OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` target to which the span exporter is going to send spans. -The endpoint MUST be a valid URL with scheme (http or https) and host, and MAY contain a port and path. -A scheme of https indicates a secure connection. +The endpoint MUST be a valid URL host, and MAY contain a scheme (http or https), port and path. +A scheme of https indicates a secure connection and takes precedence over the `insecure` configuration setting. """ OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"