Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use opentelemetry-instrument script to invoke python app #152

Closed
anuraaga opened this issue Oct 11, 2021 · 4 comments · Fixed by #164
Closed

Use opentelemetry-instrument script to invoke python app #152

anuraaga opened this issue Oct 11, 2021 · 4 comments · Fixed by #164

Comments

@anuraaga
Copy link
Contributor

anuraaga commented Oct 11, 2021

Currently, we have a wrapper script for initializing the python function

https://github.com/open-telemetry/opentelemetry-lambda/blob/main/python/src/otel/otel_sdk/otel_wrapper.py

I'm wondering if there is any known issue with instead invoking the standard opentelemetry-instrument script?

https://github.com/open-telemetry/opentelemetry-python/tree/main/opentelemetry-instrumentation#opentelemetry-instrument

Perhaps it's related to the two TODOs?

https://github.com/open-telemetry/opentelemetry-lambda/blob/main/python/src/otel/otel_sdk/otel_wrapper.py#L15

If we could leverage opentelemetry-instrument, it ensures there is only one consistent and officially maintained method for enabling Python instrumentation.

Note I am thinking of this in the context of adding auto instrumentation to k8s - I think the concept of updating the runtime for auto instrumentation in a k8s pod and a lambda function are almost, if not always, equivalent.

open-telemetry/opentelemetry-operator#455

@wangzlei @NathanielRN Any ideas?

@wangzlei
Copy link
Contributor

I think for k8s case we can use otel python sdk opentelemetry-instrument directly, not related to opentelemetry-lambda otel_wrapper.py?

But yes, definitely it is great if we can use python auto instrumentation in Lambda. It is a TODO task with moving Lambda instrumentation to Python contrib repo.
Here I provide the story of current redundant code in otel_wrapper.py and the possible solution.

  1. In AWS_LAMBDA_EXEC_WRAPPER bootstrap, it does not know the path of otel libraries, opentelemetry-instrument does not work. That is the key I have to manually enable auto instrumentation in otel_wrapper.py after Lambda python runtime gets started.
  2. Then I copied code from OTel Python sitecustomize.py to otel_wrapper.py, the difference is to suppress the exception in load instrumentators, see https://github.com/open-telemetry/opentelemetry-python/blob/v1.4.0/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py#L85

If we can solve the first problem by manually setup python sys path in AWS_LAMBDA_EXEC_WRAPPER bootstrap, then no longer need otel_wrapper.py. AWS_LAMBDA_EXEC_WRAPPER bootstrap for Python would look like:

set otel libraries into python sys path
system("opentelemetry-instrument".join(args))

If the first problem is hard to be solved, the alternative way is to further investigate why there is exception gets raised in load instrumentors, so we can call sitecustomize.py instead of maintain code in Lambda. Or work with Python SDK repo to provide a no exception version.

@NathanielRN
Copy link
Contributor

Isn't this what #125 was trying to solve? We saw the blockers that prevented solving this issue on that PR.

@wangzlei
Copy link
Contributor

Ideally we want to remove these parts from Lambda repo

@anuraaga
Copy link
Contributor Author

anuraaga commented Oct 14, 2021

@NathanielRN In #125 we attempted to move the instrumentation logic into the execution script itself. We don't expect that to work since instrumentation happening before call to system wouldn't affect the subprocess.

As far as I can tell, python's opentelemetry-instrument does not take this approach, it seems to only do a minimal modification of the environment before executing the subprocess which is the approach we'd expect to work ok

https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py#L111

I didn't dig into the details of what it does (I suspect it modified the python path so that the sitecustomize.py next to it gets read automatically by the python subprocess) but at a glance it doesn't seem to have an obvious problem. I'm suspecting we could find a simple solution to @wangzlei's point 1), maybe by using the PYTHONPATH variable or including a site-packages directory in the layer, which would get overlayed so merged into whatever's in the runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants