Automatic exporter/provider setup for opentelemetry-instrument command. #1036

owais · 2020-08-23T23:16:31Z

This PR adds support to the opentelemetry-instrument command to automatically configure a tracer provider and exporter. By default configures the OTLP exporter (like other Otel auto-instrumentations. e.g, Java: https://github.com/open-telemetry/opentelemetry-java-instrumentation#getting-started). It also allows using a different in-built or 3rd party via a CLI argument or env variable.

Details can be found on opentelemetry-instrumentation's README package.

Fixes #663

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

Ships with automated tests
Tested with a demo app

Checklist:

Followed the style guidelines of this project
Changelogs have been updated
Unit tests have been added
Documentation has been updated

open-telemetry#1036 This commit adds support to the opentelemetry-instrument command to automatically configure a tracer provider and exporter. By default, it configures the OTLP exporter (like other Otel auto-instrumentations. e.g, Java: https://github.com/open-telemetry/opentelemetry-java-instrumentation#getting-started). It also allows using a different in-built or 3rd party via a CLI argument or env variable. Details can be found on opentelemetry-instrumentation's README package. Fixes open-telemetry#663

ocelotl

Requesting just a typo to be fixed 👍

opentelemetry-instrumentation/README.rst

ocelotl · 2020-11-11T22:22:06Z

...etry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py

+
+if (
+    hasattr(sys, "argv")
+    and sys.argv[0].split(os.path.sep)[-1] == "celery"


This PR has been open for a while, so in order to move forward I am ok with this as it is right now. Nevertheless, I think we should definitely keep the "core" instrumentation completely separated from the implementation details of any and all of the instrumentation packages. This probably means allowing all the instrumentation packages to be allowed to run a preregistered function or something similar that includes this kind of code.

ocelotl

LGTM, thanks @owais!

NathanielRN · 2020-11-18T17:22:58Z

Really excited for this change! I added a PR to this branch to add the ability to set a Custom IDs Generator, would be nice to get these configs all in at once! owais#1

NathanielRN · 2020-11-18T17:31:58Z

opentelemetry-instrumentation/CHANGELOG.md

+- Fixed boostrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))
+- Added support for `OTEL_EXPORTER` to the `opentelemetry-instrument` command ([#1036](https://github.com/open-telemetry/opentelemetry-python/pull/1036))


We should probably move the PR references to be on a new line to be consistent with the other Changelog messages.

NathanielRN · 2020-11-18T17:34:36Z

opentelemetry-instrumentation/README.rst

+of the well-known exporter names (see below) or a fully
+qualified Python import path to a span  exporter implementation.


Does this mean I can set exporter opentelemetry.exporter.jaeger:JaegerSpanExporter?

Sorry for my misunderstanding, but when I look at the code it looks like it will error out if it doesn't match ep.name (where ep is an entry point).

exporters = { ep.name: ep for ep in iter_entry_points("opentelemetry_exporter") } for exporter_name in exporter_names: entry_point = exporters.get(exporter_name, None) if not entry_point: raise RuntimeError( "Requested exporter not found: {0}".format(exporter_name) )

So doesn't it have to be otlp?

You're right. This PR supported what the documentation says but I ended up removing it in order to simplify the implementation. Looks like I missed this piece of documentation when I removed the feature.

NathanielRN · 2020-11-18T17:36:58Z

...etry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py

+            logger.debug("Instrumented %s", entry_point.name)
+
+        except Exception:  # pylint: disable=broad-except
+            logger.exception("Instrumenting of %s failed", entry_point.name)


Should this re-raise the Exception?

I'm not sure if this case should halt execution of the program and IIRC, any exceptions raised inside sitecustomize are "eaten up" by python by default and the process still continues to work.

I meant because you only call auto_instrument in line 41 below which is wrapped in another try except block.

So if you re raise it we will get the "Failed to auto initialize opentelemetry" log which will be true in a sense.

Ah, makes sense.

NathanielRN · 2020-11-18T17:38:20Z

...etry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py

+if (
+    hasattr(sys, "argv")
+    and sys.argv[0].split(os.path.sep)[-1] == "celery"
+    and "worker" in sys.argv


Can this be the following? Not sure if it's more performant or less...

Suggested change

and "worker" in sys.argv

and "worker" in sys.argv[1:]

Don't think it matter either way. [1:] probably clarifies the intent a tiny bit more so will make the change.

NathanielRN · 2020-11-18T17:48:29Z

...etry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py

+    from celery.signals import worker_process_init  # pylint:disable=E0401
+
+    @worker_process_init.connect(weak=False)
+    def init_celery(*args, **kwargs):
+        initialize()


This is a sweet catch, thanks for knowing to do something like this!

NathanielRN

Really excited for this change 👍

codeboten

This looks pretty good, just a minor typo to fix and a question about the change in docs.

codeboten · 2020-11-19T19:55:31Z

opentelemetry-instrumentation/CHANGELOG.md

 ## Version 0.14b0

 Released 2020-10-13

- Fixed boostrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask
+- Fixed boostrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))


Suggested change

- Fixed boostrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))

- Fixed bootstrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))

codeboten · 2020-11-19T19:56:16Z

opentelemetry-instrumentation/README.rst


 ::

-    opentelemetry-bootstrap --action=install|requirements
+    opentelemetry-instrument -e zipkin,otlp celery -A tasks worker --loglevel=info


can you explain the reason behind removing the info about opentelemetry-bootstrap?

+1, this info is probably important to keep here.

This is unintentional. Probably caused by a bad rebase. Fixing.

This commit extends the instrument command so it automatically configures tracing with a provider, span processor and exporter. Most of the component used can be customized with env vars or CLI arguments. Details can be found on opentelemetry-instrumentation's README package. Fixes open-telemetry#663

codeboten

Thanks for fixing

owais requested a review from a team August 23, 2020 23:16

owais force-pushed the otlp-env-config branch 2 times, most recently from 1568630 to 05a1fed Compare August 29, 2020 15:05

owais changed the title ~~POC: actual auto-instrumentation~~ Automatic exporter/provider setup for opentelemetry-instrument command. Aug 29, 2020

owais force-pushed the otlp-env-config branch 2 times, most recently from 53a5233 to 0ec2526 Compare August 29, 2020 15:10

owais force-pushed the otlp-env-config branch from 0ec2526 to 29000e8 Compare August 29, 2020 15:21

owais force-pushed the otlp-env-config branch from 29000e8 to 9b72eda Compare August 29, 2020 15:27

owais force-pushed the otlp-env-config branch from 9b72eda to 0e93d67 Compare August 29, 2020 15:29

owais force-pushed the otlp-env-config branch from 0e93d67 to 1fcd121 Compare August 29, 2020 15:41

owais force-pushed the otlp-env-config branch from 1fcd121 to e2e6ddf Compare August 29, 2020 16:16

owais force-pushed the otlp-env-config branch from e2e6ddf to 1a35dc3 Compare August 29, 2020 16:16

owais force-pushed the otlp-env-config branch from 1a35dc3 to 72369ff Compare August 29, 2020 16:20

owais force-pushed the otlp-env-config branch from 72369ff to 536e913 Compare August 29, 2020 22:40

owais force-pushed the otlp-env-config branch from 536e913 to e75a7e8 Compare August 29, 2020 23:01

owais force-pushed the otlp-env-config branch from e75a7e8 to a79536e Compare August 29, 2020 23:27

owais force-pushed the otlp-env-config branch from a79536e to a8ef8aa Compare August 29, 2020 23:32

owais force-pushed the otlp-env-config branch from a8ef8aa to 4c17c20 Compare August 29, 2020 23:41

owais force-pushed the otlp-env-config branch 4 times, most recently from 762fa1b to 9162fff Compare November 10, 2020 13:49

ocelotl requested changes Nov 11, 2020

View reviewed changes

ocelotl approved these changes Nov 13, 2020

View reviewed changes

