From 57cff071cb69a9cddfb1b9b4f42868d40f3f132d Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 3 Mar 2021 14:39:53 +0200 Subject: [PATCH 1/3] Allow selecting gRPC protocol via env vars for Jaeger exporter --- .../opentelemetry/exporter/jaeger/__init__.py | 55 +++++++++++++------ .../exporter/jaeger/protocol/__init__.py | 7 +++ .../src/opentelemetry/exporter/jaeger/util.py | 21 +++++++ .../tests/test_jaeger_exporter_protobuf.py | 23 +++----- .../tests/test_jaeger_exporter_thrift.py | 48 ++++++++-------- .../sdk/environment_variables/__init__.py | 1 + 6 files changed, 100 insertions(+), 55 deletions(-) create mode 100644 exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/protocol/__init__.py diff --git a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py index be81bfaf0bd..b6fd9a67dd3 100644 --- a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py +++ b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py @@ -18,8 +18,8 @@ This exporter always sends traces to the configured agent using the Thrift compact protocol over UDP. When it is not feasible to deploy Jaeger Agent next to the application, for example, when the application code is running as Lambda function, a collector can be configured to send spans -using either Thrift over HTTP or Protobuf via gRPC. If both agent and collector are configured, -the exporter sends traces only to the collector to eliminate the duplicate entries. +using either Thrift over HTTP or Protobuf via gRPC. + Usage ----- @@ -28,6 +28,7 @@ from opentelemetry import trace from opentelemetry.exporter import jaeger + from opentelemetry.exporter.jaeger.protocol import Protocol from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor @@ -40,13 +41,13 @@ # configure agent agent_host_name='localhost', agent_port=6831, - # optional: configure also collector + # optional: configure collector # collector_endpoint='http://localhost:14268/api/traces?format=jaeger.thrift', # username=xxxx, # optional # password=xxxx, # optional # insecure=True, # optional - # credentials=xxx # optional channel creds - # transport_format='protobuf' # optional + # credentials=xxx, # optional channel creds + # protocol=Protocol.THRIFT, # optional # max_tag_value_length=None # optional ) @@ -59,6 +60,32 @@ with tracer.start_as_current_span('foo'): print('Hello world!') +The exporter supports the following environment variable for configuration: + +.. envvar:: OTEL_EXPORTER_JAEGER_AGENT_HOST + +Hostname of the Jaeger UDP agent. + +.. envvar:: OTEL_EXPORTER_JAEGER_AGENT_PORT + +Port of the Jaeger UDP agent. + +.. envvar:: OTEL_EXPORTER_JAEGER_ENDPOINT + +When using Thrift over HTTP or gRPC, the endpoint for Jaeger traces. + +.. envvar:: OTEL_EXPORTER_JAEGER_USER + +Username to be used for HTTP basic authentication. + +.. envvar:: OTEL_EXPORTER_JAEGER_PASSWORD + +Password to be used for HTTP basic authentication + +.. envvar:: OTEL_EXPORTER_JAEGER_PROTOCOL + +Protocol to use when exporting traces. Possible values: ``thrift``, ``grpc`` or ``thrift_http``. + API --- .. _Jaeger: https://www.jaegertracing.io/ @@ -79,6 +106,7 @@ CollectorServiceStub, ) from opentelemetry.exporter.jaeger.gen.jaeger import Collector as jaeger_thrift +from opentelemetry.exporter.jaeger.protocol import Protocol from opentelemetry.exporter.jaeger.send.thrift import AgentClientUDP, Collector from opentelemetry.exporter.jaeger.translate import Translate from opentelemetry.exporter.jaeger.translate.protobuf import ProtobufTranslator @@ -98,9 +126,6 @@ UDP_PACKET_MAX_LENGTH = 65000 -TRANSPORT_FORMAT_THRIFT = "thrift" -TRANSPORT_FORMAT_PROTOBUF = "protobuf" - logger = logging.getLogger(__name__) @@ -134,8 +159,8 @@ def __init__( password: Optional[str] = None, insecure: Optional[bool] = None, credentials: Optional[ChannelCredentials] = None, - transport_format: Optional[str] = None, max_tag_value_length: Optional[int] = None, + protocol: Optional[Protocol] = None, ): self.service_name = service_name self._max_tag_value_length = max_tag_value_length @@ -177,15 +202,11 @@ def __init__( self._grpc_client = None self.insecure = util._get_insecure(insecure) self.credentials = util._get_credentials(credentials) - self.transport_format = ( - transport_format.lower() - if transport_format - else TRANSPORT_FORMAT_THRIFT - ) + self.protocol = util._get_protocol(protocol) @property def _collector_grpc_client(self) -> Optional[CollectorServiceStub]: - if self.transport_format != TRANSPORT_FORMAT_PROTOBUF: + if self.protocol != Protocol.GRPC: return None endpoint = self.collector_endpoint or DEFAULT_GRPC_COLLECTOR_ENDPOINT @@ -208,7 +229,7 @@ def _collector_http_client(self) -> Optional[Collector]: if ( self.collector_endpoint is None - or self.transport_format != TRANSPORT_FORMAT_THRIFT + or self.protocol != Protocol.THRIFT_HTTP ): return None @@ -223,7 +244,7 @@ def _collector_http_client(self) -> Optional[Collector]: def export(self, spans) -> SpanExportResult: translator = Translate(spans) - if self.transport_format == TRANSPORT_FORMAT_PROTOBUF: + if self.protocol == Protocol.GRPC: pb_translator = ProtobufTranslator( self.service_name, self._max_tag_value_length ) diff --git a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/protocol/__init__.py b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/protocol/__init__.py new file mode 100644 index 00000000000..c5ceab152c1 --- /dev/null +++ b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/protocol/__init__.py @@ -0,0 +1,7 @@ +from enum import Enum + + +class Protocol(Enum): + GRPC = "grpc" + THRIFT = "thrift" + THRIFT_HTTP = "thrift_http" diff --git a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/util.py b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/util.py index 4a72c1817c2..b3f1cba2436 100644 --- a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/util.py +++ b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/util.py @@ -14,12 +14,15 @@ import logging from os import environ +from typing import Optional from grpc import ChannelCredentials, ssl_channel_credentials +from opentelemetry.exporter.jaeger.protocol import Protocol from opentelemetry.sdk.environment_variables import ( OTEL_EXPORTER_JAEGER_CERTIFICATE, OTEL_EXPORTER_JAEGER_INSECURE, + OTEL_EXPORTER_JAEGER_PROTOCOL, ) logger = logging.getLogger(__name__) @@ -53,3 +56,21 @@ def _get_credentials(param): if creds_env: return _load_credential_from_file(creds_env) return ssl_channel_credentials() + + +def _get_protocol(protocol: Optional[Protocol]) -> Protocol: + if protocol is not None: + return protocol + + env_protocol = environ.get(OTEL_EXPORTER_JAEGER_PROTOCOL) + + if env_protocol is None: + return Protocol.THRIFT + + protocols = { + "grpc": Protocol.GRPC, + "thrift": Protocol.THRIFT, + "thrift_http": Protocol.THRIFT_HTTP, + } + + return protocols.get(env_protocol.strip().lower(), Protocol.THRIFT) diff --git a/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_protobuf.py b/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_protobuf.py index eea3e54700c..ac6b2bc91b2 100644 --- a/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_protobuf.py +++ b/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_protobuf.py @@ -32,6 +32,7 @@ from opentelemetry.sdk.environment_variables import ( OTEL_EXPORTER_JAEGER_CERTIFICATE, OTEL_EXPORTER_JAEGER_ENDPOINT, + OTEL_EXPORTER_JAEGER_PROTOCOL, ) from opentelemetry.sdk.trace import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationInfo @@ -60,27 +61,21 @@ def test_constructor_by_environment_variables(self): collector_endpoint = "localhost:14250" - env_patch = patch.dict( + with patch.dict( "os.environ", { + OTEL_EXPORTER_JAEGER_PROTOCOL: "grpc", OTEL_EXPORTER_JAEGER_ENDPOINT: collector_endpoint, OTEL_EXPORTER_JAEGER_CERTIFICATE: os.path.dirname(__file__) + "/certs/cred.cert", }, - ) - - env_patch.start() - - exporter = JaegerSpanExporter( - service_name=service, transport_format="protobuf" - ) - - self.assertEqual(exporter.service_name, service) - self.assertIsNotNone(exporter._collector_grpc_client) - self.assertEqual(exporter.collector_endpoint, collector_endpoint) - self.assertIsNotNone(exporter.credentials) + ): + exporter = JaegerSpanExporter(service_name=service) - env_patch.stop() + self.assertEqual(exporter.service_name, service) + self.assertIsNotNone(exporter._collector_grpc_client) + self.assertEqual(exporter.collector_endpoint, collector_endpoint) + self.assertIsNotNone(exporter.credentials) # pylint: disable=too-many-locals,too-many-statements def test_translate_to_jaeger(self): diff --git a/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_thrift.py b/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_thrift.py index 0bee785760d..9f15699b97f 100644 --- a/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_thrift.py +++ b/exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_thrift.py @@ -21,6 +21,7 @@ import opentelemetry.exporter.jaeger as jaeger_exporter from opentelemetry import trace as trace_api from opentelemetry.exporter.jaeger.gen.jaeger import ttypes as jaeger +from opentelemetry.exporter.jaeger.protocol import Protocol from opentelemetry.exporter.jaeger.translate import Translate from opentelemetry.exporter.jaeger.translate.thrift import ThriftTranslator from opentelemetry.sdk import trace @@ -29,6 +30,7 @@ OTEL_EXPORTER_JAEGER_AGENT_PORT, OTEL_EXPORTER_JAEGER_ENDPOINT, OTEL_EXPORTER_JAEGER_PASSWORD, + OTEL_EXPORTER_JAEGER_PROTOCOL, OTEL_EXPORTER_JAEGER_USER, ) from opentelemetry.sdk.trace import Resource @@ -68,6 +70,7 @@ def test_constructor_default(self): self.assertTrue(exporter._collector_http_client is None) self.assertTrue(exporter._agent_client is not None) self.assertIsNone(exporter._max_tag_value_length) + self.assertEqual(exporter.protocol, Protocol.THRIFT) def test_constructor_explicit(self): # pylint: disable=protected-access @@ -90,6 +93,7 @@ def test_constructor_explicit(self): username=username, password=password, max_tag_value_length=42, + protocol=Protocol.THRIFT_HTTP, ) self.assertEqual(exporter.service_name, service) @@ -122,38 +126,34 @@ def test_constructor_by_environment_variables(self): password = "password" auth = (username, password) - environ_patcher = mock.patch.dict( + with mock.patch.dict( "os.environ", { OTEL_EXPORTER_JAEGER_AGENT_HOST: agent_host_name, OTEL_EXPORTER_JAEGER_AGENT_PORT: agent_port, OTEL_EXPORTER_JAEGER_ENDPOINT: collector_endpoint, + OTEL_EXPORTER_JAEGER_PROTOCOL: "thrift_http", OTEL_EXPORTER_JAEGER_USER: username, OTEL_EXPORTER_JAEGER_PASSWORD: password, }, - ) - - environ_patcher.start() - - exporter = jaeger_exporter.JaegerSpanExporter(service_name=service) - - self.assertEqual(exporter.service_name, service) - self.assertEqual(exporter.agent_host_name, agent_host_name) - self.assertEqual(exporter.agent_port, int(agent_port)) - self.assertTrue(exporter._collector_http_client is not None) - self.assertEqual(exporter.collector_endpoint, collector_endpoint) - self.assertEqual(exporter._collector_http_client.auth, auth) - # property should not construct new object - collector = exporter._collector_http_client - self.assertEqual(exporter._collector_http_client, collector) - # property should construct new object - exporter._collector = None - exporter.username = None - exporter.password = None - self.assertNotEqual(exporter._collector_http_client, collector) - self.assertTrue(exporter._collector_http_client.auth is None) - - environ_patcher.stop() + ): + exporter = jaeger_exporter.JaegerSpanExporter(service_name=service) + + self.assertEqual(exporter.service_name, service) + self.assertEqual(exporter.agent_host_name, agent_host_name) + self.assertEqual(exporter.agent_port, int(agent_port)) + self.assertTrue(exporter._collector_http_client is not None) + self.assertEqual(exporter.collector_endpoint, collector_endpoint) + self.assertEqual(exporter._collector_http_client.auth, auth) + # property should not construct new object + collector = exporter._collector_http_client + self.assertEqual(exporter._collector_http_client, collector) + # property should construct new object + exporter._collector = None + exporter.username = None + exporter.password = None + self.assertNotEqual(exporter._collector_http_client, collector) + self.assertTrue(exporter._collector_http_client.auth is None) def test_nsec_to_usec_round(self): # pylint: disable=protected-access diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py index 79d9c29c20f..82f5b276069 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py @@ -26,6 +26,7 @@ OTEL_EXPORTER_JAEGER_AGENT_HOST = "OTEL_EXPORTER_JAEGER_AGENT_HOST" OTEL_EXPORTER_JAEGER_AGENT_PORT = "OTEL_EXPORTER_JAEGER_AGENT_PORT" OTEL_EXPORTER_JAEGER_ENDPOINT = "OTEL_EXPORTER_JAEGER_ENDPOINT" +OTEL_EXPORTER_JAEGER_PROTOCOL = "OTEL_EXPORTER_JAEGER_PROTOCOL" OTEL_EXPORTER_JAEGER_USER = "OTEL_EXPORTER_JAEGER_USER" OTEL_EXPORTER_JAEGER_PASSWORD = "OTEL_EXPORTER_JAEGER_PASSWORD" OTEL_EXPORTER_ZIPKIN_ENDPOINT = "OTEL_EXPORTER_ZIPKIN_ENDPOINT" From 967aaf7c1dc89a9320c079e50800f3e83789f5cf Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 3 Mar 2021 15:36:30 +0200 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f257ced33a8..c273aa33978 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 ([#1662])(https://github.com/open-telemetry/opentelemetry-python/pull/1662) - Rename `BaggagePropagator` to `W3CBaggagePropagator` ([#1663])(https://github.com/open-telemetry/opentelemetry-python/pull/1663) +- Jaeger exporter: allow selecting gRPC and Thrift HTTP via env vars + ([#1666])(https://github.com/open-telemetry/opentelemetry-python/pull/1666) ## [0.18b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v0.18b0) - 2021-02-16 From f75acbe8392d429436e424f7739415cdd44136fc Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Wed, 3 Mar 2021 15:39:05 +0200 Subject: [PATCH 3/3] Use is to compare enums --- .../src/opentelemetry/exporter/jaeger/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py index b6fd9a67dd3..9bd2a5097bf 100644 --- a/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py +++ b/exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py @@ -206,7 +206,7 @@ def __init__( @property def _collector_grpc_client(self) -> Optional[CollectorServiceStub]: - if self.protocol != Protocol.GRPC: + if self.protocol is not Protocol.GRPC: return None endpoint = self.collector_endpoint or DEFAULT_GRPC_COLLECTOR_ENDPOINT @@ -229,7 +229,7 @@ def _collector_http_client(self) -> Optional[Collector]: if ( self.collector_endpoint is None - or self.protocol != Protocol.THRIFT_HTTP + or self.protocol is not Protocol.THRIFT_HTTP ): return None @@ -244,7 +244,7 @@ def _collector_http_client(self) -> Optional[Collector]: def export(self, spans) -> SpanExportResult: translator = Translate(spans) - if self.protocol == Protocol.GRPC: + if self.protocol is Protocol.GRPC: pb_translator = ProtobufTranslator( self.service_name, self._max_tag_value_length )