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.

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

Support allow_duplicate_keys for json processor #11059

Conversation

MunifTanjim
Copy link

@MunifTanjim MunifTanjim commented Nov 2, 2023

Description

This is to support creating an ingest pipeline for ECS JSON logs.
Due to how some logging frameworks work, ECS JSON logs may contain duplicate keys. Instead of failing, the JSON parser should be more lenient and prefer the last value.

Ported from elastic/elasticsearch#74956

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from aa08236 to 59121c1 Compare November 2, 2023 12:36
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Compatibility status:

Checks if related components are compatible with change 7498602

Incompatible components

Incompatible components: [https://github.com/opensearch-project/performance-analyzer.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/sql.git]

Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from 59121c1 to 3d434b0 Compare November 2, 2023 12:43
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from 3d434b0 to 9492ce9 Compare November 2, 2023 13:03
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch 2 times, most recently from 8d272d2 to b6326b5 Compare November 2, 2023 13:31
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from b6326b5 to e787739 Compare November 2, 2023 14:04
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from e787739 to d39a702 Compare November 2, 2023 14:14
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from d39a702 to 1a1420e Compare November 2, 2023 14:35
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@MunifTanjim MunifTanjim force-pushed the json-allow_duplicate_keys branch from 1a1420e to 7498602 Compare November 2, 2023 15:22
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #11059 (7498602) into main (63aff16) will decrease coverage by 0.04%.
Report is 5 commits behind head on main.
The diff coverage is 70.00%.

@@             Coverage Diff              @@
##               main   #11059      +/-   ##
============================================
- Coverage     71.29%   71.25%   -0.04%     
- Complexity    58742    58750       +8     
============================================
  Files          4872     4872              
  Lines        276777   276791      +14     
  Branches      40240    40241       +1     
============================================
- Hits         197316   197218      -98     
- Misses        62943    63157     +214     
+ Partials      16518    16416     -102     
Files Coverage Δ
...a/org/opensearch/core/xcontent/XContentParser.java 100.00% <ø> (ø)
...earch/common/xcontent/cbor/CborXContentParser.java 100.00% <100.00%> (ø)
...earch/common/xcontent/json/JsonXContentParser.java 83.33% <100.00%> (+0.40%) ⬆️
...rch/common/xcontent/smile/SmileXContentParser.java 100.00% <100.00%> (ø)
...va/org/opensearch/ingest/common/JsonProcessor.java 92.59% <100.00%> (+0.43%) ⬆️
...s/replication/SegmentReplicationTargetService.java 56.17% <100.00%> (+0.56%) ⬆️
...rg/opensearch/core/xcontent/MapXContentParser.java 72.61% <0.00%> (-0.47%) ⬇️
...ch/common/xcontent/JsonToStringXContentParser.java 43.90% <0.00%> (-0.55%) ⬇️
...rg/opensearch/core/xcontent/XContentSubParser.java 44.28% <0.00%> (-1.31%) ⬇️
.../java/org/opensearch/ingest/common/Processors.java 0.00% <0.00%> (ø)

... and 477 files with indirect coverage changes

@MunifTanjim MunifTanjim marked this pull request as ready for review November 2, 2023 16:09
@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Dec 3, 2023
@MunifTanjim
Copy link
Author

This is still relevant! @opensearch-ci-bot

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Dec 4, 2023
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Thanks, not sure how we missed the PR. Sorry about that. Please iterate to green. Needs a CHANGELOG notably.

I don't know much about the xcontent business, maybe @reta can review this? Who else?

@reta
Copy link
Collaborator

reta commented Dec 19, 2023

I don't know much about the xcontent business, maybe @reta can review this? Who else?

Thanks for the pull request @MunifTanjim but we cannot port features from elasticsearch "as-is" without violating license

@dblock
Copy link
Member

dblock commented Dec 21, 2023

Oh I didn't notice the port from Elastic. That PR is from Jul 6, 2021 which is after 7.10.2 I believe, closing this PR.

Anyone wanting this feature in OpenSearch: please ensure that you're not copying any non-APLv2 code.

@dblock dblock closed this Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants