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

Adds example annotations to a handful of processors #5140

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Adds example annotations to a handful of processors #5140

merged 2 commits into from
Nov 14, 2024

Conversation

graytaylor0
Copy link
Member

@graytaylor0 graytaylor0 commented Oct 31, 2024 •
edited
Loading

Description

Adds examples annotations for the following processors

  • add_entries
  • copy_values
  • convert_entry_type
  • aggregate
  • anomaly_detector
  • delay
  • csv
  • decompress

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • 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.

oeyh
oeyh previously approved these changes Nov 1, 2024
View reviewed changes
dlvenable
dlvenable requested changes Nov 1, 2024
View reviewed changes
.../java/org/opensearch/dataprepper/plugins/processor/decompress/DecompressProcessorConfig.java Outdated
@@ -31,6 +32,9 @@ public class DecompressProcessorConfig {
@JsonPropertyDescription("The type of decompression to use for the keys in the event. Only <code>gzip</code> is supported.")
@JsonProperty("type")
@NotNull
@ExampleValues({
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid example values for enumerations. The enumeration set is the full list of examples.

...lugins/common/src/main/java/org/opensearch/dataprepper/plugins/processor/DelayProcessor.java Outdated
@@ -63,6 +64,11 @@ public void shutdown() {
public static class Configuration {
@JsonProperty("for")
@JsonPropertyDescription("The duration of time to delay. Defaults to <code>1s</code>.")
@ExampleValues({
@ExampleValues.Example(value = "1s", description = "Delays for 1 second."),
Copy link
Member

Choose a reason for hiding this comment

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

You can import Example so that you have @Example throughout.

dlvenable
dlvenable requested changes Nov 12, 2024
View reviewed changes
...opensearch/dataprepper/plugins/processor/anomalydetector/AnomalyDetectorProcessorConfig.java
@@ -34,14 +36,17 @@ public class AnomalyDetectorProcessorConfig {

@JsonPropertyDescription("If provided, anomalies will be detected within each unique instance of these keys. For example, if you provide the ip field, anomalies will be detected separately for each unique IP address.")
@JsonProperty("identification_keys")
@ExampleValues({
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need an example here since this is just a key.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same here. I think for lists it doesn't hurt to have a placeholder value

...essor/src/main/java/org/opensearch/dataprepper/plugins/processor/csv/CsvProcessorConfig.java
@@ -61,11 +63,17 @@ public class CsvProcessorConfig {
"the header in <code>column_names_source_key</code> generates the event fields. If too few columns are specified " +
"in this field, the remaining column names are automatically generated. " +
"If too many column names are specified in this field, the CSV processor omits the extra column names.")
@ExampleValues({
Copy link
Member

Choose a reason for hiding this comment

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

This fits in the category of source examples which I don't see as adding a whole lot of value.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think for the list values like this, the example is helpful. This makes it clear what to input even though the resulting config is a list and the description shows a full List

...n/java/org/opensearch/dataprepper/plugins/processor/mutateevent/AddEntryProcessorConfig.java
@@ -84,6 +94,11 @@ public static class Entry {
@AlsoRequired.Required(name="format", allowedValues = {"null"}),
@AlsoRequired.Required(name=VALUE_EXPRESSION_KEY, allowedValues = {"null"})
})
@ExampleValues({
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps include a number as well.

...rg/opensearch/dataprepper/plugins/processor/mutateevent/ConvertEntryTypeProcessorConfig.java Outdated
@@ -70,6 +72,10 @@ public class ConvertEntryTypeProcessorConfig implements ConverterArguments {

@JsonProperty("convert_when")
@JsonPropertyDescription("Specifies a condition using a <a href=\"https://opensearch.org/docs/latest/data-prepper/pipelines/expression-syntax/\">conditional expression</a> for performing the <code>convert_entry_type</code> operation. If specified, the <code>convert_entry_type</code> operation runs only when the expression evaluates to true. Example: <code>/mykey != \"---\"</code>")
@ExampleValues({
@Example(value = "/some_key == null", description = "Only runs the convert_entry_type processor on the Event if the existing key some_key is null or does not exist."),
Copy link
Member

Choose a reason for hiding this comment

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

Please use convert_type in the documentation.

...rg/opensearch/dataprepper/plugins/processor/mutateevent/ConvertEntryTypeProcessorConfig.java Outdated
@@ -70,6 +72,10 @@ public class ConvertEntryTypeProcessorConfig implements ConverterArguments {

@JsonProperty("convert_when")
@JsonPropertyDescription("Specifies a condition using a <a href=\"https://opensearch.org/docs/latest/data-prepper/pipelines/expression-syntax/\">conditional expression</a> for performing the <code>convert_entry_type</code> operation. If specified, the <code>convert_entry_type</code> operation runs only when the expression evaluates to true. Example: <code>/mykey != \"---\"</code>")
@ExampleValues({
@Example(value = "/some_key == null", description = "Only runs the convert_entry_type processor on the Event if the existing key some_key is null or does not exist."),
@Example(value = "/some_key typeof integer", description = "Only runs the convert_entry_type processor on the Event if the key some_key is an integer.")
Copy link
Member

Choose a reason for hiding this comment

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

Please use convert_type in the documentation.

.../java/org/opensearch/dataprepper/plugins/processor/mutateevent/CopyValueProcessorConfig.java Outdated
@@ -35,6 +37,9 @@ public static class Entry {
@NotNull
@JsonProperty("to_key")
@JsonPropertyDescription("The key of the new entry to be added.")
@ExampleValues({
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid source/target examples.

@graytaylor0
Address PR comments
ecca218 
Signed-off-by: Taylor Gray <[email protected]>
@graytaylor0 graytaylor0 merged commit dbfb1d3 into opensearch-project:main Nov 14, 2024
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shenkw1 shenkw1 shenkw1 approved these changes

@dlvenable dlvenable dlvenable approved these changes

@oeyh oeyh oeyh left review comments

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

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

@engechas engechas Awaiting requested review from engechas engechas 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

@KarstenSchnitter KarstenSchnitter Awaiting requested review from KarstenSchnitter KarstenSchnitter 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.
4 participants
@graytaylor0 @dlvenable @oeyh @shenkw1