Open jennydaman opened 1 year ago
@jennydaman I'd like to isolate either the operator or the python autoinstrumentation as the culprit.
Is it possible for you to try adding the python auto-instrumentation yourself instead of depending on the operator?
I’ll try doing a smaller reproduction case with a minimal Django project tomorrow.
https://github.com/jennydaman/otel_django_2302
@TylerHelmuth please check out the repo above. I've isolated the culprit to be python autoinstrumentation, not the Instrumentation operator.
Interestingly, my minimal reproduction case is buggier than my actual application. I can't get both imports and opentelemetry working.
pinging @open-telemetry/python-maintainers since I don't have permission to transfer the issue.
thanks for reporting @TylerHelmuth, will ask for this issue to be transferred to us.
I looked into this issue, found these 2 PRs that seem related:
https://github.com/open-telemetry/opentelemetry-python/pull/1583 https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1066
- Prepend the container's working directory to the value of
PYTHONPATH
added by Instrumentation- Document Python packaging best practices and require that Python applications adhere to them when autoinstrumentation is used
@jennydaman Which are these best practices you mention?
@ocelotl various sources (of various official-ness) suggest that the best practice is for Python packages to be "pip installed" (or pip install -e
) and that absolute imports are preferred.
Absolute imports are recommended, as they are usually more readable and tend to be better behaved (or at least give better error messages) if the import system is incorrectly configured (such as when a directory inside a package ends up on sys.path):
https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/
This is relevant since the Python interpreter includes the current working directory as the first item on the import path. This means that if an import package exists in the current working directory with the same name as an installed import package, the variant from the current working directory will be used. This can lead to subtle misconfiguration of the project’s packaging tooling, which could result in files not being included in a distribution.
In other words, the practice of dropping files in CWD and hoping it'll work is allowed but generally discouraged.
Component(s)
instrumentation
What happened?
Description
Python's import system is full of implicit behavior. Notably, modules can be imported from the current working directory. The best practice is for applications to be
pip install
-ed so that the import logic is more predictable and reliable. Nevertheless, it is valid (albeit discouraged) for a containerized Python application to work by importing modules from the current working directory.Python imports are dynamic and procedural, and some applications/frameworks such as Django are sensitive to the order of module resolution. Thus, setting
PYTHONPATH
can break valid Python programs which would otherwise work with unsetPYTHONPATH
.Steps to Reproduce
I am trying to run the
chris
chart from https://github.com/FNNDSC/charts. This is a Python application using celery, django, and gunicorn. It works (tested in GitHub Actions) without autoinstrumentation. However, when the annotationinstrumentation.opentelemetry.io/inject-python: "true"
is added to the pod, I get seemingly unrelated error messages about the Django configuration.For debugging, I edited the
command
of my containers tosleep 100000
and then tried running the actual command interactively usingkubectl exec ...
Expected Result
I expect the following commands to do nothing successfully:
Actual Result
All except for (B) work. (B) crashes with a Django-related message
Kubernetes Version
1.27.3
Operator version
0.87.0
Collector version
0.86.0
Environment information
No response
Log output
No response
Additional context
Somewhat related to https://github.com/open-telemetry/opentelemetry-operator/issues/1884#issuecomment-1617665333, however the container image I am using does not use a custom
PYTHONPATH
. Thechris
application is a fairly standard Django application, its container image is based onregistry.access.redhat.com/ubi9/python-311:1-17.1692772360
and its dependencies are installed withpip
. Nothing magical going on here.I propose two solutions: either
PYTHONPATH
added by Instrumentation