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-85973 NH-89212 instrument function handler dependencies #144

Merged
merged 11 commits into from
Aug 27, 2024
Merged

Conversation

cheempz
Copy link
Contributor

@cheempz cheempz commented Aug 21, 2024

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.

@cheempz cheempz changed the title NH-85973 instrument function handler dependencies NH-85973 NH-89212 instrument function handler dependencies Aug 23, 2024
Copy link
Contributor Author

@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.

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.

lambda/otel/layer/otel_wrapper.rb Show resolved Hide resolved
lambda/otel/layer/otel_wrapper.rb Outdated Show resolved Hide resolved
lambda/otel/layer/otel_wrapper.rb Show resolved Hide resolved
@xuan-cao-swi
Copy link
Contributor

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.

rescue can prevent the crash if certain library is not present in user's layers

sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
sample_ruby_file.rb Fixed Show fixed Hide fixed
CONFIGURATION.md Outdated Show resolved Hide resolved
CONFIGURATION.md Outdated Show resolved Hide resolved
@cheempz cheempz marked this pull request as ready for review August 26, 2024 21:59
@cheempz cheempz requested a review from a team as a code owner August 26, 2024 21:59
Copy link
Contributor Author

@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.

Getting close! left a few suggestions.

xuan-cao-swi and others added 2 commits August 27, 2024 10:16
@cheempz cheempz merged commit a56b291 into main Aug 27, 2024
14 checks passed
@cheempz cheempz deleted the NH-85973 branch August 27, 2024 18:27
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.

4 participants