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

NH-82144 and NH-81061: multi-runtime lambda layer #131

Merged
merged 9 commits into from
Jun 24, 2024
Merged

Conversation

xuan-cao-swi
Copy link
Contributor

Description

Change the build process using docker image for multi-runtime layer.
Reserved the sam build material for extra use (mainly debugging)

Test (if applicable)

Action run: https://github.com/solarwinds/apm-ruby/actions/runs/9485541864

x86_64arm64

@xuan-cao-swi xuan-cao-swi requested a review from a team as a code owner June 12, 2024 18:17
@cheempz cheempz changed the title NH-82144: multi-runtime lambda layer NH-82144 and NH-81061: multi-runtime lambda layer Jun 13, 2024
@cheempz
Copy link
Contributor

cheempz commented Jun 13, 2024

Quick question: not seeing the APM library version in the layer name, i.e. there's a trailing - in the layer name:

arn:aws:lambda:us-east-1:858939916050:layer:solarwinds-apm-ruby-arm64-:4

I was expecting

arn:aws:lambda:us-east-1:858939916050:layer:solarwinds-apm-ruby-arm64-6_0_0:4

lambda/otel/layer/Gemfile Outdated Show resolved Hide resolved
@xuan-cao-swi
Copy link
Contributor Author

I was expecting

arn:aws:lambda:us-east-1:858939916050:layer:solarwinds-apm-ruby-arm64-6_0_0:4

Yes, it will be solarwinds-apm-ruby-arm64-6_0_0:4. Currently, because of the repo url point to source 'https://rubygems.pkg.github.com/xuan-cao-swi' so it didn't parse the version

@xuan-cao-swi xuan-cao-swi requested a review from cheempz June 18, 2024 17:38
@cheempz
Copy link
Contributor

cheempz commented Jun 24, 2024

@xuan-cao-swi I'm wondering about the reason for a slightly different approach here (using SAM container images for ruby) vs open-telemetry/opentelemetry-lambda#1376. It does seem more convenient to use the prefab SAM container, and indeed we don't need to keep our layer build the same as upstream since we have vendor-specific concerns. Mainly curious what was the rationale and your intent going forward.

Oh maybe for our layer you used this approach? https://docs.aws.amazon.com/lambda/latest/dg/ruby-package.html#ruby-package-native

@xuan-cao-swi
Copy link
Contributor Author

It's mainly because of different version of glibc (ubuntu:latest vs amazonlinux2) for compiling the liboboe. Since all aws ruby lambda runtime start with amazonlinux2 or amazonlinux2023, so when we build our apm-ruby layer, the glibc version have to be aligned with what amazonlinux glibc version. I think it's easier to use sam image so that I am sure the os has the correct glibc version

As upstream (standalone) opentelemetry-sdk doesn't need compile anything with glibc (e.g. liboboe), it doesn't matter which image to use for build.

Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

LGTM

@xuan-cao-swi xuan-cao-swi merged commit 335e722 into main Jun 24, 2024
18 checks passed
@xuan-cao-swi xuan-cao-swi deleted the NH-82144 branch June 26, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants