Support Environment Variables for JaegerSpanExporter configuration (#…

…1114)

exporter/opentelemetry-exporter-jaeger/CHANGELOG.md

@@ -1,6 +1,9 @@
 # Changelog
 
 ## Unreleased
+- Add support for Jaeger Span Exporter configuration by environment variables and<br/>
+  change JaegerSpanExporter constructor parameters
+  ([#1114](https://github.com/open-telemetry/opentelemetry-python/pull/1114)) 
 
 ## Version 0.13b0
 

exporter/opentelemetry-exporter-jaeger/README.rst

@@ -19,6 +19,14 @@ Installation
 .. _Jaeger: https://www.jaegertracing.io/
 .. _OpenTelemetry: https://github.com/open-telemetry/opentelemetry-python/
 
+Configuration
+-------------
+
+OpenTelemetry Jaeger Exporter can be configured by setting `JaegerSpanExporter parameters
+<https://github.com/open-telemetry/opentelemetry-python/blob/master/exporter/opentelemetry-exporter-jaeger
+/src/opentelemetry/exporter/jaeger/__init__.py#L88>`_ or by setting
+`environment variables <https://github.com/open-telemetry/opentelemetry-specification/blob/master/
+specification/sdk-environment-variables.md#jaeger-exporter>`_
 
 References
 ----------

exporter/opentelemetry-exporter-jaeger/src/opentelemetry/exporter/jaeger/__init__.py

@@ -39,10 +39,7 @@
         agent_host_name='localhost',
         agent_port=6831,
         # optional: configure also collector
-        # collector_host_name='localhost',
-        # collector_port=14268,
-        # collector_endpoint='/api/traces?format=jaeger.thrift',
-        # collector_protocol='http',
+        # collector_endpoint='http://localhost:14268/api/traces?format=jaeger.thrift',
         # username=xxxx, # optional
         # password=xxxx, # optional
     )
@@ -69,16 +66,14 @@
 from thrift.protocol import TBinaryProtocol, TCompactProtocol
 from thrift.transport import THttpClient, TTransport
 
-import opentelemetry.trace as trace_api
+from opentelemetry.configuration import Configuration
 from opentelemetry.exporter.jaeger.gen.agent import Agent as agent
 from opentelemetry.exporter.jaeger.gen.jaeger import Collector as jaeger
 from opentelemetry.sdk.trace.export import Span, SpanExporter, SpanExportResult
 from opentelemetry.trace.status import StatusCanonicalCode
 
 DEFAULT_AGENT_HOST_NAME = "localhost"
 DEFAULT_AGENT_PORT = 6831
-DEFAULT_COLLECTOR_ENDPOINT = "/api/traces?format=jaeger.thrift"
-DEFAULT_COLLECTOR_PROTOCOL = "http"
 
 UDP_PACKET_MAX_LENGTH = 65000
 
@@ -93,11 +88,7 @@ class JaegerSpanExporter(SpanExporter):
             when query for spans.
         agent_host_name: The host name of the Jaeger-Agent.
         agent_port: The port of the Jaeger-Agent.
-        collector_host_name: The host name of the Jaeger-Collector HTTP/HTTPS
-            Thrift.
-        collector_port: The port of the Jaeger-Collector HTTP/HTTPS Thrift.
         collector_endpoint: The endpoint of the Jaeger-Collector HTTP/HTTPS Thrift.
-        collector_protocol: The transfer protocol for the Jaeger-Collector(HTTP or HTTPS).
         username: The user name of the Basic Auth if authentication is
             required.
         password: The password of the Basic Auth if authentication is
@@ -107,25 +98,39 @@ class JaegerSpanExporter(SpanExporter):
     def __init__(
         self,
         service_name,
-        agent_host_name=DEFAULT_AGENT_HOST_NAME,
-        agent_port=DEFAULT_AGENT_PORT,
-        collector_host_name=None,
-        collector_port=None,
-        collector_endpoint=DEFAULT_COLLECTOR_ENDPOINT,
-        collector_protocol=DEFAULT_COLLECTOR_PROTOCOL,
+        agent_host_name=None,
+        agent_port=None,
+        collector_endpoint=None,
         username=None,
         password=None,
     ):
         self.service_name = service_name
-        self.agent_host_name = agent_host_name
-        self.agent_port = agent_port
+        self.agent_host_name = _parameter_setter(
+            param=agent_host_name,
+            env_variable=Configuration().EXPORTER_JAEGER_AGENT_HOST,
+            default=DEFAULT_AGENT_HOST_NAME,
+        )
+        self.agent_port = _parameter_setter(
+            param=agent_port,
+            env_variable=Configuration().EXPORTER_JAEGER_AGENT_PORT,
+            default=DEFAULT_AGENT_PORT,
+        )
         self._agent_client = None
-        self.collector_host_name = collector_host_name
-        self.collector_port = collector_port
-        self.collector_endpoint = collector_endpoint
-        self.collector_protocol = collector_protocol
-        self.username = username
-        self.password = password
+        self.collector_endpoint = _parameter_setter(
+            param=collector_endpoint,
+            env_variable=Configuration().EXPORTER_JAEGER_ENDPOINT,
+            default=None,
+        )
+        self.username = _parameter_setter(
+            param=username,
+            env_variable=Configuration().EXPORTER_JAEGER_USER,
+            default=None,
+        )
+        self.password = _parameter_setter(
+            param=password,
+            env_variable=Configuration().EXPORTER_JAEGER_PASSWORD,
+            default=None,
+        )
         self._collector = None
 
     @property
@@ -141,21 +146,16 @@ def collector(self):
         if self._collector is not None:
             return self._collector
 
-        if self.collector_host_name is None or self.collector_port is None:
+        if self.collector_endpoint is None:
             return None
 
-        thrift_url = "{}://{}:{}{}".format(
-            self.collector_protocol,
-            self.collector_host_name,
-            self.collector_port,
-            self.collector_endpoint,
-        )
-
         auth = None
         if self.username is not None and self.password is not None:
             auth = (self.username, self.password)
 
-        self._collector = Collector(thrift_url=thrift_url, auth=auth)
+        self._collector = Collector(
+            thrift_url=self.collector_endpoint, auth=auth
+        )
         return self._collector
 
     def export(self, spans):
@@ -177,6 +177,22 @@ def shutdown(self):
         pass
 
 
+def _parameter_setter(param, env_variable, default):
+    """Returns value according to the provided data.
+
+    Args:
+        param: Constructor parameter value
+        env_variable: Environment variable related to the parameter
+        default: Constructor parameter default value
+    """
+    if param is None:
+        res = env_variable or default
+    else:
+        res = param
+
+    return res
+
+
 def _nsec_to_usec_round(nsec):
     """Round nanoseconds to microseconds"""
     return (nsec + 500) // 10 ** 3

exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter.py

@@ -20,6 +20,7 @@
 # pylint:disable=import-error
 import opentelemetry.exporter.jaeger as jaeger_exporter
 from opentelemetry import trace as trace_api
+from opentelemetry.configuration import Configuration
 from opentelemetry.exporter.jaeger.gen.jaeger import ttypes as jaeger
 from opentelemetry.sdk import trace
 from opentelemetry.sdk.trace import Resource
@@ -43,20 +44,14 @@ def setUp(self):
     def test_constructor_default(self):
         """Test the default values assigned by constructor."""
         service_name = "my-service-name"
-        host_name = "localhost"
-        thrift_port = None
+        agent_host_name = "localhost"
         agent_port = 6831
-        collector_endpoint = "/api/traces?format=jaeger.thrift"
-        collector_protocol = "http"
         exporter = jaeger_exporter.JaegerSpanExporter(service_name)
 
         self.assertEqual(exporter.service_name, service_name)
-        self.assertEqual(exporter.collector_host_name, None)
-        self.assertEqual(exporter.agent_host_name, host_name)
+        self.assertEqual(exporter.agent_host_name, agent_host_name)
         self.assertEqual(exporter.agent_port, agent_port)
-        self.assertEqual(exporter.collector_port, thrift_port)
-        self.assertEqual(exporter.collector_protocol, collector_protocol)
-        self.assertEqual(exporter.collector_endpoint, collector_endpoint)
+        self.assertEqual(exporter.collector_endpoint, None)
         self.assertEqual(exporter.username, None)
         self.assertEqual(exporter.password, None)
         self.assertTrue(exporter.collector is None)
@@ -65,10 +60,7 @@ def test_constructor_default(self):
     def test_constructor_explicit(self):
         """Test the constructor passing all the options."""
         service = "my-opentelemetry-jaeger"
-        collector_host_name = "opentelemetry.io"
-        collector_port = 15875
-        collector_endpoint = "/myapi/traces?format=jaeger.thrift"
-        collector_protocol = "https"
+        collector_endpoint = "https://opentelemetry.io:15875"
 
         agent_port = 14268
         agent_host_name = "opentelemetry.io"
@@ -79,21 +71,16 @@ def test_constructor_explicit(self):
 
         exporter = jaeger_exporter.JaegerSpanExporter(
             service_name=service,
-            collector_host_name=collector_host_name,
-            collector_port=collector_port,
-            collector_endpoint=collector_endpoint,
-            collector_protocol="https",
             agent_host_name=agent_host_name,
             agent_port=agent_port,
+            collector_endpoint=collector_endpoint,
             username=username,
             password=password,
         )
+
         self.assertEqual(exporter.service_name, service)
         self.assertEqual(exporter.agent_host_name, agent_host_name)
         self.assertEqual(exporter.agent_port, agent_port)
-        self.assertEqual(exporter.collector_host_name, collector_host_name)
-        self.assertEqual(exporter.collector_port, collector_port)
-        self.assertEqual(exporter.collector_protocol, collector_protocol)
         self.assertTrue(exporter.collector is not None)
         self.assertEqual(exporter.collector.auth, auth)
         # property should not construct new object
@@ -107,6 +94,55 @@ def test_constructor_explicit(self):
         self.assertNotEqual(exporter.collector, collector)
         self.assertTrue(exporter.collector.auth is None)
 
+    def test_constructor_by_environment_variables(self):
+        """Test the constructor using Environment Variables."""
+        service = "my-opentelemetry-jaeger"
+
+        agent_host_name = "opentelemetry.io"
+        agent_port = "6831"
+
+        collector_endpoint = "https://opentelemetry.io:15875"
+
+        username = "username"
+        password = "password"
+        auth = (username, password)
+
+        environ_patcher = 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_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 is not None)
+        self.assertEqual(exporter.collector_endpoint, collector_endpoint)
+        self.assertEqual(exporter.collector.auth, auth)
+        # property should not construct new object
+        collector = exporter.collector
+        self.assertEqual(exporter.collector, collector)
+        # property should construct new object
+        # pylint: disable=protected-access
+        exporter._collector = None
+        exporter.username = None
+        exporter.password = None
+        self.assertNotEqual(exporter.collector, collector)
+        self.assertTrue(exporter.collector.auth is None)
+
+        environ_patcher.stop()
+
+        Configuration._reset()
+
     def test_nsec_to_usec_round(self):
         # pylint: disable=protected-access
         nsec_to_usec_round = jaeger_exporter._nsec_to_usec_round