-
Notifications
You must be signed in to change notification settings - Fork 17
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
Record request information as metadata #100
Conversation
Aside from just adding request information to the environment field, add it to the metadata as well to allow for use in filtering. Fixes appsignal/support#333.
8c13f73
to
49a2f25
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.
This will work, but it can't be amended by something else, like if we want to add more metadata from the Plug or Absinthe integration too.
I suggested using the set_meta_data
Nif function but I noticed that doesn't work for the Span API or the attribute functions, but we can do that later if needed. It will show in the same place.
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.
Looks good modulo @tombruijn's comments. On what Tom said, re: making it possible to override it, it might help if it was possible to set this data when the span is created, rather than when it is closed.
Co-authored-by: Tom de Bruijn <tom@tomdebruijn.com>
Co-authored-by: Tom de Bruijn <tom@tomdebruijn.com>
Co-authored-by: Tom de Bruijn <tom@tomdebruijn.com>
Co-authored-by: Tom de Bruijn <tom@tomdebruijn.com>
Recent changes prefixed the metadata names with "request" or "response", and removed the hostname field. This patch updates the test to reflect those changes.
Aside from just adding request information to the environment field, add it to the metadata as well to allow for use in filtering.
Fixes https://github.com/appsignal/support/issues/333.