-
Notifications
You must be signed in to change notification settings - Fork 202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve logging for exceptions #4942
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
import com.fasterxml.jackson.annotation.JsonClassDescription; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import com.fasterxml.jackson.annotation.JsonPropertyOrder; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
import org.opensearch.dataprepper.model.processor.Processor; | ||
|
@@ -34,7 +35,7 @@ public Collection<Record<?>> execute(final Collection<Record<?>> records) { | |
try { | ||
Thread.sleep(delayDuration.toMillis()); | ||
} catch (final InterruptedException ex) { | ||
LOG.error("Interrupted during delay processor", ex); | ||
LOG.error(NOISY,"Interrupted during delay processor", ex); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please fix the formatting to include a space after the comma. |
||
} | ||
return records; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,16 +8,17 @@ | |
import com.fasterxml.jackson.annotation.JsonClassDescription; | ||
import com.fasterxml.jackson.annotation.JsonPropertyDescription; | ||
import com.fasterxml.jackson.annotation.JsonPropertyOrder; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.core.type.TypeReference; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
import org.opensearch.dataprepper.model.configuration.PluginSetting; | ||
import org.opensearch.dataprepper.model.event.Event; | ||
import org.opensearch.dataprepper.model.event.JacksonEvent; | ||
import org.opensearch.dataprepper.model.processor.Processor; | ||
import org.opensearch.dataprepper.model.record.Record; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.core.type.TypeReference; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -26,8 +27,6 @@ | |
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
/** | ||
* A simple String implementation of {@link Processor} which generates new Records with uppercase or lowercase content. The current | ||
* simpler implementation does not handle errors (if any). | ||
|
@@ -84,7 +83,7 @@ public Collection<Record<Event>> execute(final Collection<Record<Event>> records | |
.build(); | ||
modifiedRecords.add(new Record<>(newRecordEvent)); | ||
} catch (JsonProcessingException e) { | ||
LOG.error(EVENT, "Unable to process Event data: {}", eventJson, e); | ||
LOG.error(NOISY, "Unable to process Event data: {}", eventJson, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update this to use fluent logging such that it applies both markers. See the description in #4927. It will probably look like:
|
||
} | ||
} | ||
return modifiedRecords; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,13 @@ | |
|
||
package org.opensearch.dataprepper.plugins.processor.csv; | ||
|
||
import com.fasterxml.jackson.databind.MappingIterator; | ||
import com.fasterxml.jackson.dataformat.csv.CsvMapper; | ||
import com.fasterxml.jackson.dataformat.csv.CsvParser; | ||
import com.fasterxml.jackson.dataformat.csv.CsvSchema; | ||
import io.micrometer.core.instrument.Counter; | ||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -14,11 +20,6 @@ | |
import org.opensearch.dataprepper.model.processor.AbstractProcessor; | ||
import org.opensearch.dataprepper.model.processor.Processor; | ||
import org.opensearch.dataprepper.model.record.Record; | ||
import com.fasterxml.jackson.databind.MappingIterator; | ||
import com.fasterxml.jackson.dataformat.csv.CsvMapper; | ||
import com.fasterxml.jackson.dataformat.csv.CsvParser; | ||
import com.fasterxml.jackson.dataformat.csv.CsvSchema; | ||
import io.micrometer.core.instrument.Counter; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -28,8 +29,6 @@ | |
import java.util.List; | ||
import java.util.Objects; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
/** | ||
* Processor to parse CSV data in Events. | ||
* | ||
|
@@ -105,7 +104,7 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
} | ||
} catch (final IOException e) { | ||
csvInvalidEventsCounter.increment(); | ||
LOG.error(EVENT, "An exception occurred while reading event [{}]", event, e); | ||
LOG.error(NOISY, "An exception occurred while reading event [{}]", event, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
} | ||
} | ||
return records; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,7 +72,7 @@ public Collection<Record<Event>> doExecute(Collection<Record<Event>> records) { | |
} | ||
} | ||
} catch (Exception ex){ | ||
LOG.error(EVENT, "Error dissecting the event [{}] ", record.getData(), ex); | ||
LOG.error(NOISY, "Error dissecting the event [{}] ", record.getData(), ex); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
} | ||
} | ||
return records; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
import io.micrometer.core.instrument.Counter; | ||
import io.micrometer.core.instrument.Timer; | ||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -21,6 +22,8 @@ | |
import org.opensearch.dataprepper.model.processor.AbstractProcessor; | ||
import org.opensearch.dataprepper.model.processor.Processor; | ||
import org.opensearch.dataprepper.model.record.Record; | ||
import static org.opensearch.dataprepper.plugins.processor.grok.GrokProcessorConfig.TOTAL_PATTERNS_ATTEMPTED_METADATA_KEY; | ||
import static org.opensearch.dataprepper.plugins.processor.grok.GrokProcessorConfig.TOTAL_TIME_SPENT_IN_GROK_METADATA_KEY; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -54,10 +57,6 @@ | |
import java.util.regex.PatternSyntaxException; | ||
import java.util.stream.Collectors; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
import static org.opensearch.dataprepper.plugins.processor.grok.GrokProcessorConfig.TOTAL_PATTERNS_ATTEMPTED_METADATA_KEY; | ||
import static org.opensearch.dataprepper.plugins.processor.grok.GrokProcessorConfig.TOTAL_TIME_SPENT_IN_GROK_METADATA_KEY; | ||
|
||
|
||
@SingleThread | ||
@DataPrepperPlugin(name = "grok", pluginType = Processor.class, pluginConfigurationType = GrokProcessorConfig.class) | ||
|
@@ -153,11 +152,11 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
|
||
} catch (final TimeoutException e) { | ||
event.getMetadata().addTags(tagsOnTimeout); | ||
LOG.error(EVENT, "Matching on record [{}] took longer than [{}] and timed out", record.getData(), grokProcessorConfig.getTimeoutMillis()); | ||
LOG.error(NOISY, "Matching on record [{}] took longer than [{}] and timed out", record.getData(), grokProcessorConfig.getTimeoutMillis()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
grokProcessingTimeoutsCounter.increment(); | ||
} catch (final ExecutionException | InterruptedException | RuntimeException e) { | ||
event.getMetadata().addTags(tagsOnMatchFailure); | ||
LOG.error(EVENT, "An exception occurred when matching record [{}]", record.getData(), e); | ||
LOG.error(NOISY, "An exception occurred when matching record [{}]", record.getData(), e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
grokProcessingErrorsCounter.increment(); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
package org.opensearch.dataprepper.plugins.processor.mutateevent; | ||
|
||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -26,8 +27,6 @@ | |
import java.util.function.Consumer; | ||
import java.util.function.Supplier; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
@DataPrepperPlugin(name = "add_entries", pluginType = Processor.class, pluginConfigurationType = AddEntryProcessorConfig.class) | ||
public class AddEntryProcessor extends AbstractProcessor<Record<Event>, Record<Event>> { | ||
private static final Logger LOG = LoggerFactory.getLogger(AddEntryProcessor.class); | ||
|
@@ -92,12 +91,12 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
} | ||
} | ||
} catch (Exception e) { | ||
LOG.error(EVENT, "Error adding entry to record [{}] with key [{}], metadataKey [{}], value_expression [{}] format [{}], value [{}]", | ||
LOG.error(NOISY, "Error adding entry to record [{}] with key [{}], metadataKey [{}], value_expression [{}] format [{}], value [{}]", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
recordEvent, entry.getKey(), entry.getMetadataKey(), entry.getValueExpression(), entry.getFormat(), entry.getValue(), e); | ||
} | ||
} | ||
} catch(final Exception e){ | ||
LOG.error(EVENT, "There was an exception while processing Event [{}]", recordEvent, e); | ||
LOG.error(NOISY, "There was an exception while processing Event [{}]", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ | |
package org.opensearch.dataprepper.plugins.processor.mutateevent; | ||
|
||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -24,8 +26,6 @@ | |
import java.util.List; | ||
import java.util.Objects; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
@DataPrepperPlugin(name = "convert_entry_type", pluginType = Processor.class, pluginConfigurationType = ConvertEntryTypeProcessorConfig.class) | ||
public class ConvertEntryTypeProcessor extends AbstractProcessor<Record<Event>, Record<Event>> { | ||
private static final Logger LOG = LoggerFactory.getLogger(ConvertEntryTypeProcessor.class); | ||
|
@@ -91,7 +91,7 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
} | ||
} | ||
} catch (final Exception e) { | ||
LOG.error(EVENT, "There was an exception while processing Event [{}]", recordEvent, e); | ||
LOG.error(NOISY, "There was an exception while processing Event [{}]", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
recordEvent.getMetadata().addTags(tagsOnFailure); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
package org.opensearch.dataprepper.plugins.processor.mutateevent; | ||
|
||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -24,8 +25,6 @@ | |
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
@DataPrepperPlugin(name = "copy_values", pluginType = Processor.class, pluginConfigurationType = CopyValueProcessorConfig.class) | ||
public class CopyValueProcessor extends AbstractProcessor<Record<Event>, Record<Event>> { | ||
private static final Logger LOG = LoggerFactory.getLogger(CopyValueProcessor.class); | ||
|
@@ -93,7 +92,7 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
} | ||
} | ||
} catch (final Exception e) { | ||
LOG.error(EVENT, "There was an exception while processing Event [{}]", recordEvent, e); | ||
LOG.error(NOISY, "There was an exception while processing Event [{}]", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
package org.opensearch.dataprepper.plugins.processor.mutateevent; | ||
|
||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -22,8 +23,6 @@ | |
import java.util.List; | ||
import java.util.Objects; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
@DataPrepperPlugin(name = "delete_entries", pluginType = Processor.class, pluginConfigurationType = DeleteEntryProcessorConfig.class) | ||
public class DeleteEntryProcessor extends AbstractProcessor<Record<Event>, Record<Event>> { | ||
|
||
|
@@ -62,7 +61,7 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
recordEvent.delete(entry); | ||
} | ||
} catch (final Exception e) { | ||
LOG.error(EVENT, "There was an exception while processing Event [{}]", recordEvent, e); | ||
LOG.error(NOISY, "There was an exception while processing Event [{}]", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ | |
package org.opensearch.dataprepper.plugins.processor.mutateevent; | ||
|
||
import org.opensearch.dataprepper.expression.ExpressionEvaluator; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; | ||
import org.opensearch.dataprepper.metrics.PluginMetrics; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPlugin; | ||
import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor; | ||
|
@@ -24,8 +26,6 @@ | |
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
import static org.opensearch.dataprepper.logging.DataPrepperMarkers.EVENT; | ||
|
||
@DataPrepperPlugin(name = "list_to_map", pluginType = Processor.class, pluginConfigurationType = ListToMapProcessorConfig.class) | ||
public class ListToMapProcessor extends AbstractProcessor<Record<Event>, Record<Event>> { | ||
|
||
|
@@ -78,19 +78,19 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor | |
recordEvent.getMetadata().addTags(config.getTagsOnFailure()); | ||
continue; | ||
} catch (final Exception e) { | ||
LOG.error(EVENT, "Error converting source list to map on record [{}]", recordEvent, e); | ||
LOG.error(NOISY, "Error converting source list to map on record [{}]", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
recordEvent.getMetadata().addTags(config.getTagsOnFailure()); | ||
continue; | ||
} | ||
|
||
try { | ||
updateEvent(recordEvent, targetMap); | ||
} catch (final Exception e) { | ||
LOG.error(EVENT, "Error updating record [{}] after converting source list to map", recordEvent, e); | ||
LOG.error(NOISY, "Error updating record [{}] after converting source list to map", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
recordEvent.getMetadata().addTags(config.getTagsOnFailure()); | ||
} | ||
} catch (final Exception e) { | ||
LOG.error(EVENT, "There was an exception while processing Event [{}]", recordEvent, e); | ||
LOG.error(NOISY, "There was an exception while processing Event [{}]", recordEvent, e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment about using fluent logging to apply both markers. |
||
recordEvent.getMetadata().addTags(config.getTagsOnFailure()); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want this relationship. A log may be noisy without being sensitive.