Skip to content

Added support for processors for all oracle integrations #6537

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

Merged
merged 6 commits into from
Aug 1, 2023

Conversation

philippkahr
Copy link
Contributor

@philippkahr philippkahr commented Jun 12, 2023

What does this PR do?

Adds support for the processors on all oracle integrations

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

@philippkahr philippkahr added the enhancement New feature or request label Jun 12, 2023
@philippkahr philippkahr requested a review from a team as a code owner June 12, 2023 07:38
@elasticmachine
Copy link

elasticmachine commented Jun 12, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-08-01T07:55:20.999+0000

  • Duration: 34 min 8 sec

Test stats 🧪

Test Results
Failed 0
Passed 26
Skipped 0
Total 26

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@agithomas
Copy link
Contributor

I call for a discussion to determine the impact of this change and TSDB enablement as Oracle package is an immediate candidate for TSDB enablement.

How to ensure that the fields marked as dimensions are not removed? I read that Processors are used to reduce the number of fields in the exported event or to enhance the event with metadata

@philippkahr
Copy link
Contributor Author

You can use the processors on the logs side of things in majority of all packages. You can use it to add fields, that are not possible otherwise. It's a weird situation that one integration allows the configuration of the processors and another one does not.

Assume you are a customer and you want to add a set of field to every integration, since you rely on that field to determine security, e.g. document level security. You cannot do this centrally in an ingest pipeline, since you don't know where the data came from. The best possibility is to do this on the agent itself and therefore the integration to append an add fields processor and add your department, or whatever you want. Thus allowing you to enrich the data when collected.

@elasticmachine
Copy link

💔 Build Failed

Failed CI Steps

@elasticmachine
Copy link

elasticmachine commented Jun 13, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (1/1) 💚
Classes 100.0% (1/1) 💚
Methods 100.0% (29/29) 💚 10.0
Lines 94.318% (249/264) 👎 -3.154
Conditionals 100.0% (0/0) 💚

@philippkahr
Copy link
Contributor Author

@agithomas can you review again? Not really sure what I am supposed to do to resolve the conflicts.

@ishleenk17
Copy link
Member

I call for a discussion to determine the impact of this change and TSDB enablement as Oracle package is an immediate candidate for TSDB enablement.

How to ensure that the fields marked as dimensions are not removed? I read that Processors are used to reduce the number of fields in the exported event or to enhance the event with metadata

Agi, this is a common functionality across integrations. Having a provision to specify processors for data enrichment.
Lets discuss it in one of our meetings from TSDB perspective.

@agithomas agithomas removed their request for review July 21, 2023 13:11
@ishleenk17
Copy link
Member

@philippkahr : Changes look good. Please fix the. conflicts and merge

Copy link
Member

@ishleenk17 ishleenk17 left a comment

Choose a reason for hiding this comment

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

Changes look good. Please resolve the conflicts and merge.

@philippkahr philippkahr merged commit f275092 into elastic:main Aug 1, 2023
@philippkahr philippkahr deleted the philippkahr-patch-oracle branch August 1, 2023 08:38
@elasticmachine
Copy link

Package oracle - 1.20.0 containing this change is available at https://epr.elastic.co/search?package=oracle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:oracle Oracle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants