vy commented on code in PR #2691:
URL: https://github.com/apache/logging-log4j2/pull/2691#discussion_r1654740592
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java:
##########
@@ -295,12 +295,35 @@ public String getCauseStackTraceAsString(
final List<String> ignorePackages,
final TextRenderer textRenderer,
final String suffix,
- final String lineSeparator) {
+ final String lineSeparator,
+ final Integer linesToKeep) {
final StringBuilder sb = new StringBuilder();
- ThrowableProxyRenderer.formatCauseStackTrace(this, sb, ignorePackages,
textRenderer, suffix, lineSeparator);
+ ThrowableProxyRenderer.formatCauseStackTraceTo(
+ this, sb, ignorePackages, textRenderer, suffix, lineSeparator,
linesToKeep);
return sb.toString();
}
+ /**
+ * Formats the stack trace with cause exception.
+ *
+ * @param sb Destination.
+ * @param ignorePackages List of packages to be ignored in the trace.
+ * @param textRenderer The message renderer.
+ * @param suffix Append this to the end of each stack frame.
+ * @param lineSeparator The end-of-line separator.
+ * @param linesToKeep The total line count of final result
+ */
+ public void formatCauseStackTraceTo(
Review Comment:
@alan0428a, as @ppkarwasz indicated, first you need to remove your changes
breaking the backward compatibility of existing methods – this is why
`bnd-baseline` is requiring a major version bump, you're breaking backward
compatibility. Instead,
1. Add new methods with new arguments
2. Redirect old methods to these new methods (pass the new arguments as
`null`, etc.)
3. Migrate existing code using old methods to new methods
4. Mark old methods as `@Deprecated`
5. Bump the minor version in the `package-info.java`
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxyRenderer.java:
##########
@@ -309,4 +311,24 @@ private static void renderOn(
textRenderer.render(msg, output, "Message");
}
}
+
+ private static void truncateLines(
+ StringBuilder sb, String lineSeparator, Integer linesToKeep,
TextRenderer textRenderer) {
+ if (linesToKeep == null) {
+ return;
+ }
+
+ String content = sb.toString();
+ String[] lines = content.split(lineSeparator);
+
+ if (lines.length <= linesToKeep) {
+ return;
+ }
+
+ sb.setLength(0);
+ for (int i = 0; i < linesToKeep; i++) {
+ sb.append(lines[i]);
+ textRenderer.render(lineSeparator, sb, "Text");
+ }
Review Comment:
No, we don't need to limit `lineSeparator` to a single character and we
don't need to implement KMP. You can use `StringBuilder#indexOf(String)` method.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverter.java:
##########
@@ -235,4 +235,69 @@ protected String getSuffix(final LogEvent event) {
public ThrowableFormatOptions getOptions() {
return options;
}
+
+ private boolean requireAdditionalFormatting(String suffix) {
+ return !options.allLines() || nonStandardLineSeparator ||
Strings.isNotBlank(suffix) || options.hasPackages();
+ }
+
+ private boolean ignoreElement(final StackTraceElement element, final
List<String> ignorePackages) {
+ if (ignorePackages != null) {
+ final String className = element.getClassName();
+ for (final String pkg : ignorePackages) {
+ if (className.startsWith(pkg)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private void appendSuppressedCount(
+ final StringBuilder sb, final int count, final String suffix,
final String lineSeparator) {
+ if (count == 1) {
+ sb.append("\t... ");
+ } else {
+ sb.append("\t... suppressed ");
+ sb.append(count);
+ sb.append(" lines");
+ }
+ appendSuffix(sb, suffix);
+ sb.append(lineSeparator);
+ }
+
+ private void appendEntry(
+ final StackTraceElement stackTraceElement,
+ final StringBuilder sb,
+ final String suffix,
+ final String lineSeparator) {
+ sb.append(stackTraceElement.toString());
+ appendSuffix(sb, suffix);
+ sb.append(lineSeparator);
+ }
+
+ private void appendSuffix(StringBuilder buffer, String suffix) {
+ if (Strings.isNotBlank(suffix)) {
+ buffer.append(' ');
+ buffer.append(suffix);
+ }
+ }
+
+ private void truncateLines(StringBuilder sb, String lineSeparator, Integer
linesToKeep) {
Review Comment:
No, please don't touch to the API. You need to create a new class in
`log4j-core`. Thinking about it, better we make it explicit that it is intended
for internal usage: Place the class to
`org.apache.logging.log4j.core.util.internal` package in `log4j-core`.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxyRenderer.java:
##########
@@ -309,4 +311,24 @@ private static void renderOn(
textRenderer.render(msg, output, "Message");
}
}
+
+ private static void truncateLines(
+ StringBuilder sb, String lineSeparator, Integer linesToKeep,
TextRenderer textRenderer) {
Review Comment:
I still see it in several other places: in `truncateLines()` methods,
variables in changes to `ThrowablePatternConverter`, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
