Add more logging for client and cache #656
Conversation
| @@ -502,6 +503,8 @@ class DeltaSharingRestClient( | |||
| target = target, data = request, setIncludeEndStreamAction = endStreamActionEnabled | |||
| ) | |||
| val (filteredLines, _) = maybeExtractEndStreamAction(response.lines) | |||
| logInfo(s"Took ${System.currentTimeMillis() - start} ms to query ${filteredLines.size} " + | |||
There was a problem hiding this comment.
If the goal is "better debug ability" rather than normal event logging, should these be on debug level?
There was a problem hiding this comment.
@yinouya-db I'm not sure how to enable debug level logs in DBR, and I wonder we are only able to obtain logs after the enablement so that the logs for the query that's hitting an issue is not available?
There was a problem hiding this comment.
These stats seem like general information about a query execution. It would be nice to have them in the log.
The debug level would be a lengthy process though. We need to turn on debug, run repro or ask customer to do it, then check log again.
| @@ -103,7 +103,7 @@ class CachedTableManager( | |||
| cache.remove(tablePath, cachedTable) | |||
| } else if (cachedTable.expiration - System.currentTimeMillis() < refreshThresholdMs) { | |||
| logInfo(s"Updating pre signed urls for $tablePath (expiration time: " + | |||
| s"${new java.util.Date(cachedTable.expiration)})") | |||
| s"${new java.util.Date(cachedTable.expiration)}), token:${cachedTable.refreshToken}") | |||
There was a problem hiding this comment.
Is refreshToken sensitive or it is known to recipient in memory/disk? Since you can print it out here, I guess it already known to recipient.
There was a problem hiding this comment.
no, it's an encrypted token and yes it's known to the recipient
Add more logging for client and cache for better debug ability.