-
Notifications
You must be signed in to change notification settings - Fork 734
Add credentials environment variables to let ChannelCredentials
and Session
be injected
#4689
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
1e68c3c
06dee51
c500b15
a6dcd1c
5ef26f2
2e76e59
9344ebd
cc89293
f27856f
82d72bb
2244d5a
afaf88d
4881780
66fb14d
fcadee2
6c2740a
0dc2861
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,13 +19,16 @@ | |
|
||
from __future__ import annotations | ||
|
||
import inspect | ||
import logging | ||
import logging.config | ||
import os | ||
from abc import ABC, abstractmethod | ||
from os import environ | ||
from typing import Any, Callable, Mapping, Sequence, Type, Union | ||
from typing import Any, Callable, Mapping, Optional, Sequence, Type, Union | ||
|
||
from grpc import ChannelCredentials # pylint: disable=import-error | ||
from requests import Session | ||
from typing_extensions import Literal | ||
|
||
from opentelemetry._events import set_event_logger_provider | ||
|
@@ -46,6 +49,10 @@ | |
OTEL_EXPORTER_OTLP_METRICS_PROTOCOL, | ||
OTEL_EXPORTER_OTLP_PROTOCOL, | ||
OTEL_EXPORTER_OTLP_TRACES_PROTOCOL, | ||
OTEL_PYTHON_EXPORTER_OTLP_CREDENTIAL_PROVIDER, | ||
OTEL_PYTHON_EXPORTER_OTLP_LOGS_CREDENTIAL_PROVIDER, | ||
OTEL_PYTHON_EXPORTER_OTLP_METRICS_CREDENTIAL_PROVIDER, | ||
OTEL_PYTHON_EXPORTER_OTLP_TRACES_CREDENTIAL_PROVIDER, | ||
OTEL_TRACES_SAMPLER, | ||
OTEL_TRACES_SAMPLER_ARG, | ||
) | ||
|
@@ -79,6 +86,12 @@ | |
"logs": OTEL_LOGS_EXPORTER, | ||
} | ||
|
||
_EXPORTER_CREDENTIAL_BY_SIGNAL_TYPE = { | ||
"traces": OTEL_PYTHON_EXPORTER_OTLP_TRACES_CREDENTIAL_PROVIDER, | ||
"metrics": OTEL_PYTHON_EXPORTER_OTLP_METRICS_CREDENTIAL_PROVIDER, | ||
"logs": OTEL_PYTHON_EXPORTER_OTLP_LOGS_CREDENTIAL_PROVIDER, | ||
} | ||
|
||
_PROTOCOL_ENV_BY_SIGNAL_TYPE = { | ||
"traces": OTEL_EXPORTER_OTLP_TRACES_PROTOCOL, | ||
"metrics": OTEL_EXPORTER_OTLP_METRICS_PROTOCOL, | ||
|
@@ -103,6 +116,36 @@ | |
] | ||
|
||
|
||
def _load_credential_from_envvar( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better altough we are doing this kind of defensive import for psutil in SDK for resources There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean just the credentials? Are you going to load them in exporters There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes just credentials. Yes we would load the entry point in the various OTLP exporter It's maybe a little weird doing it there because credentials can also be passed directly to the constructor, but I agree with Aaron that it makes the implementation cleaner.. I see we do use Sounding like we should do it this way instead.. |
||
environment_variable: str, | ||
) -> Optional[ | ||
tuple[ | ||
Literal["credentials", "session"], Union[ChannelCredentials, Session] | ||
] | ||
]: | ||
credential_env = os.getenv(environment_variable) | ||
if credential_env: | ||
credentials = _import_config_component( | ||
credential_env, "opentelemetry_otlp_credential_provider" | ||
DylanRussell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
if isinstance(credentials, ChannelCredentials): | ||
return ("credentials", credentials) | ||
elif isinstance(credentials, Session): | ||
return ("session", credentials) | ||
else: | ||
raise RuntimeError( | ||
f"{credential_env} is neither a ChannelCredentials or Session type." | ||
) | ||
|
||
|
||
def _import_config_component( | ||
selected_component: str, entry_point_name: str | ||
) -> Type: | ||
return _import_config_components([selected_component], entry_point_name)[ | ||
0 | ||
][1] | ||
|
||
|
||
def _import_config_components( | ||
selected_components: Sequence[str], entry_point_name: str | ||
) -> list[tuple[str, Type]]: | ||
|
@@ -202,12 +245,54 @@ def _get_exporter_names( | |
] | ||
|
||
|
||
def _init_exporter( | ||
signal_type: Literal["traces", "metrics", "logs"], | ||
exporter_args_map: Mapping[str, Any], | ||
DylanRussell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
exporter_class: Union[ | ||
Type[SpanExporter], Type[MetricExporter], Type[LogExporter] | ||
], | ||
otlp_credential_param_for_all_signal_types: Optional[ | ||
tuple[ | ||
Literal["credentials", "session"], | ||
Union[ChannelCredentials, Session], | ||
] | ||
] = None, | ||
) -> Union[SpanExporter, MetricExporter, LogExporter]: | ||
# Per signal type envvar should take precedence over all signal type env var. | ||
otlp_credential_param = ( | ||
_load_credential_from_envvar( | ||
_EXPORTER_CREDENTIAL_BY_SIGNAL_TYPE[signal_type] | ||
) | ||
or otlp_credential_param_for_all_signal_types | ||
) | ||
if not otlp_credential_param: | ||
return exporter_class(**exporter_args_map) | ||
credential_key, credential = otlp_credential_param | ||
params = inspect.signature(exporter_class.__init__).parameters | ||
if ( | ||
credential_key == "credentials" | ||
and "credentials" in params | ||
and "ChannelCredentials" in params["credentials"].annotation | ||
): | ||
return exporter_class(credentials=credential, **exporter_args_map) | ||
if ( | ||
credential_key == "session" | ||
and "session" in params | ||
and "Session" in params["session"].annotation | ||
): | ||
return exporter_class(session=credential, **exporter_args_map) | ||
return exporter_class(**exporter_args_map) | ||
|
||
|
||
def _init_tracing( | ||
exporters: dict[str, Type[SpanExporter]], | ||
id_generator: IdGenerator | None = None, | ||
sampler: Sampler | None = None, | ||
resource: Resource | None = None, | ||
exporter_args_map: ExporterArgsMap | None = None, | ||
otlp_credential_param: Optional[ | ||
tuple[str, Union[ChannelCredentials, Session]] | ||
] = None, | ||
): | ||
provider = TracerProvider( | ||
id_generator=id_generator, | ||
|
@@ -220,16 +305,26 @@ def _init_tracing( | |
for _, exporter_class in exporters.items(): | ||
exporter_args = exporter_args_map.get(exporter_class, {}) | ||
provider.add_span_processor( | ||
BatchSpanProcessor(exporter_class(**exporter_args)) | ||
BatchSpanProcessor( | ||
_init_exporter( | ||
"traces", | ||
exporter_args, | ||
exporter_class, | ||
otlp_credential_param, | ||
) | ||
) | ||
) | ||
|
||
|
||
def _init_metrics( | ||
exporters_or_readers: dict[ | ||
str, Union[Type[MetricExporter], Type[MetricReader]] | ||
], | ||
resource: Resource | None = None, | ||
resource: Resource = None, | ||
exporter_args_map: ExporterArgsMap | None = None, | ||
otlp_credential_param: Optional[ | ||
tuple[str, Union[ChannelCredentials, Session]] | ||
] = None, | ||
): | ||
metric_readers = [] | ||
|
||
|
@@ -241,7 +336,12 @@ def _init_metrics( | |
else: | ||
metric_readers.append( | ||
PeriodicExportingMetricReader( | ||
exporter_or_reader_class(**exporter_args) | ||
_init_exporter( | ||
"metrics", | ||
exporter_args, | ||
exporter_or_reader_class, | ||
otlp_credential_param, | ||
) | ||
) | ||
) | ||
|
||
|
@@ -254,6 +354,9 @@ def _init_logging( | |
resource: Resource | None = None, | ||
setup_logging_handler: bool = True, | ||
exporter_args_map: ExporterArgsMap | None = None, | ||
otlp_credential_param: Optional[ | ||
tuple[str, Union[ChannelCredentials, Session]] | ||
] = None, | ||
): | ||
provider = LoggerProvider(resource=resource) | ||
set_logger_provider(provider) | ||
|
@@ -262,7 +365,14 @@ def _init_logging( | |
for _, exporter_class in exporters.items(): | ||
exporter_args = exporter_args_map.get(exporter_class, {}) | ||
provider.add_log_record_processor( | ||
BatchLogRecordProcessor(exporter_class(**exporter_args)) | ||
BatchLogRecordProcessor( | ||
_init_exporter( | ||
"logs", | ||
exporter_args, | ||
exporter_class, | ||
otlp_credential_param, | ||
) | ||
) | ||
) | ||
|
||
event_logger_provider = EventLoggerProvider(logger_provider=provider) | ||
|
@@ -445,15 +555,22 @@ def _initialize_components( | |
# from the env variable else defaults to "unknown_service" | ||
resource = Resource.create(resource_attributes) | ||
|
||
otlp_credential_param = _load_credential_from_envvar( | ||
OTEL_PYTHON_EXPORTER_OTLP_CREDENTIAL_PROVIDER | ||
) | ||
_init_tracing( | ||
exporters=span_exporters, | ||
id_generator=id_generator, | ||
sampler=sampler, | ||
resource=resource, | ||
otlp_credential_param=otlp_credential_param, | ||
exporter_args_map=exporter_args_map, | ||
) | ||
_init_metrics( | ||
metric_exporters, resource, exporter_args_map=exporter_args_map | ||
metric_exporters, | ||
resource, | ||
otlp_credential_param=otlp_credential_param, | ||
exporter_args_map=exporter_args_map, | ||
) | ||
if setup_logging_handler is None: | ||
setup_logging_handler = ( | ||
|
@@ -468,6 +585,7 @@ def _initialize_components( | |
log_exporters, | ||
resource, | ||
setup_logging_handler, | ||
otlp_credential_param=otlp_credential_param, | ||
exporter_args_map=exporter_args_map, | ||
) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be an issue if the SDK is running in an environment without grpcio installed? I know the operator auto-instrumentation layer for Python pre-installs the HTTP exporters and not the grpc exporters to avoid extension issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's a good point ! I don't think we should require these modules (requests, grpc) for the SDK, so we have to use try/catch blocks on the imports.. I updated the PR.. Let me know what you think