Open byshen opened 3 years ago
Your pull requests add logging, but at this moment I wonder, does it add any information? What can you do with seeing for example this message?
Currently, it first checks whether the connection's IP is allowed. If not, it will simply return an access-denied 403 error code. (If there is any other information, please let me know :D )
However, there are multiple checks in the phase_setup_connection
phase. It would be hard to know why the connection fails with a 403. With the new information, it clearly points the admins to the IP related configs.
But wouldn't it be much more informative to have such information as part of the normal http logging? So we directly know which IP was not allowed? Maybe I am missing something.
Sorry for the late reply, I was swamped by other stuff last week ..
But wouldn't it be much more informative to have such information as part of the normal http logging?
The normal logging in cherokee access log
already logs the information like IP and status, but it does not have other information in the error log. From the 403 status we know it is denied, but we don't know if it's because of the IP. If the error log has hints like an error message, people would clearly know that.
So we directly know which IP was not allowed?
I can also add the IP address to the log if that's preferred.
Thanks!
I can also add the IP address to the log if that's preferred.
My main question was why should we do this in two places. Both access log and error log. I do want to prevent that the system becomes bloated because of that, because it could become noisy. Now if I review the apache error logs, I notice that a log level defines what and when goes through the error log, by a format one can add all the information to make the error logs useful. Something we only have when tracing. I think we should have a chat on what to do with these specific errors, in my perspective they are not defects of the regular webserver processing.
they are not defects of the regular webserver processing.
I agree, these errors are expected based on the current webserver configuration, but it could be difficult to troubleshoot if the configuration is not intended, i.e., we want to understand why it is denied.
a log level defines what and when goes through the error log, by a format one can add all the information to make the error logs useful. Something we only have when tracing
Ah I got it. I add it as error level log because in Apache httpd, these configuration errors are logged at ERROR
level. For example, one example here. I don't feel that will make the log bloated, because most requests do not result in a 403 status and will not be logged.
In my perspective, TRACE
level logs are also helpful, but they require a re-compile and re-install process, also people may not know there are relevant information at trace level.
If you feel log at TRACE
is more appropriate, I can submit another patch at trace level :D
If you feel log at
TRACE
is more appropriate, I can submit another patch at trace level :D
No, I don't. I think that the regular logs should have an option to tell why the error is present. So we should explore that, unless you have another idea.
the regular logs should have an option to tell why the error is present
Currently, the access logs are mostly used in the Apache format, as recommended here. There is no field for us to place the error msg in it. The other two access log formats also does not support it.
I think the access log does not record the reason of error, because the access logs are produced in the end of handling requests (e.g., purge_connection
in cherokee), which is usually far away from where the error happens.
If we want to record that in the access log, we need to add one additional field in the cherokee_connection_t
to record this error msg when the error happens, and log the msg when close the connection.
Another approach is to correlate the access log and error log with some log entry id for each connection. For example, %L
can be enabled in apache log to correlate the entry in access log and error log, as specified here. In this way, we can just add some options for error log to control the verbosity of the error log. People can correlate the regular log and error log too.
I am happy to help implement this (may add a proposal in the issue). Let me know your thoughts, thanks 😊
Before return access-denied, leave a message in the server log about the specific reason.