Repository: logging-log4j2 Updated Branches: refs/heads/release-2.x b06395221 -> 0cd480ff5
[LOG4J2-2509] Allow a JDBC Appender to truncate strings to match a table's metadata column length limit. Not documenting truncateStrings for now since it might make sense to always have it on. Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/0cd480ff Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/0cd480ff Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/0cd480ff Branch: refs/heads/release-2.x Commit: 0cd480ff5c3654e411d42f8ce34c1cf34e43d9cc Parents: b063952 Author: Gary Gregory <garydgreg...@gmail.com> Authored: Fri Nov 16 16:55:11 2018 -0700 Committer: Gary Gregory <garydgreg...@gmail.com> Committed: Fri Nov 16 16:55:11 2018 -0700 ---------------------------------------------------------------------- .../log4j/core/appender/db/ColumnMapping.java | 12 + .../core/appender/db/jdbc/ColumnConfig.java | 274 ++++++++-------- .../core/appender/db/jdbc/JdbcAppender.java | 31 +- .../appender/db/jdbc/JdbcDatabaseManager.java | 328 +++++++++++++++---- .../AbstractJdbcAppenderFactoryMethodTest.java | 17 +- .../JdbcAppenderMapMessageDataSourceTest.java | 27 +- .../src/test/resources/log4j2-jdbc-dbcp2.xml | 4 + src/changes/changes.xml | 3 + 8 files changed, 490 insertions(+), 206 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/ColumnMapping.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/ColumnMapping.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/ColumnMapping.java index 1178156..1219060 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/ColumnMapping.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/ColumnMapping.java @@ -17,6 +17,7 @@ package org.apache.logging.log4j.core.appender.db; import java.util.Date; +import java.util.Locale; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.Core; @@ -188,20 +189,27 @@ public class ColumnMapping { } private static final Logger LOGGER = StatusLogger.getLogger(); + @PluginBuilderFactory public static Builder newBuilder() { return new Builder(); } + public static String toKey(final String name) { + return name.toUpperCase(Locale.ROOT); + } + private final StringLayout layout; private final String literalValue; private final String name; + private final String nameKey; private final String parameter; private final String source; private final Class<?> type; private ColumnMapping(final String name, final String source, final StringLayout layout, final String literalValue, final String parameter, final Class<?> type) { this.name = name; + this.nameKey = toKey(name); this.source = source; this.layout = layout; this.literalValue = literalValue; @@ -221,6 +229,10 @@ public class ColumnMapping { return name; } + public String getNameKey() { + return nameKey; + } + public String getParameter() { return parameter; } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/ColumnConfig.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/ColumnConfig.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/ColumnConfig.java index ba0ad25..6dc6735 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/ColumnConfig.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/ColumnConfig.java @@ -37,90 +37,6 @@ import org.apache.logging.log4j.util.Strings; */ @Plugin(name = "Column", category = Core.CATEGORY_NAME, printObject = true) public final class ColumnConfig { - private static final Logger LOGGER = StatusLogger.getLogger(); - - private final String columnName; - private final PatternLayout layout; - private final String literalValue; - private final boolean eventTimestamp; - private final boolean unicode; - private final boolean clob; - - private ColumnConfig(final String columnName, final PatternLayout layout, final String literalValue, - final boolean eventDate, final boolean unicode, final boolean clob) { - this.columnName = columnName; - this.layout = layout; - this.literalValue = literalValue; - this.eventTimestamp = eventDate; - this.unicode = unicode; - this.clob = clob; - } - - public String getColumnName() { - return this.columnName; - } - - public PatternLayout getLayout() { - return this.layout; - } - - public String getLiteralValue() { - return this.literalValue; - } - - public boolean isEventTimestamp() { - return this.eventTimestamp; - } - - public boolean isUnicode() { - return this.unicode; - } - - public boolean isClob() { - return this.clob; - } - - @Override - public String toString() { - return "{ name=" + this.columnName + ", layout=" + this.layout + ", literal=" + this.literalValue - + ", timestamp=" + this.eventTimestamp + " }"; - } - - /** - * Factory method for creating a column config within the plugin manager. - * - * @see Builder - * @deprecated use {@link #newBuilder()} - */ - @Deprecated - public static ColumnConfig createColumnConfig(final Configuration config, final String name, final String pattern, - final String literalValue, final String eventTimestamp, - final String unicode, final String clob) { - if (Strings.isEmpty(name)) { - LOGGER.error("The column config is not valid because it does not contain a column name."); - return null; - } - - final boolean isEventTimestamp = Boolean.parseBoolean(eventTimestamp); - final boolean isUnicode = Booleans.parseBoolean(unicode, true); - final boolean isClob = Boolean.parseBoolean(clob); - - return newBuilder() - .setConfiguration(config) - .setName(name) - .setPattern(pattern) - .setLiteral(literalValue) - .setEventTimestamp(isEventTimestamp) - .setUnicode(isUnicode) - .setClob(isClob) - .build(); - } - - @PluginBuilderFactory - public static Builder newBuilder() { - return new Builder(); - } - public static class Builder implements org.apache.logging.log4j.core.util.Builder<ColumnConfig> { @PluginConfiguration @@ -145,34 +61,71 @@ public final class ColumnConfig { @PluginBuilderAttribute private boolean isClob; + @Override + public ColumnConfig build() { + if (Strings.isEmpty(name)) { + LOGGER.error("The column config is not valid because it does not contain a column name."); + return null; + } + + final boolean isPattern = Strings.isNotEmpty(pattern); + final boolean isLiteralValue = Strings.isNotEmpty(literal); + + if ((isPattern && isLiteralValue) || (isPattern && isEventTimestamp) || (isLiteralValue && isEventTimestamp)) { + LOGGER.error("The pattern, literal, and isEventTimestamp attributes are mutually exclusive."); + return null; + } + + if (isEventTimestamp) { + return new ColumnConfig(name, null, null, true, false, false); + } + + if (isLiteralValue) { + return new ColumnConfig(name, null, literal, false, false, false); + } + + if (isPattern) { + final PatternLayout layout = + PatternLayout.newBuilder() + .withPattern(pattern) + .withConfiguration(configuration) + .withAlwaysWriteExceptions(false) + .build(); + return new ColumnConfig(name, layout, null, false, isUnicode, isClob); + } + + LOGGER.error("To configure a column you must specify a pattern or literal or set isEventDate to true."); + return null; + } + /** - * The configuration object. + * If {@code "true"}, indicates that the column is a character LOB (CLOB). * * @return this. */ - public Builder setConfiguration(final Configuration configuration) { - this.configuration = configuration; + public Builder setClob(final boolean clob) { + isClob = clob; return this; } /** - * The name of the database column as it exists within the database table. + * The configuration object. * * @return this. */ - public Builder setName(final String name) { - this.name = name; + public Builder setConfiguration(final Configuration configuration) { + this.configuration = configuration; return this; } /** - * The {@link PatternLayout} pattern to insert in this column. Mutually exclusive with - * {@code literal!=null} and {@code eventTimestamp=true} + * If {@code "true"}, indicates that this column is a date-time column in which the event timestamp should be + * inserted. Mutually exclusive with {@code pattern!=null} and {@code literal!=null}. * * @return this. */ - public Builder setPattern(final String pattern) { - this.pattern = pattern; + public Builder setEventTimestamp(final boolean eventTimestamp) { + isEventTimestamp = eventTimestamp; return this; } @@ -188,71 +141,124 @@ public final class ColumnConfig { } /** - * If {@code "true"}, indicates that this column is a date-time column in which the event timestamp should be - * inserted. Mutually exclusive with {@code pattern!=null} and {@code literal!=null}. + * The name of the database column as it exists within the database table. * * @return this. */ - public Builder setEventTimestamp(final boolean eventTimestamp) { - isEventTimestamp = eventTimestamp; + public Builder setName(final String name) { + this.name = name; return this; } /** - * If {@code "true"}, indicates that the column is a Unicode String. + * The {@link PatternLayout} pattern to insert in this column. Mutually exclusive with + * {@code literal!=null} and {@code eventTimestamp=true} * * @return this. */ - public Builder setUnicode(final boolean unicode) { - isUnicode = unicode; + public Builder setPattern(final String pattern) { + this.pattern = pattern; return this; } /** - * If {@code "true"}, indicates that the column is a character LOB (CLOB). + * If {@code "true"}, indicates that the column is a Unicode String. * * @return this. */ - public Builder setClob(final boolean clob) { - isClob = clob; + public Builder setUnicode(final boolean unicode) { + isUnicode = unicode; return this; } + } - @Override - public ColumnConfig build() { - if (Strings.isEmpty(name)) { - LOGGER.error("The column config is not valid because it does not contain a column name."); - return null; - } + private static final Logger LOGGER = StatusLogger.getLogger(); + /** + * Factory method for creating a column config within the plugin manager. + * + * @see Builder + * @deprecated use {@link #newBuilder()} + */ + @Deprecated + public static ColumnConfig createColumnConfig(final Configuration config, final String name, final String pattern, + final String literalValue, final String eventTimestamp, + final String unicode, final String clob) { + if (Strings.isEmpty(name)) { + LOGGER.error("The column config is not valid because it does not contain a column name."); + return null; + } - final boolean isPattern = Strings.isNotEmpty(pattern); - final boolean isLiteralValue = Strings.isNotEmpty(literal); + final boolean isEventTimestamp = Boolean.parseBoolean(eventTimestamp); + final boolean isUnicode = Booleans.parseBoolean(unicode, true); + final boolean isClob = Boolean.parseBoolean(clob); - if ((isPattern && isLiteralValue) || (isPattern && isEventTimestamp) || (isLiteralValue && isEventTimestamp)) { - LOGGER.error("The pattern, literal, and isEventTimestamp attributes are mutually exclusive."); - return null; - } + return newBuilder() + .setConfiguration(config) + .setName(name) + .setPattern(pattern) + .setLiteral(literalValue) + .setEventTimestamp(isEventTimestamp) + .setUnicode(isUnicode) + .setClob(isClob) + .build(); + } + @PluginBuilderFactory + public static Builder newBuilder() { + return new Builder(); + } + private final String columnName; + private final String columnNameKey; + private final PatternLayout layout; + private final String literalValue; - if (isEventTimestamp) { - return new ColumnConfig(name, null, null, true, false, false); - } + private final boolean eventTimestamp; - if (isLiteralValue) { - return new ColumnConfig(name, null, literal, false, false, false); - } + private final boolean unicode; - if (isPattern) { - final PatternLayout layout = - PatternLayout.newBuilder() - .withPattern(pattern) - .withConfiguration(configuration) - .withAlwaysWriteExceptions(false) - .build(); - return new ColumnConfig(name, layout, null, false, isUnicode, isClob); - } + private final boolean clob; - LOGGER.error("To configure a column you must specify a pattern or literal or set isEventDate to true."); - return null; - } + private ColumnConfig(final String columnName, final PatternLayout layout, final String literalValue, + final boolean eventDate, final boolean unicode, final boolean clob) { + this.columnName = columnName; + this.columnNameKey = ColumnMapping.toKey(columnName); + this.layout = layout; + this.literalValue = literalValue; + this.eventTimestamp = eventDate; + this.unicode = unicode; + this.clob = clob; + } + + public String getColumnName() { + return this.columnName; + } + + public String getColumnNameKey() { + return this.columnNameKey; + } + + public PatternLayout getLayout() { + return this.layout; + } + + public String getLiteralValue() { + return this.literalValue; + } + + public boolean isClob() { + return this.clob; + } + + public boolean isEventTimestamp() { + return this.eventTimestamp; + } + + public boolean isUnicode() { + return this.unicode; + } + + @Override + public String toString() { + return "{ name=" + this.columnName + ", layout=" + this.layout + ", literal=" + this.literalValue + + ", timestamp=" + this.eventTimestamp + " }"; } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppender.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppender.java index ff801e0..7a29dc7 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppender.java @@ -62,7 +62,7 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan @PluginBuilderAttribute private boolean immediateFail; - + @PluginBuilderAttribute private int bufferSize; @@ -76,6 +76,9 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan @PluginElement("ColumnMappings") private ColumnMapping[] columnMappings; + @PluginBuilderAttribute + private boolean truncateStrings = true; + // TODO Consider moving up to AbstractDatabaseAppender.Builder. @PluginBuilderAttribute private long reconnectIntervalMillis = DEFAULT_RECONNECT_INTERVAL_MILLIS; @@ -90,7 +93,8 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan + tableName + ", columnConfigs=" + Arrays.toString(columnConfigs) + ", columnMappings=" + Arrays.toString(columnMappings) + '}'; final JdbcDatabaseManager manager = JdbcDatabaseManager.getManager(managerName, bufferSize, getLayout(), - connectionSource, tableName, columnConfigs, columnMappings, immediateFail, reconnectIntervalMillis); + connectionSource, tableName, columnConfigs, columnMappings, immediateFail, reconnectIntervalMillis, + truncateStrings); if (manager == null) { return null; } @@ -109,8 +113,8 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan /** * If an integer greater than 0, this causes the appender to buffer log events and flush whenever the buffer * reaches this size. - * - * @param bufferSize buffer size. + * + * @param bufferSize buffer size. * * @return this */ @@ -121,8 +125,8 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan /** * Information about the columns that log event data should be inserted into and how to insert that data. - * - * @param columnConfigs Column configurations. + * + * @param columnConfigs Column configurations. * * @return this */ @@ -138,7 +142,7 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan /** * The connections source from which database connections should be retrieved. - * + * * @param connectionSource The connections source. * * @return this @@ -148,18 +152,18 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan return asBuilder(); } - public void setImmediateFail(boolean immediateFail) { + public void setImmediateFail(final boolean immediateFail) { this.immediateFail = immediateFail; } - public void setReconnectIntervalMillis(long reconnectIntervalMillis) { + public void setReconnectIntervalMillis(final long reconnectIntervalMillis) { this.reconnectIntervalMillis = reconnectIntervalMillis; } /** * The name of the database table to insert log events into. - * - * @param tableName The database table name. + * + * @param tableName The database table name. * * @return this */ @@ -168,6 +172,11 @@ public final class JdbcAppender extends AbstractDatabaseAppender<JdbcDatabaseMan return asBuilder(); } + public B setTruncateStrings(final boolean truncateStrings) { + this.truncateStrings = truncateStrings; + return asBuilder(); + } + } /** http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java index 26130b4..b449272 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java @@ -23,13 +23,16 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.NClob; import java.sql.PreparedStatement; +import java.sql.ResultSetMetaData; import java.sql.SQLException; import java.sql.Timestamp; import java.sql.Types; import java.util.ArrayList; import java.util.Arrays; import java.util.Date; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.concurrent.CountDownLatch; @@ -69,10 +72,12 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { private final boolean immediateFail; private final boolean retry; private final long reconnectIntervalMillis; + private final boolean truncateStrings; protected FactoryData(final int bufferSize, final Layout<? extends Serializable> layout, final ConnectionSource connectionSource, final String tableName, final ColumnConfig[] columnConfigs, - final ColumnMapping[] columnMappings, final boolean immediateFail, final long reconnectIntervalMillis) { + final ColumnMapping[] columnMappings, final boolean immediateFail, final long reconnectIntervalMillis, + final boolean truncateStrings) { super(bufferSize, layout); this.connectionSource = connectionSource; this.tableName = tableName; @@ -81,14 +86,15 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { this.immediateFail = immediateFail; this.retry = reconnectIntervalMillis > 0; this.reconnectIntervalMillis = reconnectIntervalMillis; + this.truncateStrings = truncateStrings; } @Override public String toString() { return String.format( - "FactoryData [connectionSource=%s, tableName=%s, columnConfigs=%s, columnMappings=%s, immediateFail=%s, retry=%s, reconnectIntervalMillis=%,d]", + "FactoryData [connectionSource=%s, tableName=%s, columnConfigs=%s, columnMappings=%s, immediateFail=%s, retry=%s, reconnectIntervalMillis=%s, truncateStrings=%s]", connectionSource, tableName, Arrays.toString(columnConfigs), Arrays.toString(columnMappings), - immediateFail, retry, reconnectIntervalMillis); + immediateFail, retry, reconnectIntervalMillis, truncateStrings); } } @@ -101,25 +107,14 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { @Override public JdbcDatabaseManager createManager(final String name, final FactoryData data) { - final StringBuilder sb = new StringBuilder("INSERT INTO ").append(data.tableName).append(" ("); + final StringBuilder sb = new StringBuilder("insert into ").append(data.tableName).append(" ("); // so this gets a little more complicated now that there are two ways to configure column mappings, but // both mappings follow the same exact pattern for the prepared statement + appendColumnNames("INSERT", data, sb); + sb.append(") values ("); int i = 1; for (final ColumnMapping mapping : data.columnMappings) { final String mappingName = mapping.getName(); - logger().trace("Adding INSERT ColumnMapping[{}]: {}={} ", i++, mappingName, mapping); - sb.append(mappingName).append(','); - } - for (final ColumnConfig config : data.columnConfigs) { - sb.append(config.getColumnName()).append(','); - } - // at least one of those arrays is guaranteed to be non-empty - sb.setCharAt(sb.length() - 1, ')'); - sb.append(" VALUES ("); - i = 1; - final List<ColumnMapping> columnMappings = new ArrayList<>(data.columnMappings.length); - for (final ColumnMapping mapping : data.columnMappings) { - final String mappingName = mapping.getName(); if (Strings.isNotEmpty(mapping.getLiteralValue())) { logger().trace("Adding INSERT VALUES literal for ColumnMapping[{}]: {}={} ", i, mappingName, mapping.getLiteralValue()); @@ -128,12 +123,10 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { logger().trace("Adding INSERT VALUES parameter for ColumnMapping[{}]: {}={} ", i, mappingName, mapping.getParameter()); sb.append(mapping.getParameter()); - columnMappings.add(mapping); } else { logger().trace("Adding INSERT VALUES parameter marker for ColumnMapping[{}]: {}={} ", i, mappingName, PARAMETER_MARKER); sb.append(PARAMETER_MARKER); - columnMappings.add(mapping); } sb.append(','); i++; @@ -159,7 +152,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { /** * Handles reconnecting to JDBC once on a Thread. */ - private class Reconnector extends Log4jThread { + private final class Reconnector extends Log4jThread { private final CountDownLatch latch = new CountDownLatch(1); private volatile boolean shutdown = false; @@ -205,8 +198,161 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { } + private static final class ResultSetColumnMetaData { + + private final String schemaName; + private final String catalogName; + private final String tableName; + private final String name; + private final String nameKey; + private final String label; + private final int displaySize; + private final int type; + private final String typeName; + private final String className; + private final int precision; + private final int scale; + private final boolean isStringType; + + public ResultSetColumnMetaData(final ResultSetMetaData rsMetaData, final int j) throws SQLException { + // @formatter:off + this(rsMetaData.getSchemaName(j), + rsMetaData.getCatalogName(j), + rsMetaData.getTableName(j), + rsMetaData.getColumnName(j), + rsMetaData.getColumnLabel(j), + rsMetaData.getColumnDisplaySize(j), + rsMetaData.getColumnType(j), + rsMetaData.getColumnTypeName(j), + rsMetaData.getColumnClassName(j), + rsMetaData.getPrecision(j), + rsMetaData.getScale(j)); + // @formatter:on + } + + private ResultSetColumnMetaData(final String schemaName, final String catalogName, final String tableName, + final String name, final String label, final int displaySize, final int type, final String typeName, + final String className, final int precision, final int scale) { + super(); + this.schemaName = schemaName; + this.catalogName = catalogName; + this.tableName = tableName; + this.name = name; + this.nameKey = ColumnMapping.toKey(name); + this.label = label; + this.displaySize = displaySize; + this.type = type; + this.typeName = typeName; + this.className = className; + this.precision = precision; + this.scale = scale; + // TODO How about also using the className? + // @formatter:off + this.isStringType = + type == Types.CHAR || + type == Types.LONGNVARCHAR || + type == Types.LONGVARCHAR || + type == Types.NVARCHAR || + type == Types.VARCHAR; + // @formatter:on + } + + public String getCatalogName() { + return catalogName; + } + + public String getClassName() { + return className; + } + + public int getDisplaySize() { + return displaySize; + } + + public String getLabel() { + return label; + } + + public String getName() { + return name; + } + + public String getNameKey() { + return nameKey; + } + + public int getPrecision() { + return precision; + } + + public int getScale() { + return scale; + } + + public String getSchemaName() { + return schemaName; + } + + public String getTableName() { + return tableName; + } + + public int getType() { + return type; + } + + public String getTypeName() { + return typeName; + } + + public boolean isStringType() { + return this.isStringType; + } + + @Override + public String toString() { + return String.format( + "ColumnMetaData [schemaName=%s, catalogName=%s, tableName=%s, name=%s, nameKey=%s, label=%s, displaySize=%s, type=%s, typeName=%s, className=%s, precision=%s, scale=%s, isStringType=%s]", + schemaName, catalogName, tableName, name, nameKey, label, displaySize, type, typeName, className, + precision, scale, isStringType); + } + + public String truncate(final String string) { + return precision > 0 ? Strings.left(string, precision) : string; + } + } + private static final JdbcDatabaseManagerFactory INSTANCE = new JdbcDatabaseManagerFactory(); + private static void appendColumnName(final int i, final String columnName, final StringBuilder sb) { + if (i > 1) { + sb.append(','); + } + sb.append(columnName); + } + + /** + * Appends column names to the given buffer in the format {@code "A,B,C"}. + */ + private static void appendColumnNames(final String sqlVerb, final FactoryData data, final StringBuilder sb) { + // so this gets a little more complicated now that there are two ways to configure column mappings, but + // both mappings follow the same exact pattern for the prepared statement + int i = 1; + final String messagePattern = "Appending {} {}[{}]: {}={} "; + for (final ColumnMapping colMapping : data.columnMappings) { + final String columnName = colMapping.getName(); + appendColumnName(i, columnName, sb); + logger().trace(messagePattern, sqlVerb, colMapping.getClass().getSimpleName(), i, columnName, colMapping); + i++; + } + for (final ColumnConfig colConfig : data.columnConfigs) { + final String columnName = colConfig.getColumnName(); + appendColumnName(i, columnName, sb); + logger().trace(messagePattern, sqlVerb, colConfig.getClass().getSimpleName(), i, columnName, colConfig); + i++; + } + } + private static JdbcDatabaseManagerFactory getFactory() { return INSTANCE; } @@ -228,7 +374,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { final ConnectionSource connectionSource, final String tableName, final ColumnConfig[] columnConfigs) { return getManager( name, new FactoryData(bufferSize, null, connectionSource, tableName, columnConfigs, - new ColumnMapping[0], false, AbstractDatabaseAppender.DEFAULT_RECONNECT_INTERVAL_MILLIS), + new ColumnMapping[0], false, AbstractDatabaseAppender.DEFAULT_RECONNECT_INTERVAL_MILLIS, true), getFactory()); } @@ -237,20 +383,19 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { * * @param name The name of the manager, which should include connection details and hashed passwords where possible. * @param bufferSize The size of the log event buffer. + * @param layout The Appender-level layout * @param connectionSource The source for connections to the database. * @param tableName The name of the database table to insert log events into. * @param columnConfigs Configuration information about the log table columns. * @param columnMappings column mapping configuration (including type conversion). * @return a new or existing JDBC manager as applicable. - * @deprecated use - * {@link #getManager(String, int, Layout, ConnectionSource, String, ColumnConfig[], ColumnMapping[], boolean, long)} */ @Deprecated public static JdbcDatabaseManager getManager(final String name, final int bufferSize, - final ConnectionSource connectionSource, final String tableName, final ColumnConfig[] columnConfigs, - final ColumnMapping[] columnMappings) { - return getManager(name, new FactoryData(bufferSize, null, connectionSource, tableName, columnConfigs, - columnMappings, false, AbstractDatabaseAppender.DEFAULT_RECONNECT_INTERVAL_MILLIS), getFactory()); + final Layout<? extends Serializable> layout, final ConnectionSource connectionSource, + final String tableName, final ColumnConfig[] columnConfigs, final ColumnMapping[] columnMappings) { + return getManager(name, new FactoryData(bufferSize, layout, connectionSource, tableName, columnConfigs, + columnMappings, false, AbstractDatabaseAppender.DEFAULT_RECONNECT_INTERVAL_MILLIS, true), getFactory()); } /** @@ -258,19 +403,24 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { * * @param name The name of the manager, which should include connection details and hashed passwords where possible. * @param bufferSize The size of the log event buffer. - * @param layout The Appender-level layout + * @param layout * @param connectionSource The source for connections to the database. * @param tableName The name of the database table to insert log events into. * @param columnConfigs Configuration information about the log table columns. * @param columnMappings column mapping configuration (including type conversion). + * @param reconnectIntervalMillis + * @param immediateFail * @return a new or existing JDBC manager as applicable. + * @deprecated use + * {@link #getManager(String, int, Layout, ConnectionSource, String, ColumnConfig[], ColumnMapping[], boolean, long)} */ @Deprecated public static JdbcDatabaseManager getManager(final String name, final int bufferSize, final Layout<? extends Serializable> layout, final ConnectionSource connectionSource, - final String tableName, final ColumnConfig[] columnConfigs, final ColumnMapping[] columnMappings) { - return getManager(name, new FactoryData(bufferSize, layout, connectionSource, tableName, columnConfigs, - columnMappings, false, AbstractDatabaseAppender.DEFAULT_RECONNECT_INTERVAL_MILLIS), getFactory()); + final String tableName, final ColumnConfig[] columnConfigs, final ColumnMapping[] columnMappings, + final boolean immediateFail, final long reconnectIntervalMillis) { + return getManager(name, new FactoryData(bufferSize, null, connectionSource, tableName, columnConfigs, + columnMappings, false, AbstractDatabaseAppender.DEFAULT_RECONNECT_INTERVAL_MILLIS, true), getFactory()); } /** @@ -286,14 +436,15 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { * @param immediateFail Whether or not to fail immediately with a {@link AppenderLoggingException} when connecting * to JDBC fails. * @param reconnectIntervalMillis How often to reconnect to the database when a SQL exception is detected. + * @param truncateStrings Whether or not to truncate strings to match column metadata. * @return a new or existing JDBC manager as applicable. */ public static JdbcDatabaseManager getManager(final String name, final int bufferSize, final Layout<? extends Serializable> layout, final ConnectionSource connectionSource, final String tableName, final ColumnConfig[] columnConfigs, final ColumnMapping[] columnMappings, - final boolean immediateFail, final long reconnectIntervalMillis) { + final boolean immediateFail, final long reconnectIntervalMillis, final boolean truncateStrings) { return getManager(name, new FactoryData(bufferSize, layout, connectionSource, tableName, columnConfigs, - columnMappings, immediateFail, reconnectIntervalMillis), getFactory()); + columnMappings, immediateFail, reconnectIntervalMillis, truncateStrings), getFactory()); } // NOTE: prepared statements are prepared in this order: column mappings, then column configs @@ -304,6 +455,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { private volatile PreparedStatement statement; private volatile Reconnector reconnector; private volatile boolean isBatchSupported; + private volatile Map<String, ResultSetColumnMetaData> columnMetaData; private JdbcDatabaseManager(final String name, final String sqlStatement, final List<ColumnConfig> columnConfigs, final FactoryData factoryData) { @@ -344,7 +496,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { } } - protected void closeResources(boolean logExceptions) { + protected void closeResources(final boolean logExceptions) { try { // Closing a statement returns it to the pool when using Apache Commons DBCP. // Closing an already closed statement has no effect. @@ -372,7 +524,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { @Override protected boolean commitAndClose() { - boolean closed = true; + final boolean closed = true; try { if (this.connection != null && !this.connection.isClosed()) { if (this.isBatchSupported && this.statement != null) { @@ -396,7 +548,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { try { this.commitAndClose(); return true; - } catch (AppenderLoggingException e) { + } catch (final AppenderLoggingException e) { // Database connection has likely gone stale. final Throwable cause = e.getCause(); final Throwable actual = cause == null ? e : cause; @@ -415,14 +567,17 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { this.connection = getConnectionSource().getConnection(); logger().debug("Acquired JDBC connection {}", this.connection); logger().debug("Getting connection metadata {}", this.connection); - final DatabaseMetaData metaData = this.connection.getMetaData(); - logger().debug("Connection metadata {}", metaData); - this.isBatchSupported = metaData.supportsBatchUpdates(); + final DatabaseMetaData databaseMetaData = this.connection.getMetaData(); + logger().debug("Connection metadata {}", databaseMetaData); + this.isBatchSupported = databaseMetaData.supportsBatchUpdates(); logger().debug("Connection supportsBatchUpdates: {}", this.isBatchSupported); this.connection.setAutoCommit(false); logger().debug("Preparing SQL {}", this.sqlStatement); this.statement = this.connection.prepareStatement(this.sqlStatement); logger().debug("Prepared SQL {}", this.statement); + if (this.factoryData.truncateStrings) { + initColumnMetaData(); + } } @Override @@ -444,6 +599,15 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { return recon; } + private String createSqlSelect() { + final StringBuilder sb = new StringBuilder("select "); + appendColumnNames("SELECT", this.factoryData, sb); + sb.append(" from "); + sb.append(this.factoryData.tableName); + sb.append(" where 1=0"); + return sb.toString(); + } + public ConnectionSource getConnectionSource() { return factoryData.connectionSource; } @@ -456,6 +620,30 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { return factoryData.tableName; } + private void initColumnMetaData() throws SQLException { + // Could use: + // this.connection.getMetaData().getColumns(catalog, schemaPattern, tableNamePattern, columnNamePattern); + // But this returns more data than we need for now, so do a SQL SELECT with 0 result rows instead. + final String sqlSelect = createSqlSelect(); + logger().debug("Getting SQL metadata for table {}: {}", this.factoryData.tableName, sqlSelect); + try (final PreparedStatement mdStatement = this.connection.prepareStatement(sqlSelect)) { + final ResultSetMetaData rsMetaData = mdStatement.getMetaData(); + logger().debug("SQL metadata: {}", rsMetaData); + if (rsMetaData != null) { + final int columnCount = rsMetaData.getColumnCount(); + columnMetaData = new HashMap<>(columnCount); + for (int i = 0, j = 1; i < columnCount; i++, j++) { + final ResultSetColumnMetaData value = new ResultSetColumnMetaData(rsMetaData, j); + columnMetaData.put(value.getNameKey(), value); + } + } else { + logger().warn( + "{}: truncateStrings is true and ResultSetMetaData is null for statement: {}; manager will not perform truncation.", + getClass().getSimpleName(), mdStatement); + } + } + } + private void reconnectOn(final Exception exception) { if (!factoryData.retry) { throw new AppenderLoggingException("Cannot connect and prepare", exception); @@ -480,7 +668,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { private void setFields(final MapMessage<?, ?> mapMessage) throws SQLException { final IndexedReadOnlyStringMap map = mapMessage.getIndexedReadOnlyStringMap(); final String simpleName = statement.getClass().getName(); - int i = 1; // JDBC indices start at 1 + int j = 1; // JDBC indices start at 1 for (final ColumnMapping mapping : this.factoryData.columnMappings) { if (mapping.getLiteralValue() == null) { final String source = mapping.getSource(); @@ -489,14 +677,22 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { if (logger().isTraceEnabled()) { final String valueStr = value instanceof String ? "\"" + value + "\"" : Objects.toString(value, null); - logger().trace("{} setObject({}, {}) for key '{}' and mapping '{}'", simpleName, i, valueStr, key, + logger().trace("{} setObject({}, {}) for key '{}' and mapping '{}'", simpleName, j, valueStr, key, mapping.getName()); } - statement.setObject(i++, value); + setStatementObject(j, mapping.getNameKey(), value); + j++; } } } + /** + * Sets the given Object in the prepared statement. The value is truncated if needed. + */ + private void setStatementObject(final int j, final String nameKey, final Object value) throws SQLException { + statement.setObject(j, truncate(nameKey, value)); + } + @Override protected boolean shutdownInternal() { if (reconnector != null) { @@ -512,6 +708,23 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { // empty } + /** + * Truncates the value if needed. + */ + private Object truncate(final String nameKey, Object value) { + if (value != null && this.factoryData.truncateStrings && columnMetaData != null) { + final ResultSetColumnMetaData resultSetColumnMetaData = columnMetaData.get(nameKey); + if (resultSetColumnMetaData != null) { + if (resultSetColumnMetaData.isStringType()) { + value = resultSetColumnMetaData.truncate(value.toString()); + } + } else { + logger().error("Missing ResultSetColumnMetaData for {}", nameKey); + } + } + return value; + } + @Override protected void writeInternal(final LogEvent event, final Serializable serializable) { StringReader reader = null; @@ -526,30 +739,31 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { if (serializable instanceof MapMessage) { setFields((MapMessage<?, ?>) serializable); } - int i = 1; // JDBC indices start at 1 + int j = 1; // JDBC indices start at 1 for (final ColumnMapping mapping : this.factoryData.columnMappings) { if (ThreadContextMap.class.isAssignableFrom(mapping.getType()) || ReadOnlyStringMap.class.isAssignableFrom(mapping.getType())) { - this.statement.setObject(i++, event.getContextData().toMap()); + this.statement.setObject(j++, event.getContextData().toMap()); } else if (ThreadContextStack.class.isAssignableFrom(mapping.getType())) { - this.statement.setObject(i++, event.getContextStack().asList()); + this.statement.setObject(j++, event.getContextStack().asList()); } else if (Date.class.isAssignableFrom(mapping.getType())) { - this.statement.setObject(i++, DateTypeConverter.fromMillis(event.getTimeMillis(), + this.statement.setObject(j++, DateTypeConverter.fromMillis(event.getTimeMillis(), mapping.getType().asSubclass(Date.class))); } else { final StringLayout layout = mapping.getLayout(); if (layout != null) { if (Clob.class.isAssignableFrom(mapping.getType())) { - this.statement.setClob(i++, new StringReader(layout.toSerializable(event))); + this.statement.setClob(j++, new StringReader(layout.toSerializable(event))); } else if (NClob.class.isAssignableFrom(mapping.getType())) { - this.statement.setNClob(i++, new StringReader(layout.toSerializable(event))); + this.statement.setNClob(j++, new StringReader(layout.toSerializable(event))); } else { final Object value = TypeConverters.convert(layout.toSerializable(event), mapping.getType(), null); if (value == null) { - this.statement.setNull(i++, Types.NULL); + // TODO We might need to always initialize the columnMetaData to specify the type. + this.statement.setNull(j++, Types.NULL); } else { - this.statement.setObject(i++, value); + setStatementObject(j++, mapping.getNameKey(), value); } } } @@ -557,18 +771,20 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { } for (final ColumnConfig column : this.columnConfigs) { if (column.isEventTimestamp()) { - this.statement.setTimestamp(i++, new Timestamp(event.getTimeMillis())); + this.statement.setTimestamp(j++, new Timestamp(event.getTimeMillis())); } else if (column.isClob()) { reader = new StringReader(column.getLayout().toSerializable(event)); if (column.isUnicode()) { - this.statement.setNClob(i++, reader); + this.statement.setNClob(j++, reader); } else { - this.statement.setClob(i++, reader); + this.statement.setClob(j++, reader); } } else if (column.isUnicode()) { - this.statement.setNString(i++, column.getLayout().toSerializable(event)); + this.statement.setNString(j++, Objects.toString( + truncate(column.getColumnNameKey(), column.getLayout().toSerializable(event)), null)); } else { - this.statement.setString(i++, column.getLayout().toSerializable(event)); + this.statement.setString(j++, Objects.toString( + truncate(column.getColumnNameKey(), column.getLayout().toSerializable(event)), null)); } } @@ -601,7 +817,7 @@ public final class JdbcDatabaseManager extends AbstractDatabaseManager { } finally { this.commitAndClose(); } - } catch (DbAppenderLoggingException e) { + } catch (final DbAppenderLoggingException e) { reconnectOn(e); try { this.writeInternal(event, serializable); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderFactoryMethodTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderFactoryMethodTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderFactoryMethodTest.java index 72f7f39..de61e17 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderFactoryMethodTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderFactoryMethodTest.java @@ -16,6 +16,10 @@ */ package org.apache.logging.log4j.core.appender.db.jdbc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import java.io.ByteArrayOutputStream; import java.io.PrintWriter; import java.sql.Connection; @@ -23,6 +27,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.ThreadContext; @@ -34,10 +39,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - /** * Abstract unit test for JdbcAppender using a {@link FactoryMethodConnectionSource} configuration. */ @@ -110,4 +111,12 @@ public abstract class AbstractJdbcAppenderFactoryMethodTest { } } + @Test + public void testTruncate() { + final Logger logger = LogManager.getLogger(this.getClass().getName() + ".testFactoryMethodConfig"); + // Some drivers and database will not allow more data than the column defines. + // We really need a MySQL databases with a default configuration to test this. + logger.debug(StringUtils.repeat('A', 1000)); + } + } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java index 027919d..27a47b7 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java @@ -30,6 +30,7 @@ import java.sql.Statement; import javax.sql.DataSource; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.util.Throwables; @@ -97,7 +98,7 @@ public class JdbcAppenderMapMessageDataSourceTest { writer.close(); final Logger logger = LogManager.getLogger(this.getClass().getName() + ".testDataSourceConfig"); - final MapMessage mapMessage = new MapMessage(); + final MapMessage<?, ?> mapMessage = new MapMessage<>(); mapMessage.with("Id", 1); mapMessage.with("ColumnA", "ValueA"); mapMessage.with("ColumnB", "ValueB"); @@ -115,4 +116,28 @@ public class JdbcAppenderMapMessageDataSourceTest { } } } + + @Test + public void testTruncate() throws SQLException { + try (final Connection connection = jdbcRule.getConnectionSource().getConnection()) { + final Logger logger = LogManager.getLogger(this.getClass().getName() + ".testFactoryMethodConfig"); + // Some drivers and database will not allow more data than the column defines. + // We really need a MySQL databases with a default configuration to test this. + final MapMessage<?, ?> mapMessage = new MapMessage<>(); + mapMessage.with("Id", 1); + mapMessage.with("ColumnA", StringUtils.repeat('A', 1000)); + mapMessage.with("ColumnB", StringUtils.repeat('B', 1000)); + logger.info(mapMessage); + try (final Statement statement = connection.createStatement(); + final ResultSet resultSet = statement + .executeQuery("SELECT Id, ColumnA, ColumnB FROM dsLogEntry ORDER BY Id")) { + + assertTrue("There should be at least one row.", resultSet.next()); + + Assert.assertEquals(1, resultSet.getInt("Id")); + + assertFalse("There should not be two rows.", resultSet.next()); + } + } + } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/log4j-jdbc-dbcp2/src/test/resources/log4j2-jdbc-dbcp2.xml ---------------------------------------------------------------------- diff --git a/log4j-jdbc-dbcp2/src/test/resources/log4j2-jdbc-dbcp2.xml b/log4j-jdbc-dbcp2/src/test/resources/log4j2-jdbc-dbcp2.xml index 4c8aadb..7821fd0 100644 --- a/log4j-jdbc-dbcp2/src/test/resources/log4j2-jdbc-dbcp2.xml +++ b/log4j-jdbc-dbcp2/src/test/resources/log4j2-jdbc-dbcp2.xml @@ -39,6 +39,10 @@ rollbackOnReturn="true" validationQuery="" validationQueryTimeoutSeconds="-1"> + <DisconnectionSqlCodes> + <String>1</String> + <String>2</String> + </DisconnectionSqlCodes> </PoolableConnectionFactory> </PoolingDriver> <ColumnMapping name="ColumnA" /> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/0cd480ff/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f641644..2c5bcb8 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -130,6 +130,9 @@ <action issue="LOG4J2-2508" dev="ggregory" type="fix"> JDBC Appender fails when using both parameter, source, and literal ColumnMapping elements. </action> + <action issue="LOG4J2-2509" dev="ggregory" type="add"> + Allow a JDBC Appender to truncate strings to match a table's metadata column length limit. + </action> </release> <release version="2.11.1" date="2018-07-22" description="GA Release 2.11.1"> <action issue="LOG4J2-2389" dev="rgoers" type="fix" due-to="Liu Wen">
