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
add logs with request latency for json readData/metaData API #749
base: master
Are you sure you want to change the base?
Conversation
/gcbrun |
@@ -239,11 +246,26 @@ private GoogleCloudStorageItemInfo fetchInitialMetadata() throws IOException { | |||
RetryDeterminer.SOCKET_ERRORS, | |||
IOException.class, | |||
sleeper); | |||
|
|||
logger.atFinest().log( | |||
"GoogleCloudStorageReadChannel:getMetadata complete context:%d,time:%d,resource:%s,requestId:%s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need to log the class name GoogleCloudStorageReadChannel
logging pattern have the fully qualified class name
? createFileNotFoundException(resourceId, e) | ||
: new IOException("Error reading " + resourceId, e); | ||
if (errorExtractor.itemNotFound(e)) { | ||
throw createFileNotFoundException(resourceId, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add a log here as well
@@ -995,13 +1031,21 @@ protected InputStream openStream(long bytesToRead) throws IOException { | |||
cacheFooter(response); | |||
if (retriesCount != 0) { | |||
logger.atInfo().log( | |||
"Successfully cached footer after %s retries for '%s'", retriesCount, resourceId); | |||
"Successfully cached footer context:%d,retries:%s,time:%d,resource:%s", | |||
Thread.currentThread().getId(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to log the threadId, it should already be part of the logging pattern (thread Id/name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve merge conflicts
No description provided.