search

open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
695 stars 575 forks source link

opentelemetry-bootstrap too many false positives #1595

Open dimaqq opened 1 year ago

dimaqq commented 1 year ago

Here's what I get when I run bootstrap:

> opentelemetry-bootstrap -a requirements
opentelemetry-instrumentation-aws-lambda==0.36b0
opentelemetry-instrumentation-dbapi==0.36b0
opentelemetry-instrumentation-logging==0.36b0
opentelemetry-instrumentation-sqlite3==0.36b0
opentelemetry-instrumentation-urllib==0.36b0
opentelemetry-instrumentation-wsgi==0.36b0
opentelemetry-instrumentation-asgi==0.36b0
opentelemetry-instrumentation-fastapi==0.36b0
opentelemetry-instrumentation-tortoiseorm==0.36b0
opentelemetry-instrumentation-urllib3==0.36b0

So, in my view, this tool installs 10 things, 8 of which are most likely useless, and 2 seem OK, at least on the surface -- logging and fastapi.

srikanthccv commented 1 year ago

This script works by discovering the installed packages for which we have instrumentations. The default instrumentations maintained by the opentelemetry-bootstrap contains all the instrumentation libraries that work without any third party library since the standard python ships with them. https://github.com/open-telemetry/opentelemetry-python-contrib/blob/e1a1bada1f717d739b17e8734da169ca3dcbc025/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py#L168-L175

why is sqlite3 here? it's not used in my project at all

It's not intelligent enough to check if you have used it. You are welcome to enhance the current implementation to support this.

what is tortoiseorm? does this tool drop every possibly instrumentation in just in case?

Nothing drops anything. This looks like a bug, as the presence of pydantic in the env is installing the tortoiseorm.

Sentry which has urllib3 dep, but why urllib (without 3)? and if that's the reason, perhaps instrumenting another instrumentation is hardly a good idea, right?

Since urllib is part of stdlib and many users find it useful.

I don't think I'm using dbapi either why is wsgi here? I'm not using that

Same. Feel free to send a patch if this is something you want to get addressed.

why is there both fastapi and asgi? are they complementary or is one redundant?

The FastAPI (any ASGI framework) implementation builds on ASGI instrumentation.

tl;dr

This is not an intelligent tool (yet). Its logic is very simple. It inspects the env and tries to install the available instrumentations and some of them are by default because their deps are part of Python stdlib. You are welcome to make it better by sending a patch.

aabmass commented 5 months ago

Related to opentelemetry-instrumentation-aws-lambda being in the defaults https://github.com/open-telemetry/opentelemetry-python-contrib/issues/779

Ofc we could also just manually maintain the list of default instrumentations rather than trying to make our tooling smarter