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

Record request information as metadata #100

Merged
merged 6 commits into from
Sep 2, 2024
Merged

Record request information as metadata #100

merged 6 commits into from
Sep 2, 2024

Conversation

jeffkreeftmeijer
Copy link
Member

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.

@jeffkreeftmeijer jeffkreeftmeijer added the enhancement New feature or request label Aug 30, 2024
@jeffkreeftmeijer jeffkreeftmeijer self-assigned this Aug 30, 2024
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.
Copy link
Member

@tombruijn tombruijn left a 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.

lib/appsignal_phoenix/event_handler.ex Outdated Show resolved Hide resolved
lib/appsignal_phoenix/event_handler.ex Outdated Show resolved Hide resolved
lib/appsignal_phoenix/event_handler.ex Outdated Show resolved Hide resolved
.changesets/record-request-information-as-metadata.md Outdated Show resolved Hide resolved
Copy link
Contributor

@unflxw unflxw left a 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.

jeffkreeftmeijer and others added 5 commits September 2, 2024 13:17
Recent changes prefixed the metadata names with "request" or
"response", and removed the hostname field. This patch updates the
test to reflect those changes.
@jeffkreeftmeijer jeffkreeftmeijer merged commit 6e7444a into main Sep 2, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants