validation: Update a couple debug logs that can be large with large p… #7035
Conversation
| reports, err := v.getReportsFromGlooValidationResponse(response) | ||
| // reports are gauranteed to not be nil at this point and we can more easily not report on the proxy | ||
| // than on the response itself without more loops on non-debug calls | ||
| logger.Debugf("Got response from GlooValidationService: UpstreamReports: %v, ProxyReports: %v", *reports.UpstreamReports, *reports.ProxyReports) |
There was a problem hiding this comment.
This might be too paranoid but given that we've also seen very large reports/statuses on proxies maybe we should only log the state and/or move all the proxy contents logging to "trace"
There was a problem hiding this comment.
Yeah so even proxyreports could be super large. Ill check it out
There was a problem hiding this comment.
Also, will this construct the (potentially large) formatted string regardless of whether we're logging at debug level? Is this a case where we'd we'd want to wrap in if debugEnabled (maybe go's logger is smarter than log4j, I'm not sure)
There was a problem hiding this comment.
zap is smart enough to check that as the first statement prior to the execution checks the not exposed level. Given that I didnt want to make a loop gated by level as it wouldnt be pulling from the logger itself.
|
Visit the preview URL for this PR (updated for commit c1c4c8b): https://gloo-edge--pr7035-feat-vwh-deublogging-9kjl497l.web.app (expires Fri, 02 Sep 2022 15:37:21 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
|
Issues linked to changelog: |
There was a problem hiding this comment.
Each of the validator methods seem to have a debug log line where we log the entire response (ValidateDeleteSecret: logger.Debugf("Got response from GlooValidationService: %s", response.String())). Do we need to update these instances as well?
Side note: Given the ease of using the String method for the ValidationRequest and ValidationResponse, would it make sense to implementer some stringer method on those objects which can always be called and contain only the relevant details?
|
The problem is that it is the stringer and the appropriate small subobjects portion would have a decent loop overhead. We can address that at a later point. |
|
/kick [Fail] Access Log in memory Access Logs File [It] can create string access logs |
#7035) * validation: Update a couple debug logs that can be large with large proxies * changelog: Add issue link * validation: Log level changes to avoid more potentially large objects * changelog * validation: Last logs in validation service that contain potentially large objects removed * Adding changelog file to new location * Deleting changelog file from old location Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: changelog-bot <changelog-bot>
#7035) * validation: Update a couple debug logs that can be large with large proxies * changelog: Add issue link * validation: Log level changes to avoid more potentially large objects * changelog * validation: Last logs in validation service that contain potentially large objects removed * Adding changelog file to new location * Deleting changelog file from old location Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: changelog-bot <changelog-bot>
#7035) * validation: Update a couple debug logs that can be large with large proxies * changelog: Add issue link * validation: Log level changes to avoid more potentially large objects * changelog * validation: Last logs in validation service that contain potentially large objects removed * Adding changelog file to new location * Deleting changelog file from old location Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: changelog-bot <changelog-bot>
Description
Updates calls to logger debug that now had proxy in the log response post merge.
This previously could cause issues in environments with large proxies that had debug enabled post gateway merge especially if they have several validation requests in process.