Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pass OTLP metadata as tuple instead of string #1507

Merged
merged 5 commits into from
Jan 7, 2021
Merged

Pass OTLP metadata as tuple instead of string #1507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ffd7daa
pass OTLP metadata as tuple instead of string
Jan 4, 2021
c065ff9
update changelog
Jan 4, 2021
64acd13
Merge branch 'master' into fix-headers
Jan 4, 2021
e3dd5a4
Merge branch 'master' into fix-headers
Jan 6, 2021
132a8fc
Merge branch 'master' into fix-headers
Jan 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#1486](https://github.com/open-telemetry/opentelemetry-python/pull/1486))
- Recreate span on every run of a `start_as_current_span`-decorated function
([#1451](https://github.com/open-telemetry/opentelemetry-python/pull/1451))
- `opentelemetry-exporter-otlp` Headers are now passed in as tuple as metadata, instead of a
string, which was incorrect.
([#1507](https://github.com/open-telemetry/opentelemetry-python/pull/1507))

## [0.16b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v0.16b1) - 2020-11-26
### Added
Expand Down
6 changes: 5 additions & 1 deletion exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def __init__(
endpoint: Optional[str] = None,
insecure: Optional[bool] = None,
credentials: Optional[ChannelCredentials] = None,
headers: Optional[str] = None,
headers: Optional[Sequence] = None,
timeout: Optional[int] = None,
compression: str = None,
):
Expand All @@ -169,6 +169,10 @@ def __init__(
insecure = False

self._headers = headers or Configuration().EXPORTER_OTLP_HEADERS
if isinstance(self._headers, str):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we are changing from : to = delimiters? No opinion, just curious. Also, once we have a place where all the env vars are documented, this specific format should probably be outlined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the delimiter as per the spec is =: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/exporter.md#specifying-headers-via-environment-variables

self._headers = tuple(
tuple(item.split("=")) for item in self._headers.split(",")
)
self._timeout = (
timeout
or Configuration().EXPORTER_OTLP_TIMEOUT
Expand Down
2 changes: 1 addition & 1 deletion .../opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/metrics_exporter/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def __init__(
endpoint: Optional[str] = None,
insecure: Optional[bool] = None,
credentials: Optional[ChannelCredentials] = None,
headers: Optional[str] = None,
headers: Optional[Sequence] = None,
timeout: Optional[int] = None,
):
if insecure is None:
Expand Down
2 changes: 1 addition & 1 deletion ...er/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/trace_exporter/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def __init__(
endpoint: Optional[str] = None,
insecure: Optional[bool] = None,
credentials: Optional[ChannelCredentials] = None,
headers: Optional[str] = None,
headers: Optional[Sequence] = None,
timeout: Optional[int] = None,
):
if insecure is None:
Expand Down
33 changes: 31 additions & 2 deletions exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def tearDown(self):
"OTEL_EXPORTER_OTLP_METRIC_ENDPOINT": "collector:55680",
"OTEL_EXPORTER_OTLP_METRIC_CERTIFICATE": THIS_DIR
+ "/fixtures/test.cert",
"OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1:value1;key2:value2",
"OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1=value1,key2=value2",
"OTEL_EXPORTER_OTLP_METRIC_TIMEOUT": "10",
},
)
Expand All @@ -85,7 +85,7 @@ def test_env_variables(self, mock_exporter_mixin):
_, kwargs = mock_exporter_mixin.call_args_list[0]

self.assertEqual(kwargs["endpoint"], "collector:55680")
self.assertEqual(kwargs["headers"], "key1:value1;key2:value2")
self.assertEqual(kwargs["headers"], "key1=value1,key2=value2")
self.assertEqual(kwargs["timeout"], 10)
self.assertIsNotNone(kwargs["credentials"])
self.assertIsInstance(kwargs["credentials"], ChannelCredentials)
Expand All @@ -102,6 +102,35 @@ def test_no_credentials_error(
OTLPMetricsExporter(insecure=False)
self.assertTrue(mock_ssl_channel.called)

@patch.dict(
"os.environ",
{"OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1=value1,key2=value2"},
)
@patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
@patch("opentelemetry.exporter.otlp.exporter.secure_channel")
# pylint: disable=unused-argument
def test_otlp_headers_from_env(self, mock_ssl_channel, mock_secure):
exporter = OTLPMetricsExporter()
# pylint: disable=protected-access
self.assertEqual(
exporter._headers, (("key1", "value1"), ("key2", "value2")),
)
exporter = OTLPMetricsExporter(
headers=(("key3", "value3"), ("key4", "value4"))
)
# pylint: disable=protected-access
self.assertEqual(
exporter._headers, (("key3", "value3"), ("key4", "value4")),
)

@patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
@patch("opentelemetry.exporter.otlp.exporter.secure_channel")
# pylint: disable=unused-argument
def test_otlp_headers(self, mock_ssl_channel, mock_secure):
exporter = OTLPMetricsExporter()
# pylint: disable=protected-access
self.assertIsNone(exporter._headers, None)

@patch("opentelemetry.sdk.metrics.export.aggregate.time_ns")
def test_translate_counter_export_record(self, mock_time_ns):
mock_time_ns.configure_mock(**{"return_value": 1})
Expand Down
33 changes: 31 additions & 2 deletions exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def tearDown(self):
"OTEL_EXPORTER_OTLP_SPAN_ENDPOINT": "collector:55680",
"OTEL_EXPORTER_OTLP_SPAN_CERTIFICATE": THIS_DIR
+ "/fixtures/test.cert",
"OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1:value1;key2:value2",
"OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1=value1,key2=value2",
"OTEL_EXPORTER_OTLP_SPAN_TIMEOUT": "10",
},
)
Expand All @@ -184,7 +184,7 @@ def test_env_variables(self, mock_exporter_mixin):
_, kwargs = mock_exporter_mixin.call_args_list[0]

self.assertEqual(kwargs["endpoint"], "collector:55680")
self.assertEqual(kwargs["headers"], "key1:value1;key2:value2")
self.assertEqual(kwargs["headers"], "key1=value1,key2=value2")
self.assertEqual(kwargs["timeout"], 10)
self.assertIsNotNone(kwargs["credentials"])
self.assertIsInstance(kwargs["credentials"], ChannelCredentials)
Expand All @@ -199,6 +199,35 @@ def test_no_credentials_error(
OTLPSpanExporter(insecure=False)
self.assertTrue(mock_ssl_channel.called)

@patch.dict(
"os.environ",
{"OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1=value1,key2=value2"},
)
@patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
@patch("opentelemetry.exporter.otlp.exporter.secure_channel")
# pylint: disable=unused-argument
def test_otlp_headers_from_env(self, mock_ssl_channel, mock_secure):
exporter = OTLPSpanExporter()
# pylint: disable=protected-access
self.assertEqual(
exporter._headers, (("key1", "value1"), ("key2", "value2"))
)
exporter = OTLPSpanExporter(
headers=(("key3", "value3"), ("key4", "value4"))
)
# pylint: disable=protected-access
self.assertEqual(
exporter._headers, (("key3", "value3"), ("key4", "value4"))
)

@patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials")
@patch("opentelemetry.exporter.otlp.exporter.secure_channel")
# pylint: disable=unused-argument
def test_otlp_headers(self, mock_ssl_channel, mock_secure):
exporter = OTLPSpanExporter()
# pylint: disable=protected-access
self.assertIsNone(exporter._headers, None)

@patch("opentelemetry.exporter.otlp.exporter.expo")
@patch("opentelemetry.exporter.otlp.exporter.sleep")
def test_unavailable(self, mock_sleep, mock_expo):
Expand Down