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

Change AWS_EXECUTION_ENV set in the lambda init binary #10212

Merged
merged 3 commits into from Feb 13, 2024
Merged

Conversation

dfangl
Copy link
Member

@dfangl dfangl commented Feb 9, 2024

Motivation

The lambda init binary should have the AWS_EXECUTION_ENV set to AWS_Lambda_rapid. The setting of the concrete value depending on the runtime will be done in /var/runtime/bootstrap, and is part of the lambda images.

Some extensions depend on that value being correct for checking what runtime is running (datadog).

Even though this is a parity adjustment, it is a quite significant change, so I labeled it minor for now.

Changes

  • Change execution env variable to AWS_Lambda_rapid
  • Allow a custom init even if LAMBDA_INIT_DEBUG is not set
  • Add test for introspection of the init process environment

@dfangl dfangl added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Feb 9, 2024
Copy link

github-actions bot commented Feb 9, 2024

LocalStack Community integration with Pro

    2 files      2 suites   1h 23m 5s ⏱️
2 634 tests 2 387 ✅ 247 💤 0 ❌
2 636 runs  2 387 ✅ 249 💤 0 ❌

Results for commit 46f0936.

♻️ This comment has been updated with latest results.

Copy link
Member

@joe4dev joe4dev left a comment

Choose a reason for hiding this comment

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

Love the new init environment test 🤩

I don't expect people to customize their init environment for LocalStack based on the runtime suffix we had for AWS_EXECUTION_ENV.

Nothing blocking the merge; just added some comments/suggestions (e.g., regarding our internal dev flow).

@@ -358,10 +358,11 @@ def start(self, env_vars: dict[str, str]) -> None:
self.container_name, f"{str(get_runtime_client_path())}/.", "/"
)
# tiny bit inefficient since we actually overwrite the init, but otherwise the path might not exist
if config.LAMBDA_INIT_DEBUG:
if config.LAMBDA_INIT_BIN_PATH:
Copy link
Member

Choose a reason for hiding this comment

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

neat idea to decouple debugging from init overwrite. How does it facilitate your workflow?

LAMBDA_INIT_DEBUG was convenient to enable the entire init copying and debugging. I typically commented exec /var/rapid/init in the Lambda init repo under /debugging/init/var/rapid/entrypoint.sh to toggle debugging.
This is still necessary but separating the conditional here would require two steps (+1 toggle) to use debugging 🤔

Copy link
Member

Choose a reason for hiding this comment

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

(already hit me 😅 when trying to test the changes while having LAMBDA_INIT_BIN_PATH set by default)

Copy link
Member Author

Choose a reason for hiding this comment

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

The lambda init path was necessary anyway before. Now you can set only the init path to enable usage of a new lambda init binary without having to setup debugging itself.
You cannot toggle the init copying with the debug variable, but I would suggest using different profiles for this, instead of the toggle of init debug?

I can leave it out from this PR if you do not think it improves usability for developers, but for me it simplifies it, because I more often try out new inits without debugging than with.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that we should have a way to simplify init testing without debugging 👍

Using profiles makes a lot of sense here 💯 . Then having two entrypoint.sh versions, one for testing (exec /var/rapid/init) and one for debugging makes switching easier. Thanks for sharing 👍

tests/aws/services/lambda_/test_lambda.py Show resolved Hide resolved
tests/aws/services/lambda_/test_lambda.py Outdated Show resolved Hide resolved
tests/aws/services/lambda_/test_lambda.py Outdated Show resolved Hide resolved
tests/aws/services/lambda_/test_lambda.py Show resolved Hide resolved
@@ -696,6 +697,66 @@ def assert_events():
)
snapshot.match("invoke-result-read-number-after-timeout", result)

@markers.aws.validated
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting this diff when validating against AWS:

>> match key: create-result
	(+) /CreateFunctionResponse/LoggingConfig ( {'LogFormat': 'Text', 'LogGroup': '/aws/lambda/<function-name:1>'} )

(Initially wanted to double-check "_AWS_XRAY_DAEMON_ADDRESS": "169.254.79.129", but then noticed this one; might be a very recent update)

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I did not have botocore updated, thanks for the hint and the double check!

@joe4dev
Copy link
Member

joe4dev commented Feb 9, 2024

Obviously, LOCALSTACK_POST_INVOKE_WAIT_MS needs to be ignored as well to unblock the CI:

>> match key: lambda-init-inspection
	#x1B[32m(+)#x1B[0m /Payload/environment/LOCALSTACK_POST_INVOKE_WAIT_MS ( '50' )

@coveralls
Copy link

Coverage Status

coverage: 83.84% (+0.01%) from 83.83%
when pulling 46f0936 on aws-runtime-env
into 04d92da on master.

Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM, I don't have a strong opinion in either direction regarding the init flag changes though 👍

if self.function_version.config.runtime:
env_vars["AWS_EXECUTION_ENV"] = f"Aws_Lambda_{self.function_version.config.runtime}"
env_vars["AWS_EXECUTION_ENV"] = "AWS_Lambda_rapid"
Copy link
Member

Choose a reason for hiding this comment

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

great catch 👍

@dfangl dfangl merged commit fd067cb into master Feb 13, 2024
27 checks passed
@dfangl dfangl deleted the aws-runtime-env branch February 13, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants