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

NH-12018 Separate Distro vs Configurator #14

Merged
merged 5 commits into from
May 4, 2022
Merged

NH-12018 Separate Distro vs Configurator #14

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
7b75e65
Separate concerns: env vars in distro, component init in configurator
tammy-baylis-swi Apr 22, 2022
2842209
NH-2313 adjust CompositePropagator config with OTEL_PROPAGATORS
tammy-baylis-swi Apr 28, 2022
7ffb1a9
solarwinds_sampler cannot be env default because not in OTel _KNOWN_S…
tammy-baylis-swi Apr 28, 2022
02bde11
Merge pull request #15 from appoptics/NH-2313-adjust-propagator-confi…
tammy-baylis-swi May 2, 2022
1241e7a
load_entry_point instead of iter for singulars
tammy-baylis-swi May 3, 2022
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
86 changes: 86 additions & 0 deletions opentelemetry_distro_solarwinds/configurator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
"""Module to initialize OpenTelemetry SDK components to work with SolarWinds backend"""

import logging
from os import environ
from pkg_resources import (
iter_entry_points,
load_entry_point
)

from opentelemetry import trace
from opentelemetry.environment_variables import (
OTEL_PROPAGATORS,
OTEL_TRACES_EXPORTER
)
from opentelemetry.instrumentation.propagators import set_global_response_propagator
from opentelemetry.propagate import set_global_textmap
from opentelemetry.propagators.composite import CompositePropagator
from opentelemetry.sdk._configuration import _OTelSDKConfigurator
from opentelemetry.sdk.environment_variables import OTEL_TRACES_SAMPLER
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor

from opentelemetry_distro_solarwinds.response_propagator import SolarWindsTraceResponsePropagator

logger = logging.getLogger(__name__)

class SolarWindsConfigurator(_OTelSDKConfigurator):
"""OpenTelemetry Configurator for initializing SolarWinds-reporting SDK components"""

# Cannot set as env default because not part of OTel Python _KNOWN_SAMPLERS
# https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py#L364-L380
_DEFAULT_SW_TRACES_SAMPLER = "solarwinds_sampler"

def _configure(self, **kwargs):
environ_sampler = environ.get(
OTEL_TRACES_SAMPLER,
self._DEFAULT_SW_TRACES_SAMPLER,
)
try:
sampler = load_entry_point(
"opentelemetry_distro_solarwinds",
"opentelemetry_traces_sampler",
environ_sampler
)()
except:
logger.exception(
"Failed to load configured sampler `%s`", environ_sampler
)
raise
trace.set_tracer_provider(
TracerProvider(sampler=sampler))

environ_exporter = environ.get(OTEL_TRACES_EXPORTER)
try:
exporter = load_entry_point(
"opentelemetry_distro_solarwinds",
"opentelemetry_traces_exporter",
environ_exporter
)()
except:
logger.exception(
"Failed to load configured exporter `%s`", environ_exporter
)
raise
span_exporter = BatchSpanProcessor(exporter)
Copy link
Contributor

@cheempz cheempz May 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For the exporter and sampler above, what happens if there are multiple configured -- e..g if there were somehow two exporters configured, it seems like only the first result from next would be taken and be the one set into the BatchSpanProcesser (and similarly with sampler and TracerProvider).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think you're right. The customer could end up somehow configuring multiples of the same name (theirs or SW exporter/sampler). Without going too far down the rabbit hole, I think the chances of this are small but non-zero if the customer is doing some funny business with their service's sys.path and/or the effective pkg_resources WorkingSet.

In 1241e7a I've switched the two iter_entry_points to load_entry_point calls, to avoid that unintended behaviour.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

trace.get_tracer_provider().add_span_processor(span_exporter)

# Init and set CompositePropagator globally, like OTel API
Copy link

Choose a reason for hiding this comment

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

I'm curious about the propagators configuration here (I may not have fully read/understood your PR description or the Otel agent's design): I don't see a new propagator written by us here, so why can't the Otel agent do the configuration by itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, I think I should document this all in one place instead of one PR description and one Jira comment 😅

Our new SW propagator is either named by default or must be named by OTEL_PROPAGATORS set by customer. This is implemented/policed in our SW Distro class: https://github.com/appoptics/opentelemetry-python-instrumentation-custom-distro/blob/1241e7a149facd9f8b84f274e8d3e760ffb638e5/opentelemetry_distro_solarwinds/distro.py#L30-L46

When a customer's service has NH Python installed and is launched with auto-instrumentation, this OTel Python sitecustomize.py is run. It first inits Distro class and runs distro.configure() to set up environment variables, which must include our SW propagator as above. Then it loads the configurator, which is this SW configurator. So the new propagator is included for configuration. https://github.com/open-telemetry/opentelemetry-python-contrib/blob/9e0c2ce66202e721f99a3d70389f109ec179c9a5/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py#L121-L124

Does that make sense and justify this code block? 🙂 Or more clarification required?

Copy link

Choose a reason for hiding this comment

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

Sounds good to me 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! 😺

I've documented this for future reference here: https://swicloud.atlassian.net/wiki/spaces/NIT/pages/2867726621/NH+Python+Troubleshooting#%E2%80%9CHow-does-NH-Python-code-get-run%3F%E2%80%9D

environ_propagators = environ.get(OTEL_PROPAGATORS).split(",")
propagators = []
for propagator in environ_propagators:
try:
propagators.append(
next(
iter_entry_points("opentelemetry_propagator", propagator)
).load()()
)
except Exception:
logger.exception(
"Failed to load configured propagator `%s`", propagator
)
raise
set_global_textmap(CompositePropagator(propagators))

# Set global HTTP response propagator
set_global_response_propagator(SolarWindsTraceResponsePropagator())
101 changes: 33 additions & 68 deletions opentelemetry_distro_solarwinds/distro.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,87 +1,52 @@
"""Module to configure OpenTelemetry agent to work with SolarWinds backend"""
"""Module to configure OpenTelemetry to work with SolarWinds backend"""

import logging
from os import environ
from pkg_resources import iter_entry_points

from opentelemetry import trace
from opentelemetry.environment_variables import OTEL_PROPAGATORS, OTEL_TRACES_EXPORTER
from opentelemetry.environment_variables import (
OTEL_PROPAGATORS,
OTEL_TRACES_EXPORTER
)
from opentelemetry.instrumentation.distro import BaseDistro
from opentelemetry.instrumentation.propagators import set_global_response_propagator
from opentelemetry.propagate import set_global_textmap
from opentelemetry.propagators.composite import CompositePropagator
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor

from opentelemetry_distro_solarwinds.response_propagator import SolarWindsTraceResponsePropagator
from opentelemetry_distro_solarwinds.sampler import ParentBasedSwSampler
from opentelemetry.sdk.environment_variables import OTEL_TRACES_SAMPLER

logger = logging.getLogger(__name__)

class SolarWindsDistro(BaseDistro):
"""SolarWinds custom distro for OpenTelemetry agents."""
"""OpenTelemetry Distro for SolarWinds reporting environment"""

_DEFAULT_OTEL_EXPORTER = "solarwinds_exporter"
_DEFAULT_OTEL_PROPAGATORS = [
"tracecontext",
_TRACECONTEXT_PROPAGATOR = "tracecontext"
_SW_PROPAGATOR = "solarwinds_propagator"
_DEFAULT_SW_PROPAGATORS = [
_TRACECONTEXT_PROPAGATOR,
"baggage",
"solarwinds_propagator",
_SW_PROPAGATOR,
]
_DEFAULT_SW_TRACES_EXPORTER = "solarwinds_exporter"

def _configure(self, **kwargs):
# Automatically use custom SolarWinds sampler
trace.set_tracer_provider(
TracerProvider(sampler=ParentBasedSwSampler()))

# Customize Exporter else default to SolarWindsSpanExporter
environ_exporter = environ.get(
OTEL_TRACES_EXPORTER,
self._DEFAULT_OTEL_EXPORTER
)
try:
exporter = next(
iter_entry_points(
"opentelemetry_traces_exporter",
environ_exporter
)).load()()
except:
logger.exception(
"Failed to load configured exporter `%s`", environ_exporter
)
raise

span_exporter = BatchSpanProcessor(exporter)
trace.get_tracer_provider().add_span_processor(span_exporter)

# Configure context propagators to always include
# tracecontext,baggage,solarwinds -- first and in that order
# -- plus any others specified by env var
environ.setdefault(OTEL_TRACES_EXPORTER, self._DEFAULT_SW_TRACES_EXPORTER)

environ_propagators = environ.get(
OTEL_PROPAGATORS,
",".join(self._DEFAULT_OTEL_PROPAGATORS)
",".join(self._DEFAULT_SW_PROPAGATORS)
).split(",")
if environ_propagators != self._DEFAULT_OTEL_PROPAGATORS:
for default in self._DEFAULT_OTEL_PROPAGATORS:
while default in environ_propagators:
environ_propagators.remove(default)
environ_propagators = self._DEFAULT_OTEL_PROPAGATORS + environ_propagators
# If not using the default propagators,
# can any arbitrary list BUT
# (1) must include tracecontext and solarwinds_propagator
# (2) tracecontext must be before solarwinds_propagator
if environ_propagators != self._DEFAULT_SW_PROPAGATORS:
if not self._TRACECONTEXT_PROPAGATOR in environ_propagators or \
not self._SW_PROPAGATOR in environ_propagators:
raise ValueError("Must include tracecontext and solarwinds_propagator in OTEL_PROPAGATORS to use SolarWinds Observability.")

if environ_propagators.index(self._SW_PROPAGATOR) \
< environ_propagators.index(self._TRACECONTEXT_PROPAGATOR):
raise ValueError("tracecontext must be before solarwinds_propagator in OTEL_PROPAGATORS to use SolarWinds Observability.")
environ[OTEL_PROPAGATORS] = ",".join(environ_propagators)

# Init and set CompositePropagator globally, like OTel API
propagators = []
for propagator in environ_propagators:
try:
propagators.append(
next(
iter_entry_points("opentelemetry_propagator", propagator)
).load()()
)
except Exception:
logger.exception(
"Failed to load configured propagator `%s`", propagator
)
raise
set_global_textmap(CompositePropagator(propagators))

# Set global HTTP response propagator
set_global_response_propagator(SolarWindsTraceResponsePropagator())
logger.debug("Configured SolarWindsDistro: {}, {}, {}".format(
environ.get(OTEL_TRACES_SAMPLER),
environ.get(OTEL_TRACES_EXPORTER),
environ.get(OTEL_PROPAGATORS)
))
6 changes: 5 additions & 1 deletion setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ install_requires =
[options.entry_points]
opentelemetry_distro =
solarwinds_distro = opentelemetry_distro_solarwinds.distro:SolarWindsDistro
opentelemetry_configurator =
solarwinds_configurator = opentelemetry_distro_solarwinds.configurator:SolarWindsConfigurator
opentelemetry_propagator =
solarwinds_propagator = opentelemetry_distro_solarwinds.propagator:SolarWindsPropagator
opentelemetry_traces_exporter =
solarwinds_exporter = opentelemetry_distro_solarwinds.exporter:SolarWindsSpanExporter
solarwinds_exporter = opentelemetry_distro_solarwinds.exporter:SolarWindsSpanExporter
opentelemetry_traces_sampler =
solarwinds_sampler = opentelemetry_distro_solarwinds.sampler:ParentBasedSwSampler