Skip to content

[Apache] Add condition support to logs and metrics#7372

Merged
ishleenk17 merged 2 commits intoelastic:mainfrom
BenB196:apache-condition-support
Sep 4, 2023
Merged

[Apache] Add condition support to logs and metrics#7372
ishleenk17 merged 2 commits intoelastic:mainfrom
BenB196:apache-condition-support

Conversation

@BenB196
Copy link
Copy Markdown
Contributor

@BenB196 BenB196 commented Aug 13, 2023 •
edited
Loading

What does this PR do?

Enhancement - Adds support for setting condition to both Log file collection and metrics. This allows for defining this integration in a policy, but only having it run on during specific conditions.

Note: Log file (access/error) share a top-level condition value, and metrics is an independent value.

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.

Author's Checklist

  • [ ]

How to test this PR locally

Install new version, confirm that condition fields are available and can be set.

Related issues

Screenshots

image

image

@BenB196 BenB196 requested a review from a team as a code owner August 13, 2023 23:45
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Aug 13, 2023 •
edited
Loading

💚 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-09-04T04:56:09.815+0000

  • Duration: 16 min 56 sec

Test stats 🧪

Test Results
Failed 0
Passed 25
Skipped 0
Total 25

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@ishleenk17
Copy link
Copy Markdown
Member

ishleenk17 commented Aug 30, 2023 •
edited
Loading

@BenB196 : Could you please specify a usecase for which you think this will be helpful in Apache ?

@BenB196
Copy link
Copy Markdown
Contributor Author

BenB196 commented Aug 30, 2023

Hi @ishleenk17 this would be helpful for Apache as it would allow for having a single Elastic Agent policy that is applied to both hosts which have an Apache process running and hosts that do not. By leveraging the condition value, it can be specified which hosts this integration should be run on.

@ishleenk17
Copy link
Copy Markdown
Member

ishleenk17 commented Aug 30, 2023

Hi @ishleenk17 this would be helpful for Apache as it would allow for having a single Elastic Agent policy that is applied to both hosts which have an Apache process running and hosts that do not. By leveraging the condition value, it can be specified which hosts this integration should be run on.

Any specific reason why we are focusing on single policy when the same can be attained by having 2 different policies?

@BenB196
Copy link
Copy Markdown
Contributor Author

BenB196 commented Aug 30, 2023

Hi @ishleenk17, while technically possible to do this via 2 policies this method will devolve into exponential (matrix multiplication) operational complexity.

One of the main reasons for the condition setting to exist in the first place is to avoid this problem of needing to use multiple different policies.

FWIW, this isn't a "new" concept, there are already a number of integrations which support this field: https://github.com/search?q=repo%3Aelastic%2Fintegrations+%22title%3A+Condition%22&type=code

@ishleenk17
Copy link
Copy Markdown
Member

ishleenk17 commented Aug 30, 2023

Hi @ishleenk17, while technically possible to do this via 2 policies this method will devolve into exponential (matrix multiplication) operational complexity.

One of the main reasons for the condition setting to exist in the first place is to avoid this problem of needing to use multiple different policies.

FWIW, this isn't a "new" concept, there are already a number of integrations which support this field: https://github.com/search?q=repo%3Aelastic%2Fintegrations+%22title%3A+Condition%22&type=code

Sounds good. Thanks for your contribution @BenB196

ishleenk17
ishleenk17 approved these changes Sep 4, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Looks good!

@ishleenk17
Copy link
Copy Markdown
Member

ishleenk17 commented Sep 4, 2023

/test

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Sep 4, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 50.0% (2/4) 👎 -46.638
Classes 50.0% (2/4) 👎 -46.638
Methods 70.0% (28/40) 👎 -22.417
Lines 70.234% (210/299) 👎 -18.036
Conditionals 100.0% (0/0) 💚

@ishleenk17 ishleenk17 merged commit 8aac25c into elastic:main Sep 4, 2023
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Sep 4, 2023

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

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Sep 4, 2023

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

@BenB196 BenB196 deleted the apache-condition-support branch November 29, 2023 12:36
@andrewkroh andrewkroh added the Integration:apache Apache HTTP Server label Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ishleenk17 ishleenk17 ishleenk17 approved these changes

Assignees

No one assigned

Labels

Integration:apache Apache HTTP Server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@BenB196 @elasticmachine @ishleenk17 @andrewkroh