Add trace log of the request for the query and fetch phases #34479
Conversation
This query adds a logger for the query and fetch phases that prints all requests before their execution at the trace level. This will help debugging cases where an issue occurs during the execution since only completed queries are logged by the slow logs.
jimczi added >non-issue :Search/Search 
| Pinging @elastic/es-search-aggs |
romseygeek left a comment
romseygeek left a comment There was a problem hiding this comment.
LGTM. It would be nice to be able to associate an ID with the query to match up query and fetch phases, but I guess that can be done as part of the query body itself?
Good idea, I added the |
danielmitterdorfer left a comment
danielmitterdorfer left a comment There was a problem hiding this comment.
Thanks for doing this! I left a few comments on how to avoid some overhead.
| */ | ||
| void execute(SearchContext context); | ||
| | ||
| class SearchContextRequestLog { |
There was a problem hiding this comment.
I think this can be declared as static to avoid capturing the enclosing class' state? Further looking at the usage, I wonder whether this should actually be a static method to avoid creating garbage for every query?
There was a problem hiding this comment.
This declaration is within an interface so the static modifier is redundant, this is a static nested class already.
There was a problem hiding this comment.
Ah, good point. I did not check that.
| @Override | ||
| public void execute(SearchContext context) { | ||
| | ||
| LOGGER.trace("{}", new SearchContextRequestLog(context)); |
There was a problem hiding this comment.
It might make sense to guard this log statement? (i.e. if LOGGER.isTraceEnabled())?)
There was a problem hiding this comment.
++, this will avoid the creation of a new object on each query
| return; | ||
| } | ||
| | ||
| LOGGER.trace("{}", new SearchContextRequestLog(searchContext)); |
There was a problem hiding this comment.
It might make sense to guard this log statement? (i.e. if LOGGER.isTraceEnabled())?)
danielmitterdorfer left a comment
danielmitterdorfer left a comment There was a problem hiding this comment.
Thanks for iterating. LGTM!
This change adds a logger for the query and fetch phases that prints all requests before their execution at the trace level. This will help debugging cases where an issue occurs during the execution since only completed queries are logged by the slow logs.
5 participants
This change adds a logger for the query and fetch phases that prints all requests
before their execution at the trace level. This will help debugging cases where an issue
occurs during the execution since only completed queries are logged by the slow logs.