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.

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

Add Env variables in OTLP exporter #1101

Merged
merged 36 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
416d55d
Add env vars for endpoint, credentials, headers
jan25 Sep 13, 2020
a24a88d
Read cert from file
jan25 Sep 13, 2020
ced438a
Use configuration library
jan25 Sep 20, 2020
da5a7a2
Drop .env local setup
jan25 Sep 20, 2020
045ad91
Drop .env local setup;
jan25 Sep 20, 2020
710a60b
Ensure setting default insecure variable
jan25 Sep 20, 2020
d6995de
Lint
jan25 Sep 20, 2020
47a5dcb
Fix tests
jan25 Sep 20, 2020
c704a55
Lint
jan25 Sep 20, 2020
168771f
Print error message
jan25 Sep 20, 2020
8e66bd1
Fix lint from ci
jan25 Sep 21, 2020
fc4429e
Revert "Fix lint from ci"
jan25 Sep 21, 2020
269b8cb
Update changelog
jan25 Sep 21, 2020
d4c6192
revert insecure default change
jan25 Oct 1, 2020
e4c2c09
setup common env var for headers
jan25 Oct 1, 2020
0b3e5fc
setup common env var for headers
jan25 Oct 1, 2020
30ac5e7
fix tests
jan25 Oct 1, 2020
8ed3903
Fix typo
jan25 Oct 10, 2020
dcd4f69
Add env variables tests
jan25 Oct 11, 2020
acd3024
Lint
jan25 Oct 11, 2020
b9527ef
lint
jan25 Oct 16, 2020
af89d7d
rename metadata to headers
jan25 Oct 16, 2020
f0d415d
lint
jan25 Oct 16, 2020
f9ae58d
optional args
jan25 Oct 16, 2020
4e70e10
lint
jan25 Oct 16, 2020
eff69d4
lint
jan25 Oct 16, 2020
82e78de
Add timeout env variable
jan25 Oct 16, 2020
49b2ed6
lint
jan25 Oct 16, 2020
bdaebb1
Merge branch 'master' into issue-1004
Oct 22, 2020
a9f5bae
Merge branch 'master' into issue-1004
Oct 23, 2020
500f8c7
Fix unittest api issues
jan25 Oct 27, 2020
5a5708c
Fix changelog
jan25 Oct 27, 2020
593f459
Remove unused imports
jan25 Oct 27, 2020
6f37169
Merge branch 'master' into issue-1004
Oct 28, 2020
9483813
Update CHANGELOG.md
Oct 28, 2020
2ff2492
Merge branch 'master' into issue-1004
Oct 28, 2020
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 exporter/opentelemetry-exporter-otlp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

- Add Env variables in OTLP exporter
([#1101](https://github.com/open-telemetry/opentelemetry-python/pull/1101))

codeboten marked this conversation as resolved.
Show resolved Hide resolved
## Version 0.14b0

Released 2020-10-13
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""OTLP Exporter"""

import logging
import os
from abc import ABC, abstractmethod
from collections.abc import Mapping, Sequence
from time import sleep
Expand All @@ -30,8 +31,10 @@
StatusCode,
insecure_channel,
secure_channel,
ssl_channel_credentials,
)

from opentelemetry.configuration import Configuration
from opentelemetry.proto.common.v1.common_pb2 import AnyValue, KeyValue
from opentelemetry.proto.resource.v1.resource_pb2 import Resource
from opentelemetry.sdk.resources import Resource as SDKResource
Expand Down Expand Up @@ -113,6 +116,16 @@ def _get_resource_data(
return resource_data


def _load_credential_from_file(filepath) -> ChannelCredentials:
try:
with open(filepath, "rb") as f:
Copy link
Contributor

@NathanielRN NathanielRN Oct 30, 2020

Choose a reason for hiding this comment

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

Tried to use the OTLP Exporter today and I think I ran into an issue. If you don't have the OTEL_PYTHON_EXPORTER_OTLP_CERTIFICATE Env variable set, then the call to Configuration().EXPORTER_OTLP_CERTIFICATE

https://github.com/open-telemetry/opentelemetry-python/blob/master/exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py#L175

will return None instead of a String. Then this line will throw error

TypeError: expected str, bytes or os.PathLike object, not NoneType

which is not caught by FileNotFoundError below.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest opening a new issue to track these problems instead of continuing an already closed PR thread.

credential = f.read()
return ssl_channel_credentials(credential)
except FileNotFoundError:
logger.exception("Failed to read credential file")
return None


# pylint: disable=no-member
class OTLPExporterMixin(
ABC, Generic[SDKDataT, ExportServiceRequestT, ExportResultT]
Expand All @@ -121,24 +134,47 @@ class OTLPExporterMixin(

Args:
endpoint: OpenTelemetry Collector receiver endpoint
insecure: Connection type
credentials: ChannelCredentials object for server authentication
metadata: Metadata to send when exporting
timeout: Backend request timeout in seconds
"""

def __init__(
self,
endpoint: str = "localhost:55680",
credentials: ChannelCredentials = None,
metadata: Optional[Tuple[Any]] = None,
endpoint: Optional[str] = None,
insecure: Optional[bool] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing protocol, certificate_file in constructor, compression and timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could add env vars for protocol compression when those usecases are implemented. Currently exporter uses grpc only and no implementation for compression.

certificate_file is wrapped in ChannelCredentials object which is passed down from OTLPSpanExporter|MetricExporter. I'll change timeout logic to use env var too.

Copy link
Contributor

Choose a reason for hiding this comment

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

credentials: Optional[ChannelCredentials] = None,
headers: Optional[str] = None,
timeout: Optional[int] = None,
):
super().__init__()

self._metadata = metadata
endpoint = (
endpoint
or Configuration().EXPORTER_OTLP_ENDPOINT
or "localhost:55680"
)

if insecure is None:
lzchen marked this conversation as resolved.
Show resolved Hide resolved
insecure = Configuration().EXPORTER_OTLP_INSECURE
if insecure is None:
insecure = False

self._headers = headers or Configuration().EXPORTER_OTLP_HEADERS
self._timeout = (
timeout
or Configuration().EXPORTER_OTLP_TIMEOUT
or 10 # default: 10 seconds
)
self._collector_span_kwargs = None

if credentials is None:
if insecure:
self._client = self._stub(insecure_channel(endpoint))
else:
credentials = credentials or _load_credential_from_file(
Configuration().EXPORTER_OTLP_CERTIFICATE
)
self._client = self._stub(secure_channel(endpoint, credentials))

@abstractmethod
Expand All @@ -164,7 +200,8 @@ def _export(self, data: TypingSequence[SDKDataT]) -> ExportResultT:
try:
self._client.Export(
request=self._translate_data(data),
metadata=self._metadata,
metadata=self._headers,
timeout=self._timeout,
)

return self._result.SUCCESS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@
"""OTLP Metrics Exporter"""

import logging
from typing import List, Sequence, Type, TypeVar
import os
from typing import List, Optional, Sequence, Type, TypeVar, Union

# pylint: disable=duplicate-code
from grpc import ChannelCredentials

from opentelemetry.configuration import Configuration
from opentelemetry.exporter.otlp.exporter import (
OTLPExporterMixin,
_get_resource_data,
_load_credential_from_file,
)
from opentelemetry.proto.collector.metrics.v1.metrics_service_pb2 import (
ExportMetricsServiceRequest,
Expand Down Expand Up @@ -109,13 +113,47 @@ class OTLPMetricsExporter(

Args:
endpoint: OpenTelemetry Collector receiver endpoint
insecure: Connection type
credentials: Credentials object for server authentication
metadata: Metadata to send when exporting
timeout: Backend request timeout in seconds
"""

_stub = MetricsServiceStub
_result = MetricsExportResult

def __init__(
self,
endpoint: Optional[str] = None,
insecure: Optional[bool] = None,
credentials: Optional[ChannelCredentials] = None,
headers: Optional[str] = None,
timeout: Optional[int] = None,
):
jan25 marked this conversation as resolved.
Show resolved Hide resolved
if insecure is None:
insecure = Configuration().EXPORTER_OTLP_METRIC_INSECURE

if (
not insecure
and Configuration().EXPORTER_OTLP_METRIC_CERTIFICATE is not None
):
credentials = credentials or _load_credential_from_file(
Configuration().EXPORTER_OTLP_METRIC_CERTIFICATE
)

super().__init__(
**{
"endpoint": endpoint
or Configuration().EXPORTER_OTLP_METRIC_ENDPOINT,
"insecure": insecure,
"credentials": credentials,
"headers": headers
or Configuration().EXPORTER_OTLP_METRIC_HEADERS,
"timeout": timeout
or Configuration().EXPORTER_OTLP_METRIC_TIMEOUT,
}
)

# pylint: disable=no-self-use
def _translate_data(
self, data: Sequence[MetricRecord]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@
"""OTLP Span Exporter"""

import logging
from typing import Sequence
import os
from typing import Optional, Sequence

from grpc import ChannelCredentials

from opentelemetry.configuration import Configuration
from opentelemetry.exporter.otlp.exporter import (
OTLPExporterMixin,
_get_resource_data,
_load_credential_from_file,
_translate_key_values,
)
from opentelemetry.proto.collector.trace.v1.trace_service_pb2 import (
Expand Down Expand Up @@ -50,13 +55,47 @@ class OTLPSpanExporter(

Args:
endpoint: OpenTelemetry Collector receiver endpoint
insecure: Connection type
credentials: Credentials object for server authentication
metadata: Metadata to send when exporting
timeout: Backend request timeout in seconds
"""

_result = SpanExportResult
_stub = TraceServiceStub

def __init__(
self,
endpoint: Optional[str] = None,
insecure: Optional[bool] = None,
credentials: Optional[ChannelCredentials] = None,
headers: Optional[str] = None,
timeout: Optional[int] = None,
):
if insecure is None:
insecure = Configuration().EXPORTER_OTLP_SPAN_INSECURE

if (
not insecure
and Configuration().EXPORTER_OTLP_SPAN_CERTIFICATE is not None
):
credentials = credentials or _load_credential_from_file(
Configuration().EXPORTER_OTLP_SPAN_CERTIFICATE
)

super().__init__(
**{
"endpoint": endpoint
or Configuration().EXPORTER_OTLP_SPAN_ENDPOINT,
"insecure": insecure,
"credentials": credentials,
Copy link
Contributor

Choose a reason for hiding this comment

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

If I set OTLPExporter(insecure=True) this works fine, but if I leave that out, I get a message

File "/Users/enowell/git/opentelemetry-python/exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py", line 178, in __init__
    self._client = self._stub(secure_channel(endpoint, credentials))
  File "/Users/enowell/Library/Python/3.8/lib/python/site-packages/grpc/__init__.py", line 1943, in secure_channel
    if credentials._credentials is _insecure_channel_credentials:
AttributeError: 'NoneType' object has no attribute '_credentials'

which makes sense because insecure=None and credentials=None. I think a follow up would be to throw an Error Message if insecure=None (or False) && credentials=None saying "Error: Either provide credentials or label as insecure."

Choose a reason for hiding this comment

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

@NathanielRN Thanks for reporting this. I also ran into this issue. Causes a failure in the example code.

"headers": headers
or Configuration().EXPORTER_OTLP_SPAN_HEADERS,
"timeout": timeout
or Configuration().EXPORTER_OTLP_SPAN_TIMEOUT,
}
)

def _translate_name(self, sdk_span: SDKSpan) -> None:
self._collector_span_kwargs["name"] = sdk_span.name

Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from unittest import TestCase
from unittest.mock import patch

from grpc import ChannelCredentials

from opentelemetry.configuration import Configuration
from opentelemetry.exporter.otlp.metrics_exporter import OTLPMetricsExporter
from opentelemetry.proto.collector.metrics.v1.metrics_service_pb2 import (
ExportMetricsServiceRequest,
Expand Down Expand Up @@ -44,8 +47,9 @@

class TestOTLPMetricExporter(TestCase):
def setUp(self):
self.exporter = OTLPMetricsExporter()
self.exporter = OTLPMetricsExporter(insecure=True)
resource = SDKResource(OrderedDict([("a", 1), ("b", False)]))

self.counter_metric_record = MetricRecord(
Counter(
"a",
Expand All @@ -60,6 +64,33 @@ def setUp(self):
resource,
)

Configuration._reset() # pylint: disable=protected-access

def tearDown(self):
Configuration._reset() # pylint: disable=protected-access

@patch.dict(
"os.environ",
{
"OTEL_EXPORTER_OTLP_METRIC_ENDPOINT": "collector:55680",
"OTEL_EXPORTER_OTLP_METRIC_CERTIFICATE": "fixtures/test.cert",
"OTEL_EXPORTER_OTLP_METRIC_HEADERS": "key1:value1;key2:value2",
"OTEL_EXPORTER_OTLP_METRIC_TIMEOUT": "10",
},
)
@patch("opentelemetry.exporter.otlp.exporter.OTLPExporterMixin.__init__")
def test_env_variables(self, mock_exporter_mixin):
OTLPMetricsExporter()

self.assertTrue(len(mock_exporter_mixin.call_args_list) == 1)
_, 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["timeout"], 10)
self.assertIsNotNone(kwargs["credentials"])
self.assertIsInstance(kwargs["credentials"], ChannelCredentials)

@patch("opentelemetry.sdk.metrics.export.aggregate.time_ns")
def test_translate_metrics(self, mock_time_ns):
# pylint: disable=no-member
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

from google.protobuf.duration_pb2 import Duration
from google.rpc.error_details_pb2 import RetryInfo
from grpc import StatusCode, server
from grpc import ChannelCredentials, StatusCode, server

from opentelemetry.configuration import Configuration
from opentelemetry.exporter.otlp.trace_exporter import OTLPSpanExporter
from opentelemetry.proto.collector.trace.v1.trace_service_pb2 import (
ExportTraceServiceRequest,
Expand Down Expand Up @@ -102,7 +103,7 @@ def Export(self, request, context):
class TestOTLPSpanExporter(TestCase):
def setUp(self):
tracer_provider = TracerProvider()
self.exporter = OTLPSpanExporter()
self.exporter = OTLPSpanExporter(insecure=True)
tracer_provider.add_span_processor(
SimpleExportSpanProcessor(self.exporter)
)
Expand Down Expand Up @@ -154,8 +155,33 @@ def setUp(self):
self.span.start()
self.span.end()

Configuration._reset() # pylint: disable=protected-access

def tearDown(self):
self.server.stop(None)
Configuration._reset() # pylint: disable=protected-access

@patch.dict(
"os.environ",
{
"OTEL_EXPORTER_OTLP_SPAN_ENDPOINT": "collector:55680",
"OTEL_EXPORTER_OTLP_SPAN_CERTIFICATE": "fixtures/test.cert",
"OTEL_EXPORTER_OTLP_SPAN_HEADERS": "key1:value1;key2:value2",
"OTEL_EXPORTER_OTLP_SPAN_TIMEOUT": "10",
},
)
@patch("opentelemetry.exporter.otlp.exporter.OTLPExporterMixin.__init__")
def test_env_variables(self, mock_exporter_mixin):
OTLPSpanExporter()

self.assertTrue(len(mock_exporter_mixin.call_args_list) == 1)
_, 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["timeout"], 10)
self.assertIsNotNone(kwargs["credentials"])
self.assertIsInstance(kwargs["credentials"], ChannelCredentials)

@patch("opentelemetry.exporter.otlp.exporter.expo")
@patch("opentelemetry.exporter.otlp.exporter.sleep")
Expand Down