owais force-pushed the otlp-env-config branch 2 times, most recently from 2482d9f to 0331351 Compare November 18, 2020 17:45

NathanielRN reviewed Nov 18, 2020

View reviewed changes

owais force-pushed the otlp-env-config branch 2 times, most recently from 6513835 to c43f3c5 Compare November 18, 2020 17:56

NathanielRN approved these changes Nov 18, 2020

View reviewed changes

owais force-pushed the otlp-env-config branch from c43f3c5 to c3192c1 Compare November 18, 2020 18:09

codeboten reviewed Nov 19, 2020

View reviewed changes

owais force-pushed the otlp-env-config branch from a58e650 to e920dc5 Compare November 20, 2020 11:54

owais force-pushed the otlp-env-config branch from e920dc5 to 6024755 Compare November 20, 2020 11:56

codeboten approved these changes Nov 20, 2020

View reviewed changes

codeboten merged commit 8660dea into open-telemetry:master Nov 20, 2020

This was referenced Nov 20, 2020

Add IDs Generator as Configurable Property of Auto Instrumentation #1404

Merged

Instrumentation Package depends on the OTel SDK #1405

Merged

codeboten mentioned this pull request Nov 26, 2020

Remove SDK dependency from instrumentation package #1421

Closed

toumorokoshi mentioned this pull request Nov 30, 2020

Documentation of default exporters open-telemetry/opentelemetry.io#4348

Open

owais mentioned this pull request Dec 3, 2020

Extend exporter interface to include initializer (__init__) or introduce a new exporter factory interface #1442

Closed

NathanielRN mentioned this pull request May 18, 2022

Auto Instrumentation Needs Support for Initializing Metrics open-telemetry/opentelemetry-python-contrib#1096

Closed

adamantike mentioned this pull request Mar 27, 2021

How to use auto-instrumentation for Uvicorn with multiple worker processes open-telemetry/opentelemetry-python-contrib#385

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic exporter/provider setup for opentelemetry-instrument command. #1036

Automatic exporter/provider setup for opentelemetry-instrument command. #1036

owais commented Aug 23, 2020 •

edited

Loading

ocelotl left a comment

ocelotl Nov 11, 2020

ocelotl left a comment

NathanielRN commented Nov 18, 2020

NathanielRN Nov 18, 2020

NathanielRN Nov 18, 2020

owais Nov 18, 2020

NathanielRN Nov 18, 2020

owais Nov 18, 2020 •

edited

Loading

NathanielRN Nov 18, 2020

owais Nov 18, 2020

owais Nov 18, 2020

NathanielRN Nov 18, 2020

owais Nov 18, 2020

NathanielRN Nov 18, 2020

NathanielRN left a comment

codeboten left a comment

codeboten Nov 19, 2020

codeboten Nov 19, 2020

NathanielRN Nov 19, 2020

owais Nov 20, 2020

codeboten left a comment

		- Fixed boostrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))
		- Added support for `OTEL_EXPORTER` to the `opentelemetry-instrument` command ([#1036](https://github.com/open-telemetry/opentelemetry-python/pull/1036))

		of the well-known exporter names (see below) or a fully
		qualified Python import path to a span exporter implementation.

	- Fixed boostrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))
	- Fixed bootstrap command to correctly install opentelemetry-instrumentation-falcon instead of opentelemetry-instrumentation-flask. ([#1138](https://github.com/open-telemetry/opentelemetry-python/pull/1138))

Automatic exporter/provider setup for opentelemetry-instrument command. #1036

Automatic exporter/provider setup for opentelemetry-instrument command. #1036

Conversation

owais commented Aug 23, 2020 • edited Loading

Type of change

How Has This Been Tested?

Checklist:

ocelotl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocelotl left a comment

Choose a reason for hiding this comment

NathanielRN commented Nov 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owais Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NathanielRN left a comment

Choose a reason for hiding this comment

codeboten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeboten left a comment

Choose a reason for hiding this comment

owais commented Aug 23, 2020 •

edited

Loading

owais Nov 18, 2020 •

edited

Loading