Add support for replacing invalid characters in parsing processors#5823
Add support for replacing invalid characters in parsing processors#5823kkondaka merged 6 commits intoopensearch-project:mainfrom
Conversation
| return key.length() > 1 && key.endsWith(SEPARATOR) ? key.substring(0, key.length() - 1) : key; | ||
| } | ||
|
|
||
| static String replaceInvalidCharacters(final String key) { |
There was a problem hiding this comment.
Can we add some tests for this function. I dont see any tests in JacksonEventKeyTest.java
There was a problem hiding this comment.
I added in JacksonEventTest.java. There is no difference because JacksonEvent is just a wrapper.
| private final CsvMapper mapper; | ||
| private final CsvSchema schema; | ||
|
|
||
| private final Boolean normalizeKeys; |
There was a problem hiding this comment.
nit: can this be a primitive boolean
|
|
||
| @JsonProperty(value = "normalize_keys", defaultValue = "false") | ||
| @JsonPropertyDescription("If specified, replaces invalid characters with underscore character") | ||
| private Boolean normalizeKeys = false; |
There was a problem hiding this comment.
should the default behavior be true instead.
There was a problem hiding this comment.
No. We don't want to break existing behavior.
|
|
||
| @Override | ||
| public Boolean getNormalizeKeys() { | ||
| return false; |
There was a problem hiding this comment.
could you please check if we can add a test to cover this.
Signed-off-by: Kondaka <krishkdk@amazon.com>
Signed-off-by: Kondaka <krishkdk@amazon.com>
7a70e0f to
dc647d3
Compare
oeyh
left a comment
There was a problem hiding this comment.
Looks good overall. Some minor issues below:
| private Boolean deleteHeader = DEFAULT_DELETE_HEADERS; | ||
|
|
||
| @JsonProperty(value = "normalize_keys", defaultValue = "false") | ||
| @JsonPropertyDescription("If specified, replaces invalid characters with underscore character") |
There was a problem hiding this comment.
Nit: If set to true is probably clearer. Same for other processors.
| @JsonPropertyDescription("If specified, replaces invalid characters with underscore character") | |
| @JsonPropertyDescription("If set to true, replaces invalid characters with underscore character") |
| if (value instanceof Map) { | ||
| replaceInvalidChars((Map<String, Object>)value); | ||
| } | ||
| if (newKey != key) { |
There was a problem hiding this comment.
This should be if (!newKey.equals(key))
| if (newKey != key) { | |
| if (!newKey.equals(key)) { |
| } | ||
|
|
||
|
|
||
| public static String replaceInvalidKeyChars(final String key) { |
There was a problem hiding this comment.
I think it would be ideal to add this into the builder.
final Log event = eventFactory.eventBuilder(LogEventBuilder.class)
.withReplaceInvalidCharacters(true)
.withData(json)
.build();
All the work will be part of the EventBuilder code.
Then the codecs have very little work to do!
There was a problem hiding this comment.
This is good idea but the current changes do not need this
| if (normalizeKeys) { | ||
| key = JacksonEvent.replaceInvalidKeyChars(key); | ||
| } | ||
| event.put(key, data.get(providedHeaderColIdx)); |
There was a problem hiding this comment.
This could be:
event.put(key, data.get(providedHeaderColIdx, true);
Where the last boolean is a property to replace invalid keys.
Signed-off-by: Kondaka <krishkdk@amazon.com>
Signed-off-by: Kondaka <krishkdk@amazon.com>
| } | ||
| } | ||
|
|
||
| if (normalizeKeys) { |
There was a problem hiding this comment.
Can we remove this code now? And this utility method can be package protected to avoid using it.
Signed-off-by: Kondaka <krishkdk@amazon.com>
Signed-off-by: Kondaka <krishkdk@amazon.com>
dlvenable
left a comment
There was a problem hiding this comment.
Nice, thank you for consolidating the normalization logic!
…pensearch-project#5823) * Add support for replacing invalid characters in parsing processors Signed-off-by: Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Kondaka <krishkdk@amazon.com> * Fixed javadoc error Signed-off-by: Kondaka <krishkdk@amazon.com> * Addressed review comments Signed-off-by: Kondaka <krishkdk@amazon.com> * Fixed checkstyle errors Signed-off-by: Kondaka <krishkdk@amazon.com> --------- Signed-off-by: Kondaka <krishkdk@amazon.com> Signed-off-by: Jonah Calvo <caljonah@amazon.com>
Description
Add support for replacing invalid characters in parsing processors
When
normalize_keyconfig option is used, csv processor, key-value processor and parse-json processor would replace invalid characters in the keys with "_"(underscore). These three processors currently creates keys with invalid characters.parse-ion and parse-xml processors do not generate keys with invalid keys currently and that behavior stays same.
Issues Resolved
Resolves #5757
Check List
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.