From 2a7e3fc8efe718c9dde4c30db80ca06f42a03bee Mon Sep 17 00:00:00 2001 From: Arianna Y <92831762+areveny@users.noreply.github.com> Date: Tue, 11 Jan 2022 15:29:46 -0600 Subject: [PATCH] Support insecure configuration (#2350) --- .github/workflows/test.yml | 2 +- CHANGELOG.md | 2 + .../proto/grpc/_metric_exporter/__init__.py | 10 +++++ .../exporter/otlp/proto/grpc/exporter.py | 13 ++++-- .../proto/grpc/trace_exporter/__init__.py | 7 ++++ .../tests/logs/test_otlp_logs_exporter.py | 23 ++++++++++- .../metrics/test_otlp_metrics_exporter.py | 41 ++++++++++++++++++- .../tests/test_otlp_trace_exporter.py | 41 ++++++++++++++++++- .../sdk/environment_variables.py | 38 ++++++++++++++--- 9 files changed, 165 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7c8b1b1d0a3..bf077b3d85d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ env: # Otherwise, set variable to the commit of your branch on # opentelemetry-python-contrib which is compatible with these Core repo # changes. - CONTRIB_REPO_SHA: 23394ccd80878a91534f8421b82a7410eb775e65 + CONTRIB_REPO_SHA: 741321434f0803dd3b304f3a93022d8d451a4c90 # This is needed because we do not clone the core repo in contrib builds anymore. # When running contrib builds as part of core builds, we use actions/checkout@v2 which # does not set an environment variable (simply just runs tox), which is different when diff --git a/CHANGELOG.md b/CHANGELOG.md index 91b5be8309d..c91835f1d2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2334](https://github.com/open-telemetry/opentelemetry-python/pull/2334)) - Add otlp entrypoint for log exporter ([#2322](https://github.com/open-telemetry/opentelemetry-python/pull/2322)) +- Support insecure configuration for OTLP gRPC exporter + ([#2350](https://github.com/open-telemetry/opentelemetry-python/pull/2350)) ## [1.7.1-0.26b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.7.0-0.26b0) - 2021-11-11 diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_metric_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_metric_exporter/__init__.py index 5e76e5c035d..9db2699da76 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_metric_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/_metric_exporter/__init__.py @@ -11,6 +11,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from os import environ from typing import Optional, Sequence from grpc import ChannelCredentials, Compression from opentelemetry.exporter.otlp.proto.grpc.exporter import ( @@ -29,6 +30,9 @@ ResourceMetrics, ) from opentelemetry.proto.metrics.v1.metrics_pb2 import Metric as PB2Metric +from opentelemetry.sdk.environment_variables import ( + OTEL_EXPORTER_OTLP_METRICS_INSECURE, +) from opentelemetry.sdk._metrics.data import ( MetricData, ) @@ -57,6 +61,12 @@ def __init__( timeout: Optional[int] = None, compression: Optional[Compression] = None, ): + + if insecure is None: + insecure = environ.get(OTEL_EXPORTER_OTLP_METRICS_INSECURE) + if insecure is not None: + insecure = insecure.lower() == "true" + super().__init__( **{ "endpoint": endpoint, 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 ec30cdac1c3..cbde41bb6c4 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 @@ -47,6 +47,7 @@ OTEL_EXPORTER_OTLP_COMPRESSION, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, + OTEL_EXPORTER_OTLP_INSECURE, OTEL_EXPORTER_OTLP_TIMEOUT, ) from opentelemetry.sdk.resources import Resource as SDKResource @@ -218,11 +219,17 @@ def __init__( parsed_url = urlparse(endpoint) + if parsed_url.scheme == "https": + insecure = False if insecure is None: - if parsed_url.scheme == "https": - insecure = False + insecure = environ.get(OTEL_EXPORTER_OTLP_INSECURE) + if insecure is not None: + insecure = insecure.lower() == "true" else: - insecure = True + if parsed_url.scheme == "http": + 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 075b961250b..18f9ad997be 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 @@ -44,6 +44,7 @@ OTEL_EXPORTER_OTLP_TRACES_COMPRESSION, OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, OTEL_EXPORTER_OTLP_TRACES_HEADERS, + OTEL_EXPORTER_OTLP_TRACES_INSECURE, OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, ) from opentelemetry.sdk.trace import ReadableSpan @@ -84,6 +85,12 @@ def __init__( timeout: Optional[int] = None, compression: Optional[Compression] = None, ): + + if insecure is None: + insecure = environ.get(OTEL_EXPORTER_OTLP_TRACES_INSECURE) + if insecure is not None: + insecure = insecure.lower() == "true" + if ( not insecure and environ.get(OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE) is not None diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py index 01fe4248358..556700ab850 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/logs/test_otlp_logs_exporter.py @@ -193,13 +193,33 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "localhost:4317", None, + mock_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, + ), ( "https://localhost:4317", None, @@ -208,9 +228,10 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "https://localhost:4317", True, - mock_insecure, + mock_secure, ), ] + # pylint: disable=C0209 for endpoint, insecure, mock_method in endpoints: OTLPLogExporter(endpoint=endpoint, insecure=insecure) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py index f7cf8ec94d8..54253c91415 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py @@ -32,6 +32,9 @@ ) from opentelemetry.sdk._metrics.data import Metric, MetricData from opentelemetry.sdk._metrics.export import MetricExportResult +from opentelemetry.sdk.environment_variables import ( + OTEL_EXPORTER_OTLP_METRICS_INSECURE, +) from opentelemetry.sdk.resources import Resource as SDKResource from opentelemetry.sdk.util.instrumentation import InstrumentationInfo @@ -119,6 +122,22 @@ def test_no_credentials_error( OTLPMetricExporter(insecure=False) self.assertTrue(mock_ssl_channel.called) + @patch.dict( + "os.environ", + {OTEL_EXPORTER_OTLP_METRICS_INSECURE: "True"}, + ) + @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.insecure_channel") + # pylint: disable=unused-argument + def test_otlp_insecure_from_env(self, mock_insecure): + OTLPMetricExporter() + # pylint: disable=protected-access + self.assertTrue(mock_insecure.called) + self.assertEqual( + 1, + mock_insecure.call_count, + f"expected {mock_insecure} to be called", + ) + # pylint: disable=no-self-use @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.insecure_channel") @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.secure_channel") @@ -133,13 +152,33 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "localhost:4317", None, + mock_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, + ), ( "https://localhost:4317", None, @@ -148,7 +187,7 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "https://localhost:4317", True, - mock_insecure, + mock_secure, ), ] # pylint: disable=C0209 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 252d2f7b93d..d09c21f412d 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 @@ -58,6 +58,7 @@ OTEL_EXPORTER_OTLP_TRACES_COMPRESSION, OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, OTEL_EXPORTER_OTLP_TRACES_HEADERS, + OTEL_EXPORTER_OTLP_TRACES_INSECURE, OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, ) from opentelemetry.sdk.resources import Resource as SDKResource @@ -123,6 +124,8 @@ def Export(self, request, context): class TestOTLPSpanExporter(TestCase): + # pylint: disable=too-many-public-methods + def setUp(self): tracer_provider = TracerProvider() self.exporter = OTLPSpanExporter(insecure=True) @@ -289,6 +292,22 @@ def test_otlp_headers_from_env(self, mock_ssl_channel, mock_secure): exporter._headers, (("key5", "value5"), ("key6", "value6")) ) + @patch.dict( + "os.environ", + {OTEL_EXPORTER_OTLP_TRACES_INSECURE: "True"}, + ) + @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.insecure_channel") + # pylint: disable=unused-argument + def test_otlp_insecure_from_env(self, mock_insecure): + OTLPSpanExporter() + # pylint: disable=protected-access + self.assertTrue(mock_insecure.called) + self.assertEqual( + 1, + mock_insecure.call_count, + f"expected {mock_insecure} to be called", + ) + # pylint: disable=no-self-use @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.insecure_channel") @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.secure_channel") @@ -304,10 +323,30 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "localhost:4317", None, + mock_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, ), @@ -319,7 +358,7 @@ def test_otlp_exporter_endpoint(self, mock_secure, mock_insecure): ( "https://localhost:4317", True, - mock_insecure, + mock_secure, ), ] 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..d9bab1bb169 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py @@ -281,18 +281,37 @@ .. 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_INSECURE = "OTEL_EXPORTER_OTLP_INSECURE" +""" +.. envvar:: OTEL_EXPORTER_OTLP_INSECURE + +The :envvar:`OTEL_EXPORTER_OTLP_INSECURE` represents whether to enable client transport security for gRPC requests. +A scheme of https takes precedence over this configuration setting. +Default: False +""" + +OTEL_EXPORTER_OTLP_TRACES_INSECURE = "OTEL_EXPORTER_OTLP_TRACES_INSECURE" +""" +.. envvar:: OTEL_EXPORTER_OTLP_TRACES_INSECURE + +The :envvar:`OTEL_EXPORTER_OTLP_TRACES_INSECURE` represents whether to enable client transport security +for gRPC requests for spans. A scheme of https takes precedence over the this configuration setting. +Default: False +""" + + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT" """ .. 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 this configuration setting. """ OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL" @@ -335,6 +354,15 @@ wait for each batch export for spans. """ +OTEL_EXPORTER_OTLP_METRICS_INSECURE = "OTEL_EXPORTER_OTLP_METRICS_INSECURE" +""" +.. envvar:: OTEL_EXPORTER_OTLP_METRICS_INSECURE + +The :envvar:`OTEL_EXPORTER_OTLP_METRICS_INSECURE` represents whether to enable client transport security +for gRPC requests for metrics. A scheme of https takes precedence over the this configuration setting. +Default: False +""" + OTEL_EXPORTER_JAEGER_CERTIFICATE = "OTEL_EXPORTER_JAEGER_CERTIFICATE" """ .. envvar:: OTEL_EXPORTER_JAEGER_CERTIFICATE