Skip to content

Refactor builds#536

Merged
austinlparker merged 44 commits into
open-telemetry:mainfrom
austinlparker:buildxRefactor
Nov 14, 2022
Merged

Refactor builds#536
austinlparker merged 44 commits into
open-telemetry:mainfrom
austinlparker:buildxRefactor

Conversation

@austinlparker

Copy link
Copy Markdown
Member

This fixes the problems with multi-arch builds -- the context for each build is not consistent.

@austinlparker
austinlparker requested a review from a team October 23, 2022 22:09
@austinlparker

Copy link
Copy Markdown
Member Author

Ok, good news/bad news.

Good news, this is actually building things!
Bad news, it's slow as heck because of qemu emulation.

I believe we could fix this by using https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/ to build amd64 base images and arm64 binaries, but I'm not entirely sure if that'll help for stuff like PHP. I'd intuit that it'd help in C++, but I'm not sure if it'd work to build amd64 grpc and otel then link to an arm64 binary.

Would love if @brettmc and/or @DebajitDas could chime in with ideas as well.

@cartersocha

Copy link
Copy Markdown
Contributor

Cc @lalitb on the above question too

@lalitb

lalitb commented Oct 24, 2022

Copy link
Copy Markdown
Member

but I'm not sure if it'd work to build amd64 grpc and otel then link to an arm64 binary.

That won't work for C++. Tagging @esigo as he has done some work on qemu/ARM CI builds earlier.

@esigo

esigo commented Oct 24, 2022

Copy link
Copy Markdown
Member

I know only two ways to address the slow build issue.

  1. The dependencies being stored in some image and hosted in some registry (not possible for us I think).
  2. Saving the dependencies image in GitHub actions artifact and loading the artifact before builds similar to what I was trying to do here.

I can work on the second option after C++ metric GA.

@austinlparker austinlparker added in-progress This issue is actively being worked on and removed v1-frozen labels Oct 31, 2022
@austinlparker

Copy link
Copy Markdown
Member Author

Please see #458 (comment) for the current state of work here.

@austinlparker austinlparker changed the title Support multi-arch builds Refactor builds Nov 13, 2022
@austinlparker

Copy link
Copy Markdown
Member Author

Ok, I'm willing to call this good. I've renamed the PR to reflect what it currently does (which is just refactor the build rather than support multi-arch) but it does lay the foundation for multi-arch in the future. As a benefit, this significantly decreases build time due to parallelizing each service. Since it can do that, and run on free instances, I'm inclined to keep the per-PR build check in there to ensure that we aren't getting compilation errors.

@cartersocha cartersocha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@austinlparker

Copy link
Copy Markdown
Member Author

Doing a bit more work to try and speed up CPP by installing https://github.com/rui314/mold to speed up linking.

Pre-mold: gRPC build 1043s
Post-mold: gRPC build 813s

Pre-mold: OpenTelemetry build 172s
Post-mold: OpenTelemetry build 133s

... Unfortunately, building mold costs 520s, thus obliviating the speedup 🙃

However, I suspect greater returns on end-user hardware so it's probably worth keeping?

@mviitane

Copy link
Copy Markdown
Member

Unfortunately, I don’t see improvements in local builds:

Ryzen 5600G:

docker compose build currencyservice --no-cache | Original 400.9s | This PR 505.5s
docker compose build featureflagservice --no-cache | Original 73.8s | This PR 85.0s
docker compose build shippingservice --no-cache | Original 182.9s | This PR 177.6s

Mac M1 Pro:

docker compose build currencyservice --no-cache   | Original 472.4s | This PR 639.5s
docker compose build featureflagservice --no-cache | Original 94.1s | This PR 102.9s
docker compose build shippingservice --no-cache   | Original 254.9 | This PR 248.0s

currencyservice and featureflagservice build times increased. The mold linker gave some linking improvements on x86, but the mold install takes way too long to be beneficial overall.

@mviitane

Copy link
Copy Markdown
Member

I think the GitHub workflow changes should be delivered separately.

@austinlparker
austinlparker merged commit fd515c2 into open-telemetry:main Nov 14, 2022
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* switch to matrix build strategy

Co-authored-by: Carter Socha <[email protected]>
@julianocosta89 julianocosta89 removed the in-progress This issue is actively being worked on label Nov 25, 2024
mohamed3637 added a commit to mohamed3637/opentelemetry-demo that referenced this pull request Oct 7, 2025
* switch to matrix build strategy

Co-authored-by: Carter Socha <[email protected]>
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.

6 participants