RFC: instrument lambda handler

swift-server / swift-aws-lambda-runtime

Swift implementation of AWS Lambda Runtime

Apache License 2.0

1.13k stars 102 forks source link

RFC: instrument lambda handler #162

Closed pokryfka closed 3 years ago

pokryfka commented 4 years ago

PoC instrumentation of the lambda handler.

Motivation:

Add support for instrumentation.

Modifications:

creates an instance of a tracer (currently AWSXRayRecorder) - needs to be configurable
pass tracer TracingInstrument and (context) baggage BaggageContext in Lambda.Context
instrument invocation handling
flushes the tracer after each invocation

TODO

[ ] import swift-tracing instead of aws-xray-sdk-swift to limit dependencies
- [ ] stable TracingInstrument API
[ ] extend Lambda bootstrap to be able to use the same EventLoop in an instance of XRayUDPEmitter
[ ] discuss what exactly should be recorder by the handler

Result:

Example in X-Ray console of handling a simple HelloWorld, cold start, Foundation JSON encoder/decoder.

swift-server-bot commented 4 years ago

Can one of the admins verify this patch?

swift-server-bot commented 4 years ago

Can one of the admins verify this patch?

swift-server-bot commented 4 years ago

Can one of the admins verify this patch?

swift-server-bot commented 4 years ago

Can one of the admins verify this patch?

pokryfka commented 4 years ago

CC @ktoso @slashmo

ktoso commented 4 years ago

@swift-server-bot add to whitelist

ktoso commented 4 years ago

Awesome 👏 😎 Will give it a look :-)

pokryfka commented 4 years ago

@ktoso

Awesome 👏 😎 Will give it a look :-)

I have been using it for a week, obviously its PoC state, but I think PR may be a good way to discuss how it could be integrated including context propagation (note that Lambda defines its own Context which I used to pass the "context baggage" BaggageContext hmm...)

Attributes aside, I think it would be great to be able to share EventLoop to send UDP; something XRayRecorder does support but is currently not exposed by Lambda, InstrumentationSystem (bootstrap) and TracingInstrument (forceFlush)

pokryfka commented 4 years ago

14:23:44 Precondition failed: client not stopped before the deinit.: file /code/.build/checkouts/aws-xray-sdk-swift/Sources/AWSXRayUDPEmitter/UDPClient.swift, line 71

this breaks the tests at the moment, I was hoping ServiceLifecycle support will get merged https://github.com/swift-server/swift-aws-lambda-runtime/pull/141

slashmo commented 4 years ago

Nice @pokryfka, thanks for taking the initiative! 🎉

tomerd commented 4 years ago

thanks @pokryfka tagged this as RFC/draft for discussion

ktoso commented 4 years ago

(note that Lambda defines its own Context which I used to pass the "context baggage" BaggageContext hmm...)

Yeah, that's as-intended 👍

The lambda context should conform to LoggingBaggageContextCarrier as well (open to shorter /better names btw, could not think of a better one so far for this protocol 🤔 ).

tomerd commented 3 years ago

hi @pokryfka if you want to keep this around, could you please update this PR to be against the main branch so we can delete the old master branch