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

Pass the Lambda context to handlers #311

Merged
merged 7 commits into from
Apr 20, 2019
Merged

Pass the Lambda context to handlers #311

merged 7 commits into from
Apr 20, 2019

Conversation

amne
Copy link

@amne amne commented Apr 18, 2019

We would like access to the aws-request-id header that is returned by the runtime API.

Our specific use case is that we use the request id to correlate log entries generated by this lambda invocation. Initially we generated our own correlation id but we noticed we can't link them to the cloudwatch entries but we can link them if we use the lambda request id as the correlation id.

@mnapoli
Copy link
Member

mnapoli commented Apr 18, 2019

That's really interesting!

I think we can do without the ContextAwareHandler interface. Indeed it is possible to invoke a function with "extra" arguments. If the function doesn't use them or even declare them they will simply be ignored.

Copy link
Member

@mnapoli mnapoli left a comment

Choose a reason for hiding this comment

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

Thank you for working on this, this is really a great change and it brings us closer to other languages!

src/Runtime/Context.php Outdated Show resolved Hide resolved
src/Runtime/Context.php Outdated Show resolved Hide resolved
src/Runtime/Context.php Outdated Show resolved Hide resolved
src/Runtime/Context.php Outdated Show resolved Hide resolved
src/Runtime/Context.php Outdated Show resolved Hide resolved
Cornel-Cristian Cruceru added 2 commits April 19, 2019 00:02
moved Context object to its own namespace
made Context immutable by removing its setters and adding a constructor
added a ContextBuilder that will build an immutable Context object
adjusted curl's header function to use context builder
adjusted tests
improved phpdoc blocks
if ($body === '') {
throw new \Exception('Empty Lambda runtime API response');
}

$context = $contextBuilder->buildContext();

if ($context->getAwsRequestId() === '') {

Choose a reason for hiding this comment

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

Is a Context with an empty aws request id valid? Maybe this check should be moved in the ContextBuilder? (same question for the other properties of the context, what is the minimal set of data that constitutes a valid context?)

Copy link
Author

Choose a reason for hiding this comment

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

context can be completely empty but that piece of code expects a non-empty invocation id. It's just a coincidence I'm refetching it from the context instead of keeping around in an $invocationId variable. I could do revert back to using a $invocationId variable in that piece of code and build the context last just before return

Copy link
Member

Choose a reason for hiding this comment

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

It might ideally be better indeed to forbid having an invalid Context around, but honestly this should never happen and an exception will be thrown if it ever happens. Let's not block this PR longer than necessary.

Thank you for updating your PR @amne.

@mnapoli mnapoli changed the title add support for context aware handler Pass the Lambda context to handlers Apr 20, 2019
@mnapoli mnapoli merged commit 25c2085 into brefphp:master Apr 20, 2019
mnapoli added a commit that referenced this pull request Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants