Allow selecting Jaeger protocol via env vars

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
 

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 is not 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 is not 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 is Protocol.GRPC:
             pb_translator = ProtobufTranslator(
                 self.service_name, self._max_tag_value_length
             )

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"

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)

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):

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

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"