Tagging Events in Data Prepper. Issue #629 #2629

kkondaka · 2023-05-03T17:12:46Z

Description

Added support to add tags to events.
Tags can be added at the time of creation of a event or after creating it. The tags are added to event meta data as per the discussion in issue #629.

Resolves #629

Issues Resolved

#629

Check List

[X ] New functionality includes testing.
New functionality has been documented.
- 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 <krishkdk@amazon.com>

codecov-commenter · 2023-05-03T17:35:32Z

Codecov Report

Merging #2629 (3dd9f3d) into main (1f90f38) will increase coverage by 0.06%.
The diff coverage is 91.30%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #2629      +/-   ##
============================================
+ Coverage     93.46%   93.53%   +0.06%     
- Complexity     2185     2220      +35     
============================================
  Files           258      258              
  Lines          6107     6230     +123     
  Branches        497      512      +15     
============================================
+ Hits           5708     5827     +119     
+ Misses          268      266       -2     
- Partials        131      137       +6

Impacted Files	Coverage Δ
.../dataprepper/model/event/DefaultEventMetadata.java	`90.47% <80.00%> (-0.44%)`	⬇️
...ensearch/dataprepper/model/event/JacksonEvent.java	`98.87% <100.00%> (+0.08%)`	⬆️

... and 8 files with indirect coverage changes

graytaylor0 · 2023-05-03T17:17:57Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/DefaultEventMetadata.java

+    }
+
+    @Override
+    public void setTag(final String tag) {


Not a big deal but should rename this function to addTag

graytaylor0 · 2023-05-03T23:05:39Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java

+    @Override
+    public String toJsonStringWithTags(final String tagsKey) {
+        String result = jsonNode.toString();
+        return result.substring(0, result.length()-1) + ",\""+tagsKey+"\":"+ getMetadata().getTags()+"}";


Does this need quotes around the getMetadata().getTags? Right now the final json would look like this

end_of_event, "tagsKey": [tag1, tag2]}

Should it look like this instead? I'm not sure if it's a requirement from json but looking at this (https://www.microfocus.com/documentation/silk-performer/205/en/silkperformer-205-webhelp-en/GUID-0847DE13-2A2F-44F2-A6E7-214CD703BF84.html) seems like we want the following

end_of_event, "tagsKey": ["tag1", "tag2"]}

We could do that. By default toString() of a Set does not put quotes around the individual tags. If we want quotes around each tag, then we will have to manually generate it like that. I am OK, if that's what we want.

Can this JsonString be re-serialized into an event in its current form?

I don't think so. It's one way (ie event to string with tags) when we want to send to sink with tags.

Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

cmanning09 · 2023-05-04T19:53:31Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java

+    @Override
+    public String toJsonStringWithTags(final String tagsKey) {
+        String result = jsonNode.toString();
+        return result.substring(0, result.length()-1) + ",\""+tagsKey+"\":"+ getMetadata().getTags()+"}";


Can this JsonString be re-serialized into an event in its current form?

cmanning09 · 2023-05-04T19:55:49Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/EventMetadata.java

+    Boolean hasTag(final String tag);
+
+    /**
+     * Returns the attributes


Copy and paste error?

This method does not return attributes.

cmanning09 · 2023-05-04T19:57:01Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/EventMetadata.java

@@ -35,4 +36,26 @@ public interface EventMetadata extends Serializable {
     * @since 1.2
     */
    Map<String, Object> getAttributes();
+


Do we want to support removing tags as well? Do we need that for this initial implementation or is that something we can add later?

I think initial implementation is fine as is. When we add the tag_manager processor we will need to add support for removing

+1, I was just not sure of the requirements for this feature in the near term.

graytaylor0 · 2023-05-04T20:08:33Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java

+        String separator = "";
+
+        result = result.substring(0, result.length()-1) + ",\"tags\":[";
+        for (final String tag: getMetadata().getTags()) {


This works as is but could be done with a stream like this

String tagSetString = String.join(",", tagSet .stream() .map(tagName -> ("\"" + tagName + "\"")) .collect(Collectors.toList()));

which would mean only a single append (instead of one per tag) is needed. It will be more performant than appending a bunch of times

result += tagSetString

dlvenable · 2023-05-04T20:44:35Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java

@@ -276,6 +276,20 @@ public String toJsonString() {
        return jsonNode.toString();
    }

+    @Override
+    public String toJsonStringWithTags(final String tagsKey) {


I'm not sure this is a very extensible approach. Perhaps some form of JSON string builder would be more appropriate.

As a caller, perhaps I could call:

event.jsonStringBuilder() .includeTags("tags") .toJsonString()

Over time we may find other features that we want to include in the event.

dlvenable · 2023-05-04T21:01:05Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java

+        String separator = "";
+
+        result = result.substring(0, result.length()-1) + ",\"tags\":[";
+        for (final String tag: getMetadata().getTags()) {


I don't think we should be manipulating JSON manually. Let's do this with Jackson.

You can create a JsonNode and then add the value.

…nt to return event with additinal info (like tags) as json string Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

Tagging Events in Data Prepper. Issue opensearch-project#629

5ee91c4

Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

kkondaka requested review from chenqi0805, engechas, graytaylor0, dinujoh, cmanning09, asifsmohammed, dlvenable and oeyh as code owners May 3, 2023 17:12

graytaylor0 reviewed May 3, 2023

View reviewed changes

Addressed review comments

1a61e0c

Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

cmanning09 reviewed May 4, 2023

View reviewed changes

graytaylor0 reviewed May 4, 2023

View reviewed changes

dlvenable requested changes May 4, 2023

View reviewed changes

Addressed review comments. Introduced JsonStringBuilder in JacksonEve…

3dd9f3d

…nt to return event with additinal info (like tags) as json string Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>

graytaylor0 approved these changes May 5, 2023

View reviewed changes

dlvenable approved these changes May 5, 2023

View reviewed changes

kkondaka merged commit 38b578c into opensearch-project:main May 5, 2023

kkondaka mentioned this pull request Jun 5, 2023

Tagging Events in Data Prepper #629

Closed

kkondaka deleted the event-tagging branch July 13, 2023 04:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tagging Events in Data Prepper. Issue #629 #2629

Tagging Events in Data Prepper. Issue #629 #2629

kkondaka commented May 3, 2023

codecov-commenter commented May 3, 2023 •

edited

Loading

graytaylor0 May 3, 2023

graytaylor0 May 3, 2023

kkondaka May 4, 2023

cmanning09 May 4, 2023

kkondaka May 4, 2023

cmanning09 May 4, 2023

cmanning09 May 4, 2023

cmanning09 May 4, 2023

graytaylor0 May 4, 2023

cmanning09 May 4, 2023

graytaylor0 May 4, 2023

dlvenable May 4, 2023

dlvenable May 4, 2023

Tagging Events in Data Prepper. Issue #629 #2629

Tagging Events in Data Prepper. Issue #629 #2629

Conversation

kkondaka commented May 3, 2023

Description

Issues Resolved

Check List

codecov-commenter commented May 3, 2023 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented May 3, 2023 •

edited

Loading