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

Create Docker image for Python auto-instrumentation. #524

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Nov 10, 2021

Created https://github.com/anuraaga/opentelemetry-operator/pkgs/container/opentelemetry-operator%2Fautoinstrumentation-python with it

I will write up the PR for the operator change after #507 is in. The mechanism, which I verified locally, is

PYTHONPATH=/otel-autoinstrumentation/opentelemetry/instrumentation/auto_instrumentation:$PYTHONPATH:/otel-autoinstrumentation

The trick is that the first entry is pointing to the folder with OTel's sitecustomize.py which gets automatically executed.

This is the same as what the opentelemetry-instrument script does I think except also appends the opentelemetry libraries to pythonpath, which otherwise is usually pip installed.

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

@anuraaga anuraaga requested review from a team and codeboten November 10, 2021 08:59
@anuraaga
Copy link
Contributor Author

@pavolloffay @jpkrohling

and @owais I guess this one you might be particularly interested in ;)

@@ -0,0 +1,43 @@
opentelemetry-distro==0.25b2
# We don't use the distro[otlp] option which automatically includes exporters since gRPC is not appropriate for
Copy link
Contributor Author

@anuraaga anuraaga Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this decision, but not sure on it. @owais In particular perhaps you have some advice on what we should use here in our "autoinstrumentation distro". The somewhat annoying aspect is that the operator will have to handle the OTEL_TRACES_EXPORTER for the user. But the alternative seems to be publishing an image per python version, which yikes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using proto http sounds reasonable if you can get away with it. I was working on a similar feature at some point and had decided to build one image per platform/architecture.


RUN mkdir workspace && pip install --target workspace -r requirements.txt

FROM busybox
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering why we are using busybox as a base image. Other operator images are based on gcr.io/distroless/static:nonroot. If there is no reason I would prefer to be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The distroless image doesn't contain the cp command which we need.

@anuraaga
Copy link
Contributor Author

e2e in #532 will fail until this is merged

@anuraaga
Copy link
Contributor Author

Don't think anything is blocking this - hopefully can merge and I'll wrap up 532

@jpkrohling jpkrohling merged commit 9d09b22 into open-telemetry:main Nov 12, 2021
shree007 pushed a commit to shree007/opentelemetry-operator that referenced this pull request Dec 12, 2021
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants