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

print *all* HTTP requests to log if configured #1550

Merged
merged 1 commit into from
Jul 28, 2015

Conversation

himanshug
Copy link
Contributor

I understand that there is, druid.request.logging.type, which does not look general enough to print all http requests received by server but mainly targeted towards printing query requests.

@drcrallen
Copy link
Contributor

would it make sense to have that setting be http log level, and can be default to debug or something else that isn't on by default?

@himanshug
Copy link
Contributor Author

@drcrallen that would be ok as well, so basically we write our own Slf4jRequestLog (maybe just extend it) and do logger.degug(..) instead of logger.info(..) instead of having a config in ServerConfig?
another thing I thought (but dint do) was to put POST body also in the log . Jetty implementation doesn't log the request body.

@@ -32,6 +32,9 @@
private int numThreads = Math.max(10, (Runtime.getRuntime().availableProcessors() * 17) / 16 + 2) + 30;

@JsonProperty
private boolean enableHttpRequestLogging = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we document the config?

@drcrallen
Copy link
Contributor

@himanshug : I was thinking just use our normal logging implementation, then if you're needing http debug logs you can enable debug logs in your log4j2.xml for only that class.

@himanshug himanshug force-pushed the optionally_log_all_requests branch from ce8e098 to 96cefc9 Compare July 25, 2015 21:43
@himanshug
Copy link
Contributor Author

@drcrallen removed the new ServerConfig setting and doing log.debug(..) instead
@fjy updated the doc about setting "io.druid.jetty.RequestLog" to debug level for printing request logs.

@himanshug
Copy link
Contributor Author

@fjy @drcrallen can you please see and thumbs-up/merge if there are no further comments?

@fjy
Copy link
Contributor

fjy commented Jul 28, 2015

👍

@drcrallen
Copy link
Contributor

👍 Looks very nice. With log4j2 you should be able to open up a jconsole to the jvm with the log4j2 plugin and enable logging dynamically if you wanted to be really fancy on the operational side of this feature.

@himanshug Do you want to squash?

@himanshug himanshug force-pushed the optionally_log_all_requests branch from 96cefc9 to 90b4759 Compare July 28, 2015 17:56
@himanshug
Copy link
Contributor Author

@drcrallen squashed

drcrallen added a commit that referenced this pull request Jul 28, 2015
print *all* HTTP requests to log if configured
@drcrallen drcrallen merged commit ba59f8a into apache:master Jul 28, 2015
@himanshug himanshug deleted the optionally_log_all_requests branch August 21, 2015 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants