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

Replace Dockerfile with Scratch as Base Image #214

Merged
merged 11 commits into from
Jul 5, 2024

Conversation

harrryr
Copy link
Contributor

@harrryr harrryr commented Jun 19, 2024

Issue #, if available:
Currently amazon linux is used as the base image for OTEL python distribution. This image contains a lot of unnecessary packages that inflate the image and also cause issues during high sev scans. Ideally we would like to use an image with minimal content.

Scratch was the base image that was proposed to be used with cpUtility installed to enable the cp command to copy the python image to the sample app pod. The cpUtility is copied from aws-otel-java-instrumentation repo, but there is a small issue where if copied from a folder, the destination directory is not correct.

This piece of code in cp-utility causes the directory to change if the destination folder already exists. It was modified so that that if the destination folder exists, just reuse it and don't create a new folder. Other than also modifying the unit test, the code is identical to the otel java

Test run: https://github.com/aws-observability/aws-otel-python-instrumentation/actions/runs/9586337462

Also manually built the image locally and deployed to EKS cluster with sample app. Verified that correct telemetry was generated and opentelemetry folder is located in correct path

Additionally, ran testing in ARM64 EC2 instance

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@harrryr harrryr requested a review from a team as a code owner June 19, 2024 17:54
@harrryr harrryr force-pushed the update-dockerfile-with-scratch branch from 9e78a71 to 27a033c Compare June 19, 2024 18:48
@harrryr harrryr force-pushed the update-dockerfile-with-scratch branch from d4ac377 to 2445d24 Compare June 19, 2024 20:01
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
tools/cp-utility/src/main.rs Show resolved Hide resolved
ignore-words.txt Outdated Show resolved Hide resolved
@harrryr harrryr force-pushed the update-dockerfile-with-scratch branch from bab6464 to 33d122b Compare July 3, 2024 18:00
ignore-words.txt Outdated Show resolved Hide resolved
srprash
srprash previously approved these changes Jul 3, 2024
Copy link
Contributor

@thpierce thpierce left a comment

Choose a reason for hiding this comment

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

Please add testing for both AMD64 and ARM64 as we have conditional logic for both.

tox.ini Outdated Show resolved Hide resolved
@harrryr harrryr merged commit 6177b7e into main Jul 5, 2024
11 checks passed
@harrryr harrryr deleted the update-dockerfile-with-scratch branch July 5, 2024 23:01
mxiamxia pushed a commit to aws-observability/aws-otel-js-instrumentation that referenced this pull request Oct 24, 2024
*Issue #, if available:*
ADOT SDK instrumentation Docker Images only support `cp -a` copy command
so that the [CWAgentOperator can
extract](https://github.com/aws/amazon-cloudwatch-agent-operator/blob/main/pkg/instrumentation/nodejs.go#L65)
the instrumentation SDK via `cp -a`, but upstream OTel Operator has
[changed a few months
ago](open-telemetry/opentelemetry-operator@4cd6dcb)
to use `cp -r` instead of `-a` to copy files from the OTel SDK
Instrumentation Images. Today, OTel Operator is not compatible with some
of the ADOT SDKs, and in the future, CWAgent Operator may also change to
use `cp -r` as well

*Description of changes:*
- Copy changes from Java's PR to add `cp -r` support -
aws-observability/aws-otel-java-instrumentation#843
- Modify the above `cp -r` support with bug fix implemented in Python's
`cp -a` command -
aws-observability/aws-otel-python-instrumentation#214 (comment)


By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
mxiamxia pushed a commit that referenced this pull request Oct 25, 2024
*Issue #, if available:*
ADOT SDK instrumentation Docker Images only support `cp -a` copy command
so that the [CWAgentOperator can
extract](https://github.com/aws/amazon-cloudwatch-agent-operator/blob/main/pkg/instrumentation/nodejs.go#L65)
the instrumentation SDK via `cp -a`, but upstream OTel Operator has
[changed a few months
ago](open-telemetry/opentelemetry-operator@4cd6dcb)
to use `cp -r` instead of `-a` to copy files from the OTel SDK
Instrumentation Images. Today, OTel Operator is not compatible with some
of the ADOT SDKs, and in the future, CWAgent Operator may also change to
use `cp -r` as well

*Description of changes:*
- Copy changes from Java's PR to add `cp -r` support -
aws-observability/aws-otel-java-instrumentation#843
- Modify the above `cp -r` support with bug fix implemented in Python's
`cp -a` command -
#214 (comment)

*Testing:*
- Tested on PetClinic Sample App's Python Services with latest OTel
Operator + Collector


![image](https://github.com/user-attachments/assets/c08aec2b-01ce-46c4-abfd-27c5b4f16c6b)


By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
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.

3 participants