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-65069 Lambda layer 0.8 MB decrease #214

Merged
merged 6 commits into from
Nov 15, 2023

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Nov 8, 2023

This shrinks APM Python lambda layer zip size from 9.5 MB to 8.7 MB by removing super-upstream deps that don't seem to be needed for actual instrumentation. Otel Python does something similar in its builds: https://github.com/open-telemetry/opentelemetry-lambda/blob/main/python/src/otel/Dockerfile#L19-L20

Also removes an unused target publish-lambda-layer-rc and changes the temp dir name used for lambda publish, as it was conflicting with this dir for cert stuff.

Tested that instrumentation still works by merging this with Settings API PoC branch, building and uploading new layer, attaching layer to function, and invoking function. Traces and metrics export still work:

@tammy-baylis-swi tammy-baylis-swi changed the title NH-65069 Lambda layer content fixes NH-65069 Lambda layer 0.8 MB decrease Nov 8, 2023
@tammy-baylis-swi tammy-baylis-swi marked this pull request as ready for review November 8, 2023 02:55
@tammy-baylis-swi tammy-baylis-swi requested a review from a team November 11, 2023 00:13
@tammy-baylis-swi
Copy link
Contributor Author

Ready for review. Not sure why reviewers didn't automatically add when I first put this in.

cheempz
cheempz previously approved these changes Nov 15, 2023
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.

Makefile Outdated
aws-lambda: check-zip wrapper
@if [ -f ./dist/solarwinds_apm_lambda_${platform}.zip ]; then \
echo -e "Deleting old solarwinds_apm_lambda_${platform}.zip"; \
rm ./dist/solarwinds_apm_lambda_${platform}.zip; \
fi
rm -rf ./tmp
rm -rf ./tmp-lambda
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor curiosity why this and a few other commands below don't use ${target_dir} instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason! Fixed in 29e600c 😸

@tammy-baylis-swi
Copy link
Contributor Author

Thank you @cheempz ! One more look when you have a moment please.

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.

👍 thanks for the revisit!

@tammy-baylis-swi tammy-baylis-swi merged commit 70b3c78 into main Nov 15, 2023
11 checks passed
@tammy-baylis-swi tammy-baylis-swi deleted the NH-65069-layer-content-update branch November 15, 2023 18:52
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