-
Notifications
You must be signed in to change notification settings - Fork 1
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
NH-27837: add aarch64 support #20
Conversation
Thanks @xuan-cao-swi ! Lgtm, though I'd like for others to have a look at this too 😺 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @xuan-cao-swi, the extconf.rb
change to support ARM itself looks good (I gave the RC version in packagecloud a spin in Linux ARM containers on an M1 mac and worked as advertised).
The rest of this PR you grappled with building test containers and trying to get the extensive testsuite to run and pass on ARM, which seems a bit unresolved since the GHA test runs so far haven't passed.
Since some test dependencies don't yet support ARM, and in GHA we're forced to run test containers under simulation (aka sloooooow), I'd like to suggest a different approach: create a set of "install tests" that cover the platforms supported -- i.e. ruby versions x distros x architecture
. These install tests can be fairly simple similar to the post release generate-a-few-traces test already in place, or for a much more intense experience check out https://github.com/solarwindscloud/solarwinds-apm-python/blob/main/.github/workflows/verify_install.yaml. The underlying idea is the same -- our APM library's platform dependence hinges on liboboe, which a large part of it is exercised simply by running an instrumented app and reporting traces (connection to backend, retrieval of settings, batching and transmission of data).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, though I'd recommend @cheempz have another look before merging to check the latest changes against the suggestions from this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, looks like the verify install changes from other PR have been merged into this one, thanks @xuan-cao-swi!
Why?
Have the agent be able to run on aarch64 architecture.
Impact?
Update the
extconf.rb
file that download correct liboboe binaryUpdate the test case with support on aarch64 (I interchange aarch64 with arm64) docker images
Update the github action file to have support for aarch64 images
Supplementary
Test can't be performed with grpc and memcached on aarch64 images because grpc and memcached gem can't be installed on aarch64 images. So I simply skipped these test case on every aarch64 os
--build=aarch64-unknown-linux-gnu
inextconf.rb
like this issue. Futhermore, the memcached gem is not maintained anymore, and people are shifting to dalli (we also instrumented dalli)Removed the support for centos 7 aarch64 support because of the linking issue. @jerrytfleung may offer help to overcome the issue I mentioned in here. But the centos 7 will be de-supported so I am ready to drop it off.
PS. also had failed testing it on aws ubuntu aarch64 with docker container
Work around for centos: upgrade gcc by building from the source.
P.S. Jerry will update the build dependency for liboboe 11.1.0, this step is not necessary anymore.
P.S. centos 7 aarch64 can't use solarwinds-apm-ruby, but using the above instruction can help overcome the old library issue