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.

Sign up for GitHub

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

Jump to bottom

Configurable allow list of HTTP headers to record #215

Closed
MikeGoldsmith opened this issue Sep 20, 2023 · 1 comment · Fixed by #277
Closed

Configurable allow list of HTTP headers to record #215

MikeGoldsmith opened this issue Sep 20, 2023 · 1 comment · Fixed by #277
Assignees
@MikeGoldsmith
Labels
type: enhancement New feature or request
Milestone
Pre beta

Comments

@MikeGoldsmith
Copy link
Contributor

MikeGoldsmith commented Sep 20, 2023 •
edited by robbkidd
Loading

The agent captures an hard-coded list of specific HTTP request headers. Headers often include sensitive information that should not be recorded in telemetry, so we ought not record them all. Adding a configurable allow list of header names to record would allow users to selectively include headers they want the agent to record into events sent to Honeycomb.
@MikeGoldsmith MikeGoldsmith added the type: enhancement New feature or request label Sep 20, 2023
@MikeGoldsmith MikeGoldsmith added this to the Pre beta milestone Sep 20, 2023
@robbkidd robbkidd changed the title Add allow list of HTTP headers to record Configurable allow list of HTTP headers to record Sep 20, 2023
@MikeGoldsmith MikeGoldsmith self-assigned this Oct 11, 2023
@MikeGoldsmith MikeGoldsmith mentioned this issue Oct 11, 2023
Merged
@JamieDanielson
Copy link
Contributor

JamieDanielson commented Oct 11, 2023 •
edited
Loading

Consider having a specific allow-list. Include user-agent by default, and show that in an example and encourage folks to always have it. We can do this like the OTEL_PROPAGATORS, which defaults to tracecontext,baggage and if that environment variable is updated it must include those explicitly if they are wanted, otherwise they will be overwritten.
Since this will likely be case-sensitive, be sure to document that. We'll see if it's necessary to lowercase everything based on feedback, but to reduce performance overhead in processing headers. If we can find a low-impact way to check lowercase things, that would be ideal.

robbkidd added a commit that referenced this issue Oct 13, 2023
@MikeGoldsmith @JamieDanielson @robbkidd
feat: Add configurable list of HTTP headers to extract (#277)
5cc0ff5 
## Which problem is this PR solving?
The current list of HTTP headers that are extracted for request/response
pairs is limited to just `User-Agent`.

This PR adds a configuration option to provide a custom set of headers
that will be extracted from HTTP request/response pair. The agent looks
in the new `HTTP_HEADERS` env var for a comma separated list of headers
to extract in place of default set of headers.

- Closes #215 

## Short description of the changes
- Move default headers to extract from http parser to config 
- Add `HTTPHeadersToExtract` to config struct which is populated from
the default set plus any additional headers found in `HTTP_HEADERS` env
var
- Add new `LookupEnvAsStringSlice` func to utils that is used to get the
list of headers
- Update HTTP Parser to take the list of headers to extract as part of
`NewHTTPParser`
- Add unit tests to config and http parser
- Add new env var to README

## How to verify that this has the expected result
When configuring the agent, a list of additional HTTP headers can be set
and will be extracted from HTTP request/responses.

---------

Co-authored-by: Jamie Danielson <jamieedanielson@gmail.com>
Co-authored-by: Robb Kidd <robbkidd@honeycomb.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MikeGoldsmith MikeGoldsmith

Labels
type: enhancement New feature or request
Projects
None yet
Milestone
Pre beta
Development

Successfully merging a pull request may close this issue.

feat: Add configurable list of HTTP headers to extract honeycombio/honeycomb-network-agent
2 participants
@MikeGoldsmith @JamieDanielson