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

windows: make pipeline routing robust to channel letter case #8242

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Oct 18, 2023

Proposed commit message

Apparently some events from Windows servers and workstations in Security channel have a lowercase channel name. This has not been observed in other channels, but defensively apply the same care there.

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

Related issues

Screenshots

@elasticmachine
Copy link

elasticmachine commented Oct 18, 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-10-22T21:37:42.383+0000

  • Duration: 18 min 15 sec

Test stats 🧪

Test Results
Failed 0
Passed 150
Skipped 0
Total 150

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Oct 18, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (8/8) 💚
Files 91.667% (11/12) 👎 -3.901
Classes 91.667% (11/12) 👎 -3.901
Methods 85.156% (109/128) 👎 -6.194
Lines 91.55% (5840/6379) 👍 4.248
Conditionals 100.0% (0/0) 💚

@efd6 efd6 marked this pull request as ready for review October 18, 2023 21:02
@efd6 efd6 requested review from a team as code owners October 18, 2023 21:02
@efd6 efd6 requested review from belimawr and rdner October 18, 2023 21:02
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@pierrehilbert pierrehilbert added the Team:Elastic-Agent Label for the Agent team label Oct 19, 2023
@elasticmachine
Copy link

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

Apparently some events from Windows servers and workstations in Security channel
have a lowercase channel name. This has not been observed in other channels, but
defensively apply the same care there.
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

This looks good to me.

But this has me thinking that we should be routing based exclusively on the provider name instead of the channel. The channel names may be customized in the case of forwarded events. Sometimes users will setup their WEC with custom channels so you might have something named "WEC-Security" that holds the data produced by Microsoft-Windows-Security-Auditing. WDYT?

@efd6
Copy link
Contributor Author

efd6 commented Oct 23, 2023

That seem reasonable. Here or later?

@andrewkroh
Copy link
Member

Later. That will give me time to write up an issue tomorrow and think through some use cases relating to WEC. The goal is to make sure that WEC users can get the same behavior as if they had directly collected the logs from a host using Agent. So checking that we consistently apply event.{dataset,module} so that rules, queries, and dashboards work the same with WEC. Make sure that this forwarded dataset has a configurable channel name. And probably more after I think this through.

@efd6 efd6 merged commit 26c8cb3 into elastic:main Oct 23, 2023
1 check passed
@elasticmachine
Copy link

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

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:windows Windows Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants