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

Added LogstashPluginAttributesMapper for custom mapping #568

Merged

Conversation

dlvenable
Copy link
Member

@dlvenable dlvenable commented Nov 11, 2021

Description

I split the behavior of DefaultPluginMapper so that it loads the plugin mapping file and then delegates the attribute mapping to an instance of LogstashPluginAttributesMapper. There is a default implementation named DefaultLogstashPluginAttributesMapper. But, any plugin can configure the class by setting the attributesMapperClass property in the mapping YAML file.

I would like to provide a customizable base class for LogstashPluginAttributesMapper, but will do that in a separate PR after this one has been nailed down.

Issues Resolved

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

…per as the mechanism for mapping plugins. This can be configure in the mapping YAML to allow detailed mapping configurations. opensearch-project#466

Signed-off-by: David Venable <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2021

Codecov Report

Merging #568 (4991078) into main (90f3427) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #568   +/-   ##
=========================================
  Coverage     92.28%   92.28%           
  Complexity      569      569           
=========================================
  Files            71       71           
  Lines          1725     1725           
  Branches        144      144           
=========================================
  Hits           1592     1592           
  Misses          102      102           
  Partials         31       31           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90f3427...4991078. Read the comment docs.

chenqi0805
chenqi0805 previously approved these changes Nov 12, 2021
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.

Overall, the change looks good to me. I am only a little concerned that AttributesMapperProvider and LogstashMappingModel have not yet been covered by tests.

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.

👍

@dlvenable dlvenable merged commit e04a40f into opensearch-project:main Nov 12, 2021
dlvenable added a commit to dlvenable/data-prepper that referenced this pull request Nov 12, 2021
dlvenable added a commit to dlvenable/data-prepper that referenced this pull request Nov 12, 2021
dlvenable added a commit that referenced this pull request Nov 12, 2021
Bug fixes in the DefaultLogstashPluginAttributesMapper related to null values. The LogstashMappingModel now returns empty maps when they are null or absent in the YAML. Part of #568.

Signed-off-by: David Venable <[email protected]>
dlvenable added a commit to dlvenable/data-prepper that referenced this pull request Nov 12, 2021
… to be treating differently from explicit nulls. opensearch-project#568

Signed-off-by: David Venable <[email protected]>
dlvenable added a commit that referenced this pull request Nov 12, 2021
… to be treating differently from explicit nulls. #568 (#579)

Signed-off-by: David Venable <[email protected]>
chenqi0805 added a commit to chenqi0805/data-prepper that referenced this pull request Nov 12, 2021
* main:
  Fixes the main branch from a renaming that happened when two related PRs were merged around the same time. (opensearch-project#583)
  Grpc Basic auth for Otel trace Source (opensearch-project#570)
  Fixed a NullPointerException which LogstashVisitor was throwing for attributes without a value. (opensearch-project#582)
  Update PipelineModel to fix serialization (opensearch-project#577)
  Log analytics getting started (opensearch-project#573)
  Support maps which are not present in the YAML, which Jackson appears to be treating differently from explicit nulls. opensearch-project#568 (opensearch-project#579)
  Add support for codeowners to repo (opensearch-project#578)
  Added logstash mapper which maps logstash configuration to pipeline m… (opensearch-project#575)
  Document OpenSearch Sink configuration parameters number_of_shards and number_of_replicas. (opensearch-project#562)
  Added instructions to build and run the Docker image locally. (opensearch-project#564)
  NPE bug fixes in the DefaultLogstashPluginAttributesMapper (opensearch-project#574)
  switch Path.of to Paths.get (opensearch-project#566)
  Fix PrepperState javadoc (opensearch-project#567)

Signed-off-by: qchea <[email protected]>
@dlvenable dlvenable deleted the logstash-attribute-mapping-466 branch November 13, 2021 20:42
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.

4 participants