-
Notifications
You must be signed in to change notification settings - Fork 15
add snapstart span #911
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
add snapstart span #911
Conversation
318ae3f to
9ea2a19
Compare
| pub duration_ms: f64, | ||
| } | ||
|
|
||
| /// Restore report metrics |
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.
Please add test coverage for new struct and new field above.
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.
|
besides the unit tests, can we consider adding tests in bottlecap/tests/ to verify: |
| { | ||
| error!("Failed to send platform restore report to processor: {}", e); | ||
| } | ||
| } |
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.
Can we log error in case metrics is None as it may indicate malformed data?
litianningdatadog
left a comment
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.
Left some comments.
Task
https://datadoghq.atlassian.net/browse/SVLS-7836?atlOrigin=eyJpIjoiMWNmZTMzOGE4NGEwNDE4MTk5Njk0N2ZmMmU3MzExMjgiLCJwIjoiaiJ9
Overview
The extension neither creates SnapStart spans nor emits SnapStart metrics. This PR adds both.
When a lambda with snapshot enabled is invoked for the first time, we get
Platform.RestoreStartandPlatform.RestoreReport. These effectively take the place ofPlatform.InitStartandPlatform.InitReportevents, so our code flow is pretty much identical to how we handle the cold start span and duration metric.Note - When a SnapStart instance is restored, we actually receive the
Platform.InitStartandPlatform.InitReportevents in addition to thePlatform.RestoreStartandPlatform.RestoreReport. However, theInitevents are not from the sandbox starting for that invoke. TheseInitevents are actually generated from when the Snapshot is created. This is very misleading - You can see that this trace is more than 3 hours long. The lambda was invoked more than 3 hours after the snapshot version was created. (This is the current experience).Testing
I deployed my own extension with the changes and confirmed we are now getting a restore span and not an init span, link.