-
Notifications
You must be signed in to change notification settings - Fork 977
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
adding metadata on requests #3635
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Gabe Cemaj <gcemaj@bloomberg.net>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gcemaj The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I am not sure about it. |
dump the headers you mean? that would also work, have a usecase where we need some extra metadata to be associated with the request. |
could also be an allowlist of headers? |
I might no be fully understand the reason of this change, is there a specific use case that just dumping the request headers in the logs is not enough? |
security concerns where there might be a access token in the headers |
I see, maybe just dump the request direct in the logs would be enough, wdyt? |
i think we can just have a allowlist for the headers? so you can explicitly configure what you want sent over? Does that seem reasonable? If so i can make the change in this PR |
Let's wait others to review. |
option to dump/forward headers + allowlist on headers will be something more flexible. prefix may not be so convenient as not all headers have the same prefix. |
ill make this change |
Signed-off-by: Gabe Cemaj <gcemaj@bloomberg.net>
What this PR does / why we need it:
Attempts to close #3634.
The PR adds a new configuration to the logger system to set a prefix string such that any header in the request that contains the header gets parsed and sent to the cloud event in a new metadata field.
Changes consists of
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #3634
Type of changes
Please delete options that are not relevant.
Feature/Issue validation/testing:
Please describe the tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.
Special notes for your reviewer:
Checklist:
Release note: