-
-
Notifications
You must be signed in to change notification settings - Fork 50
Suggested changes #3577
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
Suggested changes #3577
Conversation
since this is replacing the request middleware, if it’s disabled there will be no middleware logging requests. I’m assuming that’s needed for logs, so we might want to revise this down the road to replacing or not in config on the env variable, rather than enabling or disabling in the middleware. Hard to read on an iphone, but i think that’s what I’m seeing. |
Oh, right. I thought by forwarding to the next responder it'd be a no-op but now I recall that you're replacing the default middleware, don't you? I'll test the impact on dev after I'm done with the redis stuff. |
…lly restore original behaviour when off
I've simply moved the |
d3e3666
to
5862cf5
Compare
Also, since we've got redis now I wonder if we actually want to send the CFRay headers to the logs rather than straight into redis. Might be easier to extract? |
I wanted to start with it just going into logging so that I could see exactly how clustered normal usage was when viewed through the aggregate lens of a cf-ray identifier. I know it's broader than an IP address, but don't have a really good sense, so I'm hoping to getting logging kicking earlier and visible to see how things aggregate in production prior to assembling any rate limiter - either classic mode (429 responses and refusal) or honeypot slow-downs. I had no idea which way would be easier to extract - just straight in the logs, or tossed into the log metadata. I'm totally game either way. |
Allow CFRay logging based on env variable
ENABLE_CF_RAY_LOGGING
is set totrue
in dev and prod now.