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

[proposal] Use rust based cp tool rather than busybox in autoinstrumentation images. #1698

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bryan-aguilar
Copy link
Contributor

@bryan-aguilar bryan-aguilar commented May 1, 2023

Issue: #1600

Apologies for this taking so long. As mentioned in the issue we would like to propose a replacement to the busybox base image used in the auto-instrumentation images. This PR is a demonstration on what that implementation would look like for a single auto-instrumentation images. If this proposal is accepted followup PRs would be made to adjust the remaining dockerfiles and github workflows.

The Rust based CP tool is statically linked to MUSL, which is licensed under MIT license. This implementation handles all the cases currently required by the Operator auto-instrumentation images: regular file copy (used in the Java) and archive copy (used in Python and NodeJS).

This is currently in use in our ADOT Java Instrumentation agent. Dockerfile ref.
Please feel free to ask any follow up questions. I will ensure this is brought up in the next sig meeting.

@bryan-aguilar bryan-aguilar requested a review from a team as a code owner May 1, 2023 23:58
@bryan-aguilar
Copy link
Contributor Author

I have added @rapphil as a codeowner also. He was the primary author of the rust cp-utility in our distribution and has agreed to assist in ongoing maintenance.

Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

Excited to hear more at the SIG meeting about this. Thanks for putting it together!

@@ -20,3 +20,6 @@

# Target Allocator owners
cmd/otel-allocator @open-telemetry/operator-ta-maintainers

# cp-utility tool
tools/cp-utility @rapphil @bryan-aguilar
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll have to make another group in order to use CODEOWNERS here

FROM busybox

# Stage 1: Build the cp-utility binary
FROM rust:1.69 as builder
Copy link
Member

Choose a reason for hiding this comment

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

can the rust cp impl. be built for all target platforms supported by go? What I have in mind are ibm z + power (that's what we have to support).

Copy link

Choose a reason for hiding this comment

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

yes, s390x and ppc64le are supported by the rust base image and musl.

Are you using the autoinstrumentation images that are published from this repo in those platforms? I'm asking because today they only support arm and x86 (please take a look here)

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.

None yet

4 participants