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

logging of request headers in decision logs #5849

Closed
benhass opened this issue Apr 19, 2023 · 4 comments
Closed

logging of request headers in decision logs #5849

benhass opened this issue Apr 19, 2023 · 4 comments

Comments

@benhass
Copy link

benhass commented Apr 19, 2023

What is the underlying problem you're trying to solve?

Decision logs could include header fields from the request. This would improve the ability to understand decisions and other issues in production, where debug logging is not appropriate. I looked at system.log.mask upsert functionality but did not have access to input.headers there. Possibly a change similar to #1456 could address that limitation.

Describe the ideal solution

Ideally, logging of request headers in decision logs would be configurable. The user could decide which headers to mask using the existing system.log.mask rule.

Describe a "Good Enough" solution

Enable access to input.headers when defining rules for system.log.mask and use the existing upsert functionality to modify the decision log body.

Additional Context

@srenatus
Copy link
Contributor

Traditionally decision logs record the inputs and results of policy decisions. I'm not sure I understand how the incoming request headers, which aren't available in the policy evaluation, can impact the decision making. If they can't, they don't belong into the decision logs, I think.

This does not mean that there's no value in improving tracing, and especially giving your decision logs a reference to tracing spans, like done recently in #5230. Would that functionality help you in debugging production issues, too?

@benhass
Copy link
Author

benhass commented Apr 20, 2023

especially giving your decision logs a reference to tracing spans

Yup! This is exactly why I'm requesting it. It sounds like #5230 is specific to the envoy plugin, is that right? I can think of some other use-cases where we might want to log out headers that tells us about the client or other general request metadata, but I'm happy enough if it is focused on the tracing use cases.

@ashutosh-narkar
Copy link
Member

We've updated OPA's decision log format to now include the trace and span identifier. So it's not specific to the envoy plugin.

@benhass
Copy link
Author

benhass commented Apr 21, 2023

I'm not sure I understand how the incoming request headers, which aren't available in the policy evaluation, can impact the decision making

@srenatus they are used in system.authz so they might have some impact on access to opa, but I get your point and I understand it may not be optimal to include headers for many users.

I am happy to know you've likely addressed my main use-case, so will look forward to testing out the next release. Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants