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

Provide request headers in various payload events #557

Closed
bodawei opened this issue Jul 18, 2017 · 7 comments
Closed

Provide request headers in various payload events #557

bodawei opened this issue Jul 18, 2017 · 7 comments
Milestone

Comments

@bodawei
Copy link
Contributor

@bodawei bodawei commented Jul 18, 2017

We'd like to be able to access request headers in the various events (RequestError and RequestLog) listed in https://github.com/hapijs/good/blob/master/lib/utils.js that accept a request (similar to RequestSent).

The reason for this is to be able to access certain request headers that we then want to add to our logs.

@bodawei

This comment has been minimized.

Copy link
Contributor Author

@bodawei bodawei commented Jul 18, 2017

Note: I can provide a pull request with this change!

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jul 18, 2017

Are you sure request headers are always available?

@bodawei

This comment has been minimized.

Copy link
Contributor Author

@bodawei bodawei commented Jul 18, 2017

In our case, yes. But, I think we'd do a conditional check and adding them if they are there.

@bodawei

This comment has been minimized.

Copy link
Contributor Author

@bodawei bodawei commented Jul 20, 2017

@arb I've been thinking about your comment, and wonder if you know something I don't. Is there some reason the request headers wouldn't be there?

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jul 20, 2017

I don't know, I just am kind of remembering that certain events don't include headers as there wasn't a request associated with the event.

I also feel like this could be a security concern. I think generally you attach some kind of unique ID in headers or body, rather than logging user information.

@bodawei

This comment has been minimized.

Copy link
Contributor Author

@bodawei bodawei commented Jul 20, 2017

Oh, definitely, some events don't have a request. I'm speaking only of the ones that do.

As for the security concern, I agree with you that logging user information isn't desirable! In our particular case, we've got a trace id, that is an id that identifies a particular request that we want to be able to track as it runs across services in our system. That's carried in a particular HTTP header. So, if an error occurs, we'd like to be able to log that ID along with the error information so we know which request it happened with. The problem is just that those headers are not accessible from within the value generated by, say RequestError .

bodawei pushed a commit to bodawei/contrib-good that referenced this issue Jul 20, 2017
@bodawei

This comment has been minimized.

Copy link
Contributor Author

@bodawei bodawei commented Jul 20, 2017

@arb fwiw, this is roughly what I have in mind (note this is just a draft... no testing or anything yet. so don't take it too seriously. I'm just demonstrating with code something like what I am suggesting:

bodawei@7666ddd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.