Python autoinstrumentation setting of PYTHONPATH is not compliant with Python's module resolution behavior, breaking Django applications

[jennydaman]

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 unset PYTHONPATH.

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 annotation instrumentation.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 to sleep 100000 and then tried running the actual command interactively using kubectl exec ...

Expected Result

I expect the following commands to do nothing successfully:

# (A) pod is annotated, so PYTHONPATH is set to use autoinstrumentation
kubectl exec cacao-chris-heart-75666d5558-vw5gm -- python manage.py shell -c 'import core.storage.storagemanager'

# (B) same as above, but trying to import a module containing classes which extend django.db.models.Model
kubectl exec cacao-chris-heart-75666d5558-vw5gm -- python manage.py shell -c 'import feeds.models'

# (C) PYTHONPATH overridden with empty string to disable autoinstrumentation
kubectl exec cacao-chris-heart-75666d5558-vw5gm -- env PYTHONPATH= python manage.py shell -c 'import feeds.models'

# (D) Adding current working directory in front of the PYTHONPATH set by autoinstrumentation
kubectl exec cacao-chris-heart-75666d5558-vw5gm -- sh -c 'env PYTHONPATH="$PWD:$PYTHONPATH" python manage.py shell -c "import feeds.models"'

Actual Result

All except for (B) work. (B) crashes with a Django-related message

Traceback (most recent call last):
  File "/opt/app-root/src/manage.py", line 23, in <module>
    main()
  File "/opt/app-root/src/manage.py", line 19, in main
    execute_from_command_line(sys.argv)
  File "/opt/app-root/lib64/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/opt/app-root/lib64/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/opt/app-root/lib64/python3.11/site-packages/django/core/management/base.py", line 412, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/opt/app-root/lib64/python3.11/site-packages/django/core/management/base.py", line 458, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/app-root/lib64/python3.11/site-packages/django/core/management/commands/shell.py", line 117, in handle
    exec(options["command"], globals())
  File "<string>", line 1, in <module>
  File "/opt/app-root/src/feeds/models.py", line 8, in <module>
    class Feed(models.Model):
  File "/opt/app-root/lib64/python3.11/site-packages/django/db/models/base.py", line 134, in __new__
    raise RuntimeError(
RuntimeError: Model class feeds.models.Feed doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS.

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. The chris application is a fairly standard Django application, its container image is based on registry.access.redhat.com/ubi9/python-311:1-17.1692772360 and its dependencies are installed with pip. Nothing magical going on here.

I propose two solutions: either

• 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

[TylerHelmuth]

@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?

[jennydaman]

I’ll try doing a smaller reproduction case with a minimal Django project tomorrow.

[jennydaman]

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.

[TylerHelmuth]

pinging @open-telemetry/python-maintainers since I don't have permission to transfer the issue.

[ocelotl]

thanks for reporting @TylerHelmuth, will ask for this issue to be transferred to us.

[ocelotl]

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

[ocelotl]

• 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?

[jennydaman]

@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.

https://pep8.org/

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.