Add truncate string processor #3924

kkondaka · 2024-01-08T16:58:01Z

Description

Add new processor to mutate string family of processors, called truncate processor, which truncates the string values to the. configured length.

Resolves #3925

Issues Resolved

Resolves #3925

Check List

[ X] New functionality includes testing.
New functionality has a documentation issue. Please link to it in this PR.
- New functionality has javadoc added
[ X] Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Krishna Kondaka <[email protected]>

dlvenable · 2024-01-08T21:04:33Z

.../java/org/opensearch/dataprepper/plugins/processor/mutatestring/TruncateStringProcessor.java

+import org.opensearch.dataprepper.model.processor.Processor;
+
+/**
+ * This processor takes in a key and changes its value to a string with the leading and trailing spaces trimmed.


This Javadoc does not match the behavior shown below.

dlvenable · 2024-01-08T21:05:39Z

.../org/opensearch/dataprepper/plugins/processor/mutatestring/TruncateStringProcessorTests.java

+        @Override
+        public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
+            return Stream.of(
+                    Arguments.arguments("hello,world,no-truncate", 100, "hello,world,no-truncate"),


I think it would be nice to add a static import on Arguments.arguments such that these lines just look like:

arguments("hello,world,no-truncate", 100, "hello,world,no-truncate")

dlvenable · 2024-01-08T21:07:33Z

.../org/opensearch/dataprepper/plugins/processor/mutatestring/TruncateStringProcessorTests.java

+        final Record<Event> record = createEvent(message);
+        final List<Record<Event>> truncatedRecords = (List<Record<Event>>) truncateStringProcessor.doExecute(Collections.singletonList(record));
+        assertThat(truncatedRecords.get(0).getData().get("message", Object.class), notNullValue());
+        assertThat(truncatedRecords.get(0).getData().get("message", Object.class), equalTo(truncatedMessage));


I think this test is simple enough to use a mocked Event.

Then you just verify:

verify(truncatedRecords.get(0)).put("message", truncatedMessage);

I think it is simple that we do not have to mock the event. I think it's better to not mock when the alternative is not hard.

dlvenable · 2024-01-08T21:08:14Z

.../org/opensearch/dataprepper/plugins/processor/mutatestring/TruncateStringProcessorTests.java

+        when(expressionEvaluator.evaluateConditional(truncateWhen, record.getData())).thenReturn(false);
+        final List<Record<Event>> truncatedRecords = (List<Record<Event>>) truncateStringProcessor.doExecute(Collections.singletonList(record));
+
+        assertThat(truncatedRecords.get(0).getData().toMap(), equalTo(record.getData().toMap()));


Again, I think a mock here is easy:

verify(truncatedRecords(0).getData(), never()).put(anyString(), any());

Signed-off-by: Krishna Kondaka <[email protected]>

graytaylor0 · 2024-01-08T22:24:04Z

.../java/org/opensearch/dataprepper/plugins/processor/mutatestring/TruncateStringProcessor.java

-        recordEvent.put(entry.getSource(), value.substring(0, entry.getLength()));
+        int startIndex = entry.getStartAt() == null ? 0 : entry.getStartAt();
+        Integer length = entry.getLength();
+        String truncatedValue = (length == null || startIndex+length >= value.length()) ? value.substring(startIndex) : value.substring(startIndex, startIndex + length);


Should we catch IndexOutOfBoundsException so that this doesn't cause the pipeline to crash?

Signed-off-by: Krishna Kondaka <[email protected]>

…ic to strings Signed-off-by: Krishna Kondaka <[email protected]>

dlvenable · 2024-01-09T23:00:21Z

...ava/org/opensearch/dataprepper/plugins/processor/truncate/TruncateStringProcessorConfig.java

+        public Entry() {}
+    }
+
+    private List<@Valid Entry> entries;


As we are moving away from the existing string processor, should we carry this convention with it?

Perhaps just make the usage:

- truncate: source: my_key length: 100

With multiple values:

- truncate: source: my_key1 length: 100 - truncate: source: my_key2 length: 100

This seems simpler than:

- truncate: entries: - source: my_key1 length: 100 - source: my_key2 length: 100

OK, I will remove entries field

Signed-off-by: Krishna Kondaka <[email protected]>

graytaylor0 · 2024-01-10T00:29:32Z

data-prepper-plugins/truncate-processor/README.md

+      record_type: "event"
+      format: "json"
+  processor:
+    - trucate_string:


Still says truncate_string here, but there's also a typo in trucate

Thanks for finding the typo. Will fix it.

Signed-off-by: Krishna Kondaka <[email protected]>

* Add truncate string processor Signed-off-by: Krishna Kondaka <[email protected]> * Addressed review comments Signed-off-by: Krishna Kondaka <[email protected]> * Added check for negative numbers in the config input Signed-off-by: Krishna Kondaka <[email protected]> * Fixed checkstyle error Signed-off-by: Krishna Kondaka <[email protected]> * Modified to make truncate processor a top level processor, not specific to strings Signed-off-by: Krishna Kondaka <[email protected]> * Addressed review comments Signed-off-by: Krishna Kondaka <[email protected]> * Updated documentation with correct configuration Signed-off-by: Krishna Kondaka <[email protected]> * Fixed typos in the documentation Signed-off-by: Krishna Kondaka <[email protected]> * Modified to allow more than one source keys in the config Signed-off-by: Krishna Kondaka <[email protected]> * Modified to allow multiple entries under configuration Signed-off-by: Krishna Kondaka <[email protected]> --------- Signed-off-by: Krishna Kondaka <[email protected]> Co-authored-by: Krishna Kondaka <[email protected]> (cherry picked from commit c3fe808)

Add truncate string processor

379c6ae

Signed-off-by: Krishna Kondaka <[email protected]>

kkondaka requested review from chenqi0805, engechas, graytaylor0, dinujoh, asifsmohammed, dlvenable and oeyh as code owners January 8, 2024 16:58

dlvenable requested changes Jan 8, 2024

View reviewed changes

graytaylor0 previously approved these changes Jan 8, 2024

View reviewed changes

Addressed review comments

743f347

Signed-off-by: Krishna Kondaka <[email protected]>

kkondaka dismissed graytaylor0’s stale review via 743f347 January 8, 2024 22:05

graytaylor0 reviewed Jan 8, 2024

View reviewed changes

graytaylor0 previously approved these changes Jan 8, 2024

View reviewed changes

dlvenable previously approved these changes Jan 8, 2024

View reviewed changes

Added check for negative numbers in the config input

49aa0eb

Signed-off-by: Krishna Kondaka <[email protected]>

kkondaka dismissed stale reviews from dlvenable and graytaylor0 via 49aa0eb January 8, 2024 23:03

dlvenable previously approved these changes Jan 8, 2024

View reviewed changes

Fixed checkstyle error

f73f553

Signed-off-by: Krishna Kondaka <[email protected]>

kkondaka dismissed dlvenable’s stale review via f73f553 January 9, 2024 04:09

graytaylor0 previously approved these changes Jan 9, 2024

View reviewed changes

Modified to make truncate processor a top level processor, not specif…

96c2568

…ic to strings Signed-off-by: Krishna Kondaka <[email protected]>

kkondaka dismissed graytaylor0’s stale review via 96c2568 January 9, 2024 22:17

dlvenable reviewed Jan 9, 2024

View reviewed changes

Addressed review comments

e935916

Signed-off-by: Krishna Kondaka <[email protected]>

graytaylor0 reviewed Jan 10, 2024

View reviewed changes

Krishna Kondaka added 2 commits January 10, 2024 00:38

Updated documentation with correct configuration

d2f107b

Signed-off-by: Krishna Kondaka <[email protected]>

Fixed typos in the documentation

3ba6e2f

Signed-off-by: Krishna Kondaka <[email protected]>

graytaylor0 previously approved these changes Jan 10, 2024

View reviewed changes

Krishna Kondaka added 2 commits January 10, 2024 20:57

Modified to allow more than one source keys in the config

78d5cf6

Signed-off-by: Krishna Kondaka <[email protected]>

Modified to allow multiple entries under configuration

9609029

Signed-off-by: Krishna Kondaka <[email protected]>

kkondaka dismissed graytaylor0’s stale review via 9609029 January 11, 2024 04:54

dlvenable approved these changes Jan 11, 2024

View reviewed changes

graytaylor0 approved these changes Jan 11, 2024

View reviewed changes

kkondaka merged commit c3fe808 into opensearch-project:main Jan 11, 2024
71 of 74 checks passed

kkondaka added the backport 2.6 label Jan 11, 2024

opensearch-trigger-bot bot mentioned this pull request Jan 11, 2024

[Backport 2.6] Add truncate string processor #3950

Closed

kkondaka deleted the truncate-processor branch May 13, 2024 05:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add truncate string processor #3924

Add truncate string processor #3924

kkondaka commented Jan 8, 2024 •

edited

Loading

dlvenable Jan 8, 2024

dlvenable Jan 8, 2024

dlvenable Jan 8, 2024

kkondaka Jan 8, 2024

dlvenable Jan 8, 2024

kkondaka Jan 8, 2024

graytaylor0 Jan 8, 2024

dlvenable Jan 9, 2024

kkondaka Jan 9, 2024

graytaylor0 Jan 10, 2024

kkondaka Jan 10, 2024

Add truncate string processor #3924

Add truncate string processor #3924

Conversation

kkondaka commented Jan 8, 2024 • edited Loading

Description

Issues Resolved

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkondaka commented Jan 8, 2024 •

edited

Loading