From 6f03bdb26671ce4dcb4f6b76539dc77a2dc7098c Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 1 Sep 2020 21:12:41 -0700 Subject: [PATCH 1/4] add support for env var: OTEL_EXPORTER_ZIPKIN_ENDPOINT --- .../opentelemetry/exporter/zipkin/__init__.py | 20 ++++++++----- .../tests/test_zipkin_exporter.py | 30 +++++++++++++------ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index b0eb1bce0f1..748652c2936 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -63,7 +63,9 @@ import json import logging +import os from typing import Optional, Sequence +from urllib.parse import urlparse import requests @@ -75,6 +77,9 @@ DEFAULT_PORT = 9411 DEFAULT_PROTOCOL = "http" DEFAULT_RETRY = False +DEFAULT_URL = "{}://{}:{}{}".format( + DEFAULT_PROTOCOL, DEFAULT_HOST_NAME, DEFAULT_PORT, DEFAULT_ENDPOINT +) ZIPKIN_HEADERS = {"Content-Type": "application/json"} SPAN_KIND_MAP = { @@ -117,13 +122,14 @@ def __init__( retry: Optional[str] = DEFAULT_RETRY, ): self.service_name = service_name - self.host_name = host_name - self.port = port - self.endpoint = endpoint - self.protocol = protocol - self.url = "{}://{}:{}{}".format( - self.protocol, self.host_name, self.port, self.endpoint - ) + self.url = os.environ.get("OTEL_EXPORTER_ZIPKIN_ENDPOINT", DEFAULT_URL) + + url = "{}://{}:{}{}".format(protocol, host_name, port, endpoint) + if url != DEFAULT_URL: + self.url = url + + self.port = urlparse(self.url).port + self.ipv4 = ipv4 self.ipv6 = ipv6 self.retry = retry diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index f6e24a1495a..b55726f850c 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -13,6 +13,7 @@ # limitations under the License. import json +import os import unittest from unittest.mock import MagicMock, patch @@ -43,25 +44,39 @@ def setUp(self): self._test_span.start() self._test_span.end() + def tearDown(self): + if "OTEL_EXPORTER_ZIPKIN_ENDPOINT" in os.environ: + del os.environ["OTEL_EXPORTER_ZIPKIN_ENDPOINT"] + + def test_constructor_env_var(self): + """Test the default values assigned by constructor.""" + os.environ["OTEL_EXPORTER_ZIPKIN_ENDPOINT"] = "https://foo:9911/path" + service_name = "my-service-name" + port = 9911 + exporter = ZipkinSpanExporter(service_name) + ipv4 = None + ipv6 = None + url = "https://foo:9911/path" + + self.assertEqual(exporter.service_name, service_name) + self.assertEqual(exporter.port, port) + self.assertEqual(exporter.ipv4, ipv4) + self.assertEqual(exporter.ipv6, ipv6) + self.assertEqual(exporter.url, url) + def test_constructor_default(self): """Test the default values assigned by constructor.""" service_name = "my-service-name" - host_name = "localhost" port = 9411 - endpoint = "/api/v2/spans" exporter = ZipkinSpanExporter(service_name) ipv4 = None ipv6 = None - protocol = "http" url = "http://localhost:9411/api/v2/spans" self.assertEqual(exporter.service_name, service_name) - self.assertEqual(exporter.host_name, host_name) self.assertEqual(exporter.port, port) - self.assertEqual(exporter.endpoint, endpoint) self.assertEqual(exporter.ipv4, ipv4) self.assertEqual(exporter.ipv6, ipv6) - self.assertEqual(exporter.protocol, protocol) self.assertEqual(exporter.url, url) def test_constructor_explicit(self): @@ -85,12 +100,9 @@ def test_constructor_explicit(self): ) self.assertEqual(exporter.service_name, service_name) - self.assertEqual(exporter.host_name, host_name) self.assertEqual(exporter.port, port) - self.assertEqual(exporter.endpoint, endpoint) self.assertEqual(exporter.ipv4, ipv4) self.assertEqual(exporter.ipv6, ipv6) - self.assertEqual(exporter.protocol, protocol) self.assertEqual(exporter.url, url) # pylint: disable=too-many-locals From 320f84cc5b7f504da6a2195f933006403108e3de Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 1 Sep 2020 21:15:00 -0700 Subject: [PATCH 2/4] update changelog --- exporter/opentelemetry-exporter-zipkin/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md index 56678833143..6743f27d4dc 100644 --- a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Add support for OTEL_EXPORTER_ZIPKIN_ENDPOINT env var + ([#1064](https://github.com/open-telemetry/opentelemetry-python/pull/1064)) + ## Version 0.12b0 Released 2020-08-14 From 309d5b8e570a930e49788598fc8b542cb0207262 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 2 Sep 2020 11:47:36 -0700 Subject: [PATCH 3/4] add docs note --- .../src/opentelemetry/exporter/zipkin/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index 748652c2936..f15ce8422d6 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -24,6 +24,7 @@ .. _Zipkin: https://zipkin.io/ .. _OpenTelemetry: https://github.com/open-telemetry/opentelemetry-python/ +.. _Specification: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#zipkin-exporter .. code:: python @@ -57,6 +58,8 @@ with tracer.start_as_current_span("foo"): print("Hello world!") +The exporter supports endpoint configuration via the OTEL_EXPORTER_ZIPKIN_ENDPOINT environment variables as defined in the `Specification`_ + API --- """ From 0beb750896b6786da3db61429826d765e2cb61c7 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 3 Sep 2020 13:18:27 -0700 Subject: [PATCH 4/4] update method signature to use url --- .../CHANGELOG.md | 5 ++- .../opentelemetry/exporter/zipkin/__init__.py | 32 ++++++------------- .../tests/test_zipkin_exporter.py | 17 +++------- 3 files changed, 17 insertions(+), 37 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md index 6743f27d4dc..b1066d081a5 100644 --- a/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-zipkin/CHANGELOG.md @@ -2,7 +2,10 @@ ## Unreleased -- Add support for OTEL_EXPORTER_ZIPKIN_ENDPOINT env var +- Add support for OTEL_EXPORTER_ZIPKIN_ENDPOINT env var. As part of this change, the + configuration of the ZipkinSpanExporter exposes a `url` argument to replace `host_name`, + `port`, `protocol`, `endpoint`. This brings this implementation inline with other + implementations. ([#1064](https://github.com/open-telemetry/opentelemetry-python/pull/1064)) ## Version 0.12b0 diff --git a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py index f15ce8422d6..c62e07e4143 100644 --- a/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin/src/opentelemetry/exporter/zipkin/__init__.py @@ -40,10 +40,7 @@ zipkin_exporter = zipkin.ZipkinSpanExporter( service_name="my-helloworld-service", # optional: - # host_name="localhost", - # port=9411, - # endpoint="/api/v2/spans", - # protocol="http", + # url="http://localhost:9411/api/v2/spans", # ipv4="", # ipv6="", # retry=False, @@ -75,14 +72,8 @@ from opentelemetry.sdk.trace.export import SpanExporter, SpanExportResult from opentelemetry.trace import Span, SpanContext, SpanKind -DEFAULT_ENDPOINT = "/api/v2/spans" -DEFAULT_HOST_NAME = "localhost" -DEFAULT_PORT = 9411 -DEFAULT_PROTOCOL = "http" DEFAULT_RETRY = False -DEFAULT_URL = "{}://{}:{}{}".format( - DEFAULT_PROTOCOL, DEFAULT_HOST_NAME, DEFAULT_PORT, DEFAULT_ENDPOINT -) +DEFAULT_URL = "http://localhost:9411/api/v2/spans" ZIPKIN_HEADERS = {"Content-Type": "application/json"} SPAN_KIND_MAP = { @@ -104,10 +95,7 @@ class ZipkinSpanExporter(SpanExporter): Args: service_name: Service that logged an annotation in a trace.Classifier when query for spans. - host_name: The host name of the Zipkin server - port: The port of the Zipkin server - endpoint: The endpoint of the Zipkin server - protocol: The protocol used for the request. + url: The Zipkin endpoint URL ipv4: Primary IPv4 address associated with this connection. ipv6: Primary IPv6 address associated with this connection. retry: Set to True to configure the exporter to retry on failure. @@ -116,19 +104,17 @@ class ZipkinSpanExporter(SpanExporter): def __init__( self, service_name: str, - host_name: str = DEFAULT_HOST_NAME, - port: int = DEFAULT_PORT, - endpoint: str = DEFAULT_ENDPOINT, - protocol: str = DEFAULT_PROTOCOL, + url: str = None, ipv4: Optional[str] = None, ipv6: Optional[str] = None, retry: Optional[str] = DEFAULT_RETRY, ): self.service_name = service_name - self.url = os.environ.get("OTEL_EXPORTER_ZIPKIN_ENDPOINT", DEFAULT_URL) - - url = "{}://{}:{}{}".format(protocol, host_name, port, endpoint) - if url != DEFAULT_URL: + if url is None: + self.url = os.environ.get( + "OTEL_EXPORTER_ZIPKIN_ENDPOINT", DEFAULT_URL + ) + else: self.url = url self.port = urlparse(self.url).port diff --git a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py index b55726f850c..ae28dd342bc 100644 --- a/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin/tests/test_zipkin_exporter.py @@ -50,19 +50,19 @@ def tearDown(self): def test_constructor_env_var(self): """Test the default values assigned by constructor.""" - os.environ["OTEL_EXPORTER_ZIPKIN_ENDPOINT"] = "https://foo:9911/path" + url = "https://foo:9911/path" + os.environ["OTEL_EXPORTER_ZIPKIN_ENDPOINT"] = url service_name = "my-service-name" port = 9911 exporter = ZipkinSpanExporter(service_name) ipv4 = None ipv6 = None - url = "https://foo:9911/path" self.assertEqual(exporter.service_name, service_name) - self.assertEqual(exporter.port, port) self.assertEqual(exporter.ipv4, ipv4) self.assertEqual(exporter.ipv6, ipv6) self.assertEqual(exporter.url, url) + self.assertEqual(exporter.port, port) def test_constructor_default(self): """Test the default values assigned by constructor.""" @@ -82,21 +82,12 @@ def test_constructor_default(self): def test_constructor_explicit(self): """Test the constructor passing all the options.""" service_name = "my-opentelemetry-zipkin" - host_name = "opentelemetry.io" port = 15875 - endpoint = "/myapi/traces?format=zipkin" ipv4 = "1.2.3.4" ipv6 = "2001:0db8:85a3:0000:0000:8a2e:0370:7334" - protocol = "https" url = "https://opentelemetry.io:15875/myapi/traces?format=zipkin" exporter = ZipkinSpanExporter( - service_name=service_name, - host_name=host_name, - port=port, - endpoint=endpoint, - ipv4=ipv4, - ipv6=ipv6, - protocol=protocol, + service_name=service_name, url=url, ipv4=ipv4, ipv6=ipv6, ) self.assertEqual(exporter.service_name, service_name)