From 6254f7d4bb429fef00f287ba284e2eeec5c3378c Mon Sep 17 00:00:00 2001 From: zezutom Date: Mon, 19 Jun 2017 06:47:22 +0100 Subject: [PATCH] Code review comments: Parameterized logging to ensure good performance. --- .../writer/SimpleHbaseEnrichmentWriter.java | 43 ++++- ...impleHBaseEnrichmentWriterLoggingTest.java | 176 ++++++++++++++++++ 2 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 metron-platform/metron-parsers/src/test/java/org/apache/metron/writers/SimpleHBaseEnrichmentWriterLoggingTest.java diff --git a/metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/writer/SimpleHbaseEnrichmentWriter.java b/metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/writer/SimpleHbaseEnrichmentWriter.java index e637156fb9..ad3b6d743f 100644 --- a/metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/writer/SimpleHbaseEnrichmentWriter.java +++ b/metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/writer/SimpleHbaseEnrichmentWriter.java @@ -38,6 +38,8 @@ import org.apache.metron.writer.AbstractWriter; import org.apache.metron.common.writer.BulkWriterResponse; import org.json.simple.JSONObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.Serializable; @@ -45,6 +47,9 @@ import java.util.stream.Collectors; public class SimpleHbaseEnrichmentWriter extends AbstractWriter implements BulkMessageWriter, Serializable { + + private static final Logger LOG = LoggerFactory.getLogger(SimpleHbaseEnrichmentWriter.class); + public enum Configurations { HBASE_TABLE("shew.table") ,HBASE_CF("shew.cf") @@ -62,13 +67,18 @@ public String getKey() { return key; } public Object get(Map config) { - return config.get(key); + Object o = config.get(key); + if (o == null) { + LOG.warn("No config object found for key: '{}'", key); + } + return o; } public T getAndConvert(Map config, Class clazz) { Object o = get(config); if(o != null) { return ConversionUtils.convert(o, clazz); } + LOG.warn("No object of type '{}' found in config", clazz); return null; } } @@ -94,11 +104,12 @@ public KeyTransformer(Iterable keys, String delim) { } public String transform(final JSONObject message) { - return - keys.stream().map( x -> { + String transformedMessage = keys.stream().map(x -> { Object o = message.get(x); return o == null?"":o.toString(); }).collect(Collectors.joining(delim)); + LOG.debug("Transformed message: '{}'", transformedMessage); + return transformedMessage; } } private transient EnrichmentConverter converter; @@ -120,6 +131,11 @@ public void configure(String sensorName, WriterConfiguration configuration) { if(converter == null) { converter = new EnrichmentConverter(); } + LOG.debug("Sensor: '{}': {Provider: '{}', Converter: '{}'}", sensorName, getClassName(provider), getClassName(converter)); + } + + private String getClassName(Object object) { + return object == null ? "" : object.getClass().getName(); } @Override @@ -150,6 +166,7 @@ public HTableInterface getTable(String tableName, String cf) throws IOException if(table != null) { table.close(); } + LOG.debug("Fetching table '{}', column family: '{}'", tableName, cf); table = getProvider().getTable(conf, tableName); this.tableName = tableName; this.cf = cf; @@ -169,9 +186,11 @@ public HTableInterface getTable(Map config) throws IOException { private List getColumns(Object keyColumnsObj, boolean allowNull) { Object o = keyColumnsObj; if(allowNull && keyColumnsObj == null) { + LOG.debug("No key columns were specified"); return Collections.emptyList(); } if(o instanceof String) { + LOG.debug("Key column: '{}'", o); return ImmutableList.of(o.toString()); } else if (o instanceof List) { @@ -179,6 +198,7 @@ else if (o instanceof List) { for(Object key : (List)o) { keyCols.add(key.toString()); } + LOG.debug("Key columns: '{}'", String.join(",", keyCols)); return keyCols; } else { @@ -190,7 +210,9 @@ private KeyTransformer getTransformer(Map config) { Object o = Configurations.KEY_COLUMNS.get(config); KeyTransformer transformer = null; if(keyTransformer != null && keyTransformer.getKey() == o) { - return keyTransformer.getValue(); + transformer = keyTransformer.getValue(); + LOG.debug("Transformer found for key '{}': '{}'", o, transformer); + return transformer; } else { List keys = getColumns(o, false); @@ -198,6 +220,7 @@ private KeyTransformer getTransformer(Map config) { String delim = (delimObj == null || !(delimObj instanceof String))?null:delimObj.toString(); transformer = new KeyTransformer(keys, delim); keyTransformer = new AbstractMap.SimpleEntry<>(o, transformer); + LOG.debug("Transformer found for keys '{}' and delimiter '{}': '{}'", String.join(",", keys), delim, transformer); return transformer; } } @@ -213,7 +236,7 @@ private EnrichmentValue getValue( JSONObject message for (Object kv : message.entrySet()) { Map.Entry entry = (Map.Entry) kv; if (!keyColumns.contains(entry.getKey())) { - metadata.put(entry.getKey().toString(), entry.getValue()); + addMetadataEntry(metadata, entry); } } return new EnrichmentValue(metadata); @@ -222,13 +245,20 @@ private EnrichmentValue getValue( JSONObject message for (Object kv : message.entrySet()) { Map.Entry entry = (Map.Entry) kv; if (valueColumns.contains(entry.getKey())) { - metadata.put(entry.getKey().toString(), entry.getValue()); + addMetadataEntry(metadata, entry); } } return new EnrichmentValue(metadata); } } + private void addMetadataEntry(Map metadata, Map.Entry entry) { + String key = entry.getKey().toString(); + Object value = entry.getValue(); + LOG.debug("Adding metadata: {Key: '{}', Value: '{}'}", key, value); + metadata.put(key, value); + } + private EnrichmentKey getKey(JSONObject message, KeyTransformer transformer, String enrichmentType) { if(enrichmentType != null) { return new EnrichmentKey(enrichmentType, transformer.transform(message)); @@ -260,6 +290,7 @@ public BulkWriterResponse write(String sensorType } Put put = converter.toPut(this.cf, key, value); if(put != null) { + LOG.debug("Put: {Column Family: '{}', Key: '{}', Value: '{}'}", this.cf, key, value); puts.add(put); } } diff --git a/metron-platform/metron-parsers/src/test/java/org/apache/metron/writers/SimpleHBaseEnrichmentWriterLoggingTest.java b/metron-platform/metron-parsers/src/test/java/org/apache/metron/writers/SimpleHBaseEnrichmentWriterLoggingTest.java new file mode 100644 index 0000000000..3949162a69 --- /dev/null +++ b/metron-platform/metron-parsers/src/test/java/org/apache/metron/writers/SimpleHBaseEnrichmentWriterLoggingTest.java @@ -0,0 +1,176 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.metron.writers; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.metron.common.configuration.writer.WriterConfiguration; +import org.apache.metron.enrichment.converter.EnrichmentConverter; +import org.apache.metron.enrichment.integration.mock.MockTableProvider; +import org.apache.metron.enrichment.writer.SimpleHbaseEnrichmentWriter; +import org.json.simple.JSONObject; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.*; + +import static org.junit.Assert.*; +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.verify; +import static org.powermock.api.mockito.PowerMockito.*; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({SimpleHbaseEnrichmentWriter.class, LoggerFactory.class, HBaseConfiguration.class}) +public class SimpleHBaseEnrichmentWriterLoggingTest { + + private static Logger logger; + + private static final String COLUMN_FAMILY = "cf1"; + + private SimpleHbaseEnrichmentWriter hbaseEnrichmentWriter; + + @BeforeClass + public static void init() { + mockStatic(LoggerFactory.class); + logger = mock(Logger.class); + when(LoggerFactory.getLogger(SimpleHbaseEnrichmentWriter.class)).thenReturn(logger); + } + + @Before + public void setUp() { + hbaseEnrichmentWriter = new SimpleHbaseEnrichmentWriter(); + mockStatic(HBaseConfiguration.class); + Configuration mockConfig = mock(Configuration.class); + when(mockConfig.getInt(eq("hbase.column.max.version"), anyInt())).thenReturn(1); + when(HBaseConfiguration.create()).thenReturn(mockConfig); + } + + @Test + public void shouldWarnOnMissingConfig() { + SimpleHbaseEnrichmentWriter.Configurations config = getHbaseTableConfig(); + config.get(Collections.emptyMap()); + verify(logger).warn("No config object found for key: '{}'", config.getKey()); + } + + @Test + public void shouldWarnOnMissedConfigConversion() { + getHbaseTableConfig().getAndConvert(Collections.emptyMap(), String.class); + verify(logger).warn("No object of type '{}' found in config", String.class); + } + + @Test + public void shouldDebugTransformedMessage() { + + // An expected property + String key = "message"; + String value = "Hello World!"; + + // A received message bearing the property + SimpleHbaseEnrichmentWriter.KeyTransformer keyTransformer = new SimpleHbaseEnrichmentWriter.KeyTransformer(key); + JSONObject message = newJSONObject(key, value); + + // Message transformation + keyTransformer.transform(message); + + // Proof the result has been captured + verify(logger).debug("Transformed message: '{}'", value); + } + + @Test + public void shouldDebugSensorConfig() { + String sensorName = "someSensor"; + hbaseEnrichmentWriter.configure(sensorName, getMockWriterConfiguration()); + verify(logger).debug("Sensor: '{}': {Provider: '{}', Converter: '{}'}", + sensorName, MockTableProvider.class.getName(), EnrichmentConverter.class.getName()); + } + + @Test + public void shouldDebugFetchedHBaseTable() throws IOException { + String sensorName = "someSensor"; + String tableName = "someTable"; + hbaseEnrichmentWriter.configure(sensorName, getMockWriterConfiguration()); + hbaseEnrichmentWriter.getTable(tableName, COLUMN_FAMILY); + verify(logger).debug("Fetching table '{}', column family: '{}'", tableName, COLUMN_FAMILY); + } + + @Test + public void shouldDebugWriteOperation() throws Exception { + String sensorName = "someSensor"; + String key = "testKey"; + String value = "testValue"; + WriterConfiguration mockWriterConfiguration = getMockWriterConfiguration(); + hbaseEnrichmentWriter.configure(sensorName, mockWriterConfiguration); + hbaseEnrichmentWriter.write(sensorName, + mockWriterConfiguration, + Collections.emptyList(), + Collections.singletonList(newJSONObject(key, value))); + + ArgumentCaptor logTemplateCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor logArgsCaptor = ArgumentCaptor.forClass(Object.class); + verify(logger, atLeastOnce()).debug(logTemplateCaptor.capture(), + logArgsCaptor.capture(), logArgsCaptor.capture(), logArgsCaptor.capture()); + + assertEquals("Put: {Column Family: '{}', Key: '{}', Value: '{}'}", logTemplateCaptor.getValue()); + + List logArgs = logArgsCaptor.getAllValues(); + assertEquals(COLUMN_FAMILY, assertAndGetArg(logArgs, 3)); + assertEquals("EnrichmentKey{indicator=':', type='testEnrichment'}", assertAndGetArg(logArgs, 2).toString()); + assertEquals("EnrichmentValue{metadata={testKey=testValue}}", assertAndGetArg(logArgs, 1).toString()); + } + + private T assertAndGetArg(List capturedArgs, int index) { + assertTrue(capturedArgs.size() - index >= 0); + T result = (T) capturedArgs.get(capturedArgs.size() - index); + assertNotNull(result); + return result; + } + + private JSONObject newJSONObject(String key, String value) { + return new JSONObject(Collections.singletonMap(key, value)); + } + + private SimpleHbaseEnrichmentWriter.Configurations getHbaseTableConfig() { + return SimpleHbaseEnrichmentWriter.Configurations.HBASE_TABLE; + } + + private WriterConfiguration getMockWriterConfiguration() { + WriterConfiguration writerConfiguration = mock(WriterConfiguration.class); + Map configMap = new HashMap<>(); + + String tableName = SimpleHBaseEnrichmentWriterLoggingTest.class.getSimpleName(); + MockTableProvider.addTable(tableName, COLUMN_FAMILY); + + configMap.put(SimpleHbaseEnrichmentWriter.Configurations.HBASE_PROVIDER.getKey(), MockTableProvider.class); + configMap.put(SimpleHbaseEnrichmentWriter.Configurations.KEY_COLUMNS.getKey(), Arrays.asList("col1", "col2")); + configMap.put(SimpleHbaseEnrichmentWriter.Configurations.HBASE_TABLE.getKey(), tableName); + configMap.put(SimpleHbaseEnrichmentWriter.Configurations.HBASE_CF.getKey(), COLUMN_FAMILY); + configMap.put(SimpleHbaseEnrichmentWriter.Configurations.ENRICHMENT_TYPE.getKey(), "testEnrichment"); + when(writerConfiguration.getSensorConfig(anyString())).thenReturn(configMap); + return writerConfiguration; + } +}