-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: meta_struct support #184
Conversation
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
567af52
to
d5035d1
Compare
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
d5035d1
to
53ef913
Compare
It seems that the CI is broken with the following error: |
ah yes ignore that error, there's some weird dependency issue that broke in the changelog job! |
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.
just one suggestion for where the mutation logic should be, otherwise looks good to me!
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.
LGTM (modulo the same concern that @Kyle-Verhoog expressed in his review)
Signed-off-by: Eliott Bouhana <eliott.bouhana@datadoghq.com>
9d1aceb
to
6ed45f8
Compare
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.
just one non-blocking thought. Great work. Thank you for the tests and release note 🚢
Description
This PR adds support for a specific field supported since agent version v7.31 called
meta_struct
which encapsulate another layer of msgpack values inside the main msgpack chunk.Motivation
Appsec has been using this field to store stacktraces for multiple quarters already. We need to be able to test this feature
Notes to reviewer
Not overly happy about this implementation but there is not obvious way to do a first round of type checking, modify the span object, and then do a second round of type-checking. So I've simply put everything in the
verify_span
function.