Reduce noisy truncation logging #1968
Conversation
| maxLength, | ||
| trimTo80(value)); | ||
| } | ||
| logger.debug( |
There was a problem hiding this comment.
should this in the else block? without it, it will get log twice, warn and debug for the very first time.
There was a problem hiding this comment.
the debug message has more detail (non-trimmed value)
There was a problem hiding this comment.
in that case, do we need to trim in warn logger level?
since it's only logging once, i think it's fine to show details.
There was a problem hiding this comment.
I think logging an (expected) sql text > 8192 characters (like in #1964) could still be annoying enough for users to ask to suppress it (trying to make this warning not be so bad for the expected cases)
| maxLength, | ||
| trimTo80(value)); | ||
| } | ||
| logger.debug("truncated {} property value to {} characters: {}", propertyKey, maxLength, value); |
| if (!alreadyLoggedAttributeNames.add(attributeName)) { | ||
| // this can be expected, so don't want to flood the logs with a lot of these | ||
| // (and don't want to log the full value, e.g. sql text > 8192 characters) | ||
| logger.warn( |
There was a problem hiding this comment.
should we continue to use the AggregatingLogger? it will log for the first 5 mins..
i think aggregation is still helpful for other customers.. when truncation happens occasionally.
There was a problem hiding this comment.
i'm not sure i follow "it will log for the first 5 mins"?
There was a problem hiding this comment.
it's using a scheduler to run every 5 mins. it's not possible to log once using aggregated logger unless to stop the scheduler, but we don't have any condition to stop it.. i see why switch from aggregated logger to regular logger.
| warningLogger.recordWarning("truncated property[" + key + "]: " + value); | ||
| if (alreadyLoggedPropertyKeys.size() < 10 && !alreadyLoggedPropertyKeys.add(propertyKey)) { | ||
| // this can be expected, so don't want to flood the logs with a lot of these | ||
| logger.warn( |
There was a problem hiding this comment.
this is almost identical to line 43. "property" and "attribute" are the only difference.
should we reword it a bit to make it reusable in both cases.
There was a problem hiding this comment.
ya, i tried a few variations, but ended up opting for clearer messages/code, with slight duplication
No description provided.