Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Default keys #3075

Merged
merged 16 commits into from
Aug 8, 2023
Merged

Default keys #3075

merged 16 commits into from
Aug 8, 2023

Conversation

shenkw1
Copy link
Contributor

@shenkw1 shenkw1 commented Jul 28, 2023

Description

A hash specifying the default keys and their values which should be added to the event in case these keys do not exist in the source field being parsed.

Issues Resolved

#891

Check List

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

@shenkw1
readme and config
df150a4 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
skeleton logic written
56db3be 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
fix merge conflict
d0e12a6 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
default keys impl and tests
08b8fb9 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
finish tests
403d466 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
rerun checks
5223500 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
change impl to have parity with logstash
42d3090 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
add clarifying example to readme, fix edge cases, add tests
50e90af 
Signed-off-by: Kat Shen <katshen@amazon.com>
chenqi0805
chenqi0805 previously approved these changes Jul 31, 2023
View reviewed changes
Copy link
Collaborator

@chenqi0805 chenqi0805 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit on test case name. LGTM!

.../test/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessorTests.java Outdated Show resolved Hide resolved
@shenkw1
rename test for clarity
446a718 
Signed-off-by: Kat Shen <katshen@amazon.com>
chenqi0805
chenqi0805 previously approved these changes Jul 31, 2023
View reviewed changes
@shenkw1
change logging statements from string.format()
5711e30 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
fix default key check error
6667d82 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
change default config name to default_values, fix to have parity with…
8a4d2a1 
… logstash, enhance relevant tests

Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
rerun checks
9496247 
Signed-off-by: Kat Shen <katshen@amazon.com>
@shenkw1
fix nits
b2fc52b 
Signed-off-by: Kat Shen <katshen@amazon.com>
chenqi0805
chenqi0805 previously approved these changes Aug 4, 2023
View reviewed changes
@shenkw1
remove extraneous test
fa8d2f2 
Signed-off-by: Kat Shen <katshen@amazon.com>
dlvenable
dlvenable requested changes Aug 4, 2023
View reviewed changes
Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @shenkw1 ! Overall this is looking good. I have a few quality comments, mostly related to tests.

...r/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java Outdated Show resolved Hide resolved
.../test/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessorTests.java Show resolved Hide resolved
.../test/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessorTests.java Outdated Show resolved Hide resolved
.../test/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessorTests.java Show resolved Hide resolved
@shenkw1
clean up illegal argument statements, parameterize tests
b2c772c 
Signed-off-by: Kat Shen <katshen@amazon.com>
@dlvenable dlvenable merged commit 956e5ad into opensearch-project:main Aug 8, 2023
24 checks passed
@shenkw1 shenkw1 deleted the default-keys branch October 15, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenqi0805 chenqi0805 chenqi0805 approved these changes

@dlvenable dlvenable dlvenable approved these changes

@engechas engechas Awaiting requested review from engechas engechas is a code owner

@graytaylor0 graytaylor0 Awaiting requested review from graytaylor0 graytaylor0 is a code owner

@dinujoh dinujoh Awaiting requested review from dinujoh dinujoh is a code owner

@kkondaka kkondaka Awaiting requested review from kkondaka kkondaka is a code owner

@cmanning09 cmanning09 Awaiting requested review from cmanning09

@asifsmohammed asifsmohammed Awaiting requested review from asifsmohammed

@oeyh oeyh Awaiting requested review from oeyh oeyh is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shenkw1 @dlvenable @chenqi0805