-
Notifications
You must be signed in to change notification settings - Fork 3
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-85973 NH-89212 instrument function handler dependencies #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some thoughts @xuan-cao-swi. And i see what you meant with ugly code to achieve this ;)... having to parse through the function code for require
definitely isn't great but i also can't see a way around this for now.
The main thing i'm concerned with is that the extra preload and bootstrap logic don't introduce additional risk of function init failures (aka lambda crash) and startup overhead.
Co-authored-by: Lin Lin <[email protected]>
rescue can prevent the crash if certain library is not present in user's layers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting close! left a few suggestions.
Co-authored-by: Lin Lin <[email protected]>
Co-authored-by: Lin Lin <[email protected]>
Description
Prior to this change, solarwinds_apm is loaded before the original handler, which means function handler dependencies are never instrumented because they are loaded too late (after OTel bootstraps the instrumentations).
Additionally, we found an AWS SDK client initialized outside the function handler was not instrumented even if solarwinds_apm is loaded after function code--this is due to the way AWS SDK instruments during client initialization rather than prepending.
The solution is to preload function's dependencies and explicitly bootstrap instrumentation, before loading solarwinds_apm.
Test (if applicable)
Manual testing with RC layer.