From c67157d48c1eeb079f85c774d45bc62d3aab2c63 Mon Sep 17 00:00:00 2001 From: Nathaniel Ruiz Nowell Date: Tue, 17 Nov 2020 19:18:44 -0800 Subject: [PATCH 1/4] Add IDs Generator to auto instrumentation config --- opentelemetry-api/setup.cfg | 2 ++ opentelemetry-instrumentation/CHANGELOG.md | 2 ++ opentelemetry-instrumentation/README.rst | 12 +++++++ .../auto_instrumentation/__init__.py | 15 ++++++++ .../auto_instrumentation/components.py | 34 +++++++++++++++++-- .../tests/test_auto_tracing.py | 30 ++++++++++++++-- 6 files changed, 90 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/setup.cfg b/opentelemetry-api/setup.cfg index c697e308546..78c3123222a 100644 --- a/opentelemetry-api/setup.cfg +++ b/opentelemetry-api/setup.cfg @@ -57,6 +57,8 @@ opentelemetry_tracer_provider = opentelemetry_propagator = tracecontext = opentelemetry.trace.propagation.tracecontext:TraceContextTextMapPropagator baggage = opentelemetry.baggage.propagation:BaggagePropagator +opentelemetry_ids_generator = + random = opentelemetry.trace.ids_generator:RandomIdsGenerator [options.extras_require] test = diff --git a/opentelemetry-instrumentation/CHANGELOG.md b/opentelemetry-instrumentation/CHANGELOG.md index d59bc01c756..34366cb6b14 100644 --- a/opentelemetry-instrumentation/CHANGELOG.md +++ b/opentelemetry-instrumentation/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add IDs Generator as Configurable Property of Auto Instrumentation + ([#1404](https://github.com/open-telemetry/opentelemetry-python/pull/1404)) - Added support for `OTEL_EXPORTER` to the `opentelemetry-instrument` command ([#1036](https://github.com/open-telemetry/opentelemetry-python/pull/1036)) ## Version 0.14b0 diff --git a/opentelemetry-instrumentation/README.rst b/opentelemetry-instrumentation/README.rst index 22f22280166..89f346581bb 100644 --- a/opentelemetry-instrumentation/README.rst +++ b/opentelemetry-instrumentation/README.rst @@ -73,6 +73,11 @@ Well known trace exporter names: When present the value is passed on to the relevant exporter initializer as ``service_name`` argument. +* ``--ids-generator`` or ``OTEL_IDS_GENERATOR`` + +Used to specify which IDs Generator to use for the global Tracer Provider. By default, it +will use the random IDs generator. + The code in ``program.py`` needs to use one of the packages for which there is an OpenTelemetry integration. For a list of the available integrations please check `here `_ @@ -93,6 +98,13 @@ The above command will pass ``-e otlp`` to the instrument command and ``--port=3 The above command will configure global trace provider, attach zipkin and otlp exporters to it and then start celery with the rest of the arguments. +:: + + opentelemetry-instrument -g random flask run --port=3000 + +The above command will configure the global trace provider to use the Random IDs Generator, and then +pass ``--port=3000`` to ``flask run``. + References ---------- diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py index 51af6dffd80..8938d8de47f 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py @@ -47,6 +47,19 @@ def parse_args(): """, ) + parser.add_argument( + "-g", + "--ids-generator", + required=False, + help=""" + The IDs Generator to be used with the Tracer Provider. + + Examples: + + -g=random + """, + ) + parser.add_argument( "-s", "--service-name", @@ -70,6 +83,8 @@ def load_config_from_cli_args(args): environ["OTEL_EXPORTER"] = args.exporter if args.service_name: environ["OTEL_SERVICE_NAME"] = args.service_name + if args.ids_generator: + environ["OTEL_IDS_GENERATOR"] = args.ids_generator def run() -> None: diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py index 105a41722c0..4fdb3c8ee13 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py @@ -34,6 +34,13 @@ EXPORTER_OTLP_METRIC = "otlp_metric" _DEFAULT_EXPORTER = EXPORTER_OTLP +RANDOM_IDS_GENERATOR = "random" +_DEFAULT_IDS_GENERATOR = RANDOM_IDS_GENERATOR + + +def get_ids_generator() -> str: + return Configuration().IDS_GENERATOR or _DEFAULT_IDS_GENERATOR + def get_service_name() -> str: return Configuration().SERVICE_NAME or "" @@ -55,10 +62,13 @@ def get_exporter_names() -> Sequence[str]: return names -def init_tracing(exporters: Sequence[SpanExporter]): +def init_tracing( + exporters: Sequence[SpanExporter], ids_generator: trace.IdsGenerator +): service_name = get_service_name() provider = TracerProvider( resource=Resource.create({"service.name": service_name}), + ids_generator=ids_generator(), ) trace.set_tracer_provider(provider) @@ -110,10 +120,30 @@ def import_exporters( return trace_exporters, metric_exporters +def import_ids_generator(ids_generator_name: str) -> trace.IdsGenerator: + ids_generator_impl = None + + for id_generator in iter_entry_points("opentelemetry_ids_generator"): + if id_generator.name == ids_generator_name.strip(): + ids_generator_impl = id_generator.load() + break + else: + raise RuntimeError( + "Requested IDs Generator not found: {0}".format(ids_generator_name) + ) + + if issubclass(ids_generator_impl, trace.IdsGenerator): + return ids_generator_impl + + raise RuntimeError("{0} is not an IdsGenerator".format(ids_generator_name)) + + def initialize_components(): exporter_names = get_exporter_names() trace_exporters, metric_exporters = import_exporters(exporter_names) - init_tracing(trace_exporters) + ids_generator_name = get_ids_generator() + ids_generator = import_ids_generator(ids_generator_name) + init_tracing(trace_exporters, ids_generator) # We don't support automatic initialization for metric yet but have added # some boilerplate in order to make sure current implementation does not diff --git a/opentelemetry-instrumentation/tests/test_auto_tracing.py b/opentelemetry-instrumentation/tests/test_auto_tracing.py index 359dc12fad7..9d1e833a7f2 100644 --- a/opentelemetry-instrumentation/tests/test_auto_tracing.py +++ b/opentelemetry-instrumentation/tests/test_auto_tracing.py @@ -20,10 +20,12 @@ from opentelemetry.configuration import Configuration from opentelemetry.instrumentation.auto_instrumentation import components from opentelemetry.sdk.resources import Resource +from opentelemetry.trace.ids_generator import RandomIdsGenerator class Provider: - def __init__(self, resource=None): + def __init__(self, resource=None, ids_generator=None): + self.ids_generator = ids_generator self.processor = None self.resource = resource @@ -77,11 +79,12 @@ def tearDown(self): def test_trace_init_default(self): environ["OTEL_SERVICE_NAME"] = "my-test-service" Configuration._reset() - components.init_tracing({"zipkin": Exporter}) + components.init_tracing({"zipkin": Exporter}, RandomIdsGenerator) self.assertEqual(self.set_provider_mock.call_count, 1) provider = self.set_provider_mock.call_args[0][0] self.assertIsInstance(provider, Provider) + self.assertIsInstance(provider.ids_generator, RandomIdsGenerator) self.assertIsInstance(provider.processor, Processor) self.assertIsInstance(provider.processor.exporter, Exporter) self.assertEqual( @@ -91,11 +94,12 @@ def test_trace_init_default(self): def test_trace_init_otlp(self): environ["OTEL_SERVICE_NAME"] = "my-otlp-test-service" Configuration._reset() - components.init_tracing({"otlp": OTLPExporter}) + components.init_tracing({"otlp": OTLPExporter}, RandomIdsGenerator) self.assertEqual(self.set_provider_mock.call_count, 1) provider = self.set_provider_mock.call_args[0][0] self.assertIsInstance(provider, Provider) + self.assertIsInstance(provider.ids_generator, RandomIdsGenerator) self.assertIsInstance(provider.processor, Processor) self.assertIsInstance(provider.processor.exporter, OTLPExporter) self.assertIsInstance(provider.resource, Resource) @@ -104,3 +108,23 @@ def test_trace_init_otlp(self): "my-otlp-test-service", ) del environ["OTEL_SERVICE_NAME"] + + def test_trace_init_default_random_ids_generator(self): + environ["OTEL_SERVICE_NAME"] = "my-random-ids-generator-test-service" + Configuration._reset() + ids_generator_name = components.get_ids_generator() + ids_generator = components.import_ids_generator(ids_generator_name) + components.init_tracing({"otlp": OTLPExporter}, ids_generator) + + self.assertEqual(self.set_provider_mock.call_count, 1) + provider = self.set_provider_mock.call_args[0][0] + self.assertIsInstance(provider, Provider) + self.assertIsInstance(provider.ids_generator, RandomIdsGenerator) + self.assertIsInstance(provider.processor, Processor) + self.assertIsInstance(provider.processor.exporter, OTLPExporter) + self.assertIsInstance(provider.resource, Resource) + self.assertEqual( + provider.resource.attributes.get("service.name"), + "my-random-ids-generator-test-service", + ) + del environ["OTEL_SERVICE_NAME"] From b0f3a5438fe0426b248e12209a6d48947134fa3b Mon Sep 17 00:00:00 2001 From: Nathaniel Ruiz Nowell Date: Fri, 20 Nov 2020 13:45:38 -0800 Subject: [PATCH 2/4] Patch dict so delete not needed --- opentelemetry-instrumentation/tests/test_auto_tracing.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/opentelemetry-instrumentation/tests/test_auto_tracing.py b/opentelemetry-instrumentation/tests/test_auto_tracing.py index 9d1e833a7f2..cedbff1ead9 100644 --- a/opentelemetry-instrumentation/tests/test_auto_tracing.py +++ b/opentelemetry-instrumentation/tests/test_auto_tracing.py @@ -109,8 +109,10 @@ def test_trace_init_otlp(self): ) del environ["OTEL_SERVICE_NAME"] + @patch.dict( + environ, {"OTEL_SERVICE_NAME": "my-random-ids-generator-test-service"} + ) def test_trace_init_default_random_ids_generator(self): - environ["OTEL_SERVICE_NAME"] = "my-random-ids-generator-test-service" Configuration._reset() ids_generator_name = components.get_ids_generator() ids_generator = components.import_ids_generator(ids_generator_name) @@ -127,4 +129,3 @@ def test_trace_init_default_random_ids_generator(self): provider.resource.attributes.get("service.name"), "my-random-ids-generator-test-service", ) - del environ["OTEL_SERVICE_NAME"] From bd7182525ff0ae00dceca2b1026d3e762329cecc Mon Sep 17 00:00:00 2001 From: Nathaniel Ruiz Nowell Date: Sat, 21 Nov 2020 01:31:43 -0800 Subject: [PATCH 3/4] Test ids generator env var and change flag --- opentelemetry-instrumentation/README.rst | 2 +- .../auto_instrumentation/__init__.py | 3 +- .../tests/test_auto_tracing.py | 47 +++++++++++++------ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/opentelemetry-instrumentation/README.rst b/opentelemetry-instrumentation/README.rst index 89f346581bb..aed9909a041 100644 --- a/opentelemetry-instrumentation/README.rst +++ b/opentelemetry-instrumentation/README.rst @@ -100,7 +100,7 @@ start celery with the rest of the arguments. :: - opentelemetry-instrument -g random flask run --port=3000 + opentelemetry-instrument --ids-generator random flask run --port=3000 The above command will configure the global trace provider to use the Random IDs Generator, and then pass ``--port=3000`` to ``flask run``. diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py index 8938d8de47f..959708de964 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py @@ -48,7 +48,6 @@ def parse_args(): ) parser.add_argument( - "-g", "--ids-generator", required=False, help=""" @@ -56,7 +55,7 @@ def parse_args(): Examples: - -g=random + --ids-generator=random """, ) diff --git a/opentelemetry-instrumentation/tests/test_auto_tracing.py b/opentelemetry-instrumentation/tests/test_auto_tracing.py index cedbff1ead9..9ffe421a25d 100644 --- a/opentelemetry-instrumentation/tests/test_auto_tracing.py +++ b/opentelemetry-instrumentation/tests/test_auto_tracing.py @@ -50,6 +50,23 @@ class OTLPExporter: pass +class IdsGenerator: + pass + + +class CustomIdsGenerator(IdsGenerator): + pass + + +class IterEntryPoint: + def __init__(self, name, class_type): + self.name = name + self.class_type = class_type + + def load(self): + return self.class_type + + class TestTraceInit(TestCase): def setUp(self): super() @@ -109,23 +126,23 @@ def test_trace_init_otlp(self): ) del environ["OTEL_SERVICE_NAME"] - @patch.dict( - environ, {"OTEL_SERVICE_NAME": "my-random-ids-generator-test-service"} + @patch.dict(environ, {"OTEL_IDS_GENERATOR": "custom_ids_generator"}) + @patch( + "opentelemetry.instrumentation.auto_instrumentation.components.trace.IdsGenerator", + new=IdsGenerator, ) - def test_trace_init_default_random_ids_generator(self): + @patch( + "opentelemetry.instrumentation.auto_instrumentation.components.iter_entry_points" + ) + def test_trace_init_custom_ids_generator(self, mock_iter_entry_points): + mock_iter_entry_points.configure_mock( + return_value=[ + IterEntryPoint("custom_ids_generator", CustomIdsGenerator) + ] + ) Configuration._reset() ids_generator_name = components.get_ids_generator() ids_generator = components.import_ids_generator(ids_generator_name) - components.init_tracing({"otlp": OTLPExporter}, ids_generator) - - self.assertEqual(self.set_provider_mock.call_count, 1) + components.init_tracing({}, ids_generator) provider = self.set_provider_mock.call_args[0][0] - self.assertIsInstance(provider, Provider) - self.assertIsInstance(provider.ids_generator, RandomIdsGenerator) - self.assertIsInstance(provider.processor, Processor) - self.assertIsInstance(provider.processor.exporter, OTLPExporter) - self.assertIsInstance(provider.resource, Resource) - self.assertEqual( - provider.resource.attributes.get("service.name"), - "my-random-ids-generator-test-service", - ) + self.assertIsInstance(provider.ids_generator, CustomIdsGenerator) From 2c60b8930cb0bcd20383834a077b077bb30b210f Mon Sep 17 00:00:00 2001 From: Nathaniel Ruiz Nowell Date: Sat, 21 Nov 2020 02:04:46 -0800 Subject: [PATCH 4/4] Generalize exports and IDs generator code --- .../auto_instrumentation/components.py | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py index 4fdb3c8ee13..8b4ef4f3e8b 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py @@ -90,23 +90,39 @@ def init_metrics(exporters: Sequence[MetricsExporter]): logger.warning("automatic metric initialization is not supported yet.") -def import_exporters( - exporter_names: Sequence[str], -) -> Tuple[Sequence[SpanExporter], Sequence[MetricsExporter]]: - trace_exporters, metric_exporters = {}, {} - - exporters = { - ep.name: ep for ep in iter_entry_points("opentelemetry_exporter") +def import_tracer_provider_config_components( + selected_components, entry_point_name +) -> Sequence[Tuple[str, object]]: + component_entry_points = { + ep.name: ep for ep in iter_entry_points(entry_point_name) } - - for exporter_name in exporter_names: - entry_point = exporters.get(exporter_name, None) + component_impls = [] + for selected_component in selected_components: + entry_point = component_entry_points.get(selected_component, None) if not entry_point: raise RuntimeError( - "Requested exporter not found: {0}".format(exporter_name) + "Requested component '{}' not found in entry points for '{}'".format( + selected_component, entry_point_name + ) ) - exporter_impl = entry_point.load() + component_impl = entry_point.load() + component_impls.append((selected_component, component_impl)) + + return component_impls + + +def import_exporters( + exporter_names: Sequence[str], +) -> Tuple[Sequence[SpanExporter], Sequence[MetricsExporter]]: + trace_exporters, metric_exporters = {}, {} + + for ( + exporter_name, + exporter_impl, + ) in import_tracer_provider_config_components( + exporter_names, "opentelemetry_exporter" + ): if issubclass(exporter_impl, SpanExporter): trace_exporters[exporter_name] = exporter_impl elif issubclass(exporter_impl, MetricsExporter): @@ -121,16 +137,12 @@ def import_exporters( def import_ids_generator(ids_generator_name: str) -> trace.IdsGenerator: - ids_generator_impl = None - - for id_generator in iter_entry_points("opentelemetry_ids_generator"): - if id_generator.name == ids_generator_name.strip(): - ids_generator_impl = id_generator.load() - break - else: - raise RuntimeError( - "Requested IDs Generator not found: {0}".format(ids_generator_name) - ) + # pylint: disable=unbalanced-tuple-unpacking + [ + (ids_generator_name, ids_generator_impl) + ] = import_tracer_provider_config_components( + [ids_generator_name.strip()], "opentelemetry_ids_generator" + ) if issubclass(ids_generator_impl, trace.IdsGenerator): return ids_generator_impl