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

Custom Windows event logs #794

Merged
merged 5 commits into from
Apr 2, 2021
Merged

Conversation

leehinman
Copy link
Contributor

@leehinman leehinman commented Mar 16, 2021

What does this PR do?

Adds a new integration for adding custom Windows event logs

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.

Related issues

Screenshots

Catalog Screenshot

Screen Shot 2021-03-16 at 10 58 34

Overview Screenshot

Screen Shot 2021-03-22 at 16 13 27

Add Integration Screenshot

Screen Shot 2021-03-22 at 16 14 30

Advanced Options Screenshot

Screen Shot 2021-03-16 at 11 00 41

@fearful-symmetry
Copy link
Contributor

Is there a reason this is its own integration? We might just want to put it in the Windows integration.

@leehinman
Copy link
Contributor Author

Is there a reason this is its own integration? We might just want to put it in the Windows integration.

:-) This is an ongoing debate. If you have multiple custom logs, you have to add the integration multiple times and it was felt that adding the Windows integration multiple times was "odd". So having it's own integration was thought to be less confusing.

Long term we want to use the "multi" option from elastic/package-spec#110 and I think at that time we would move it to the Windows integration.

@fearful-symmetry
Copy link
Contributor

This is an ongoing debate. If you have multiple custom logs, you have to add the integration multiple times and it was felt that adding the Windows integration multiple times was "odd". So having it's own integration was thought to be less confusing.

Ahh, okay, makes sense. Yah, over time we'll definitely want to integrate it. In general, the Integration install/management process isn't great for discoverability.

@elasticmachine
Copy link

elasticmachine commented Mar 16, 2021

💚 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

Expand to view the summary

Build stats

  • Build Cause: Pull request #794 updated

  • Start Time: 2021-04-02T01:31:17.460+0000

  • Duration: 59 min 15 sec

  • Commit: 8776247

Test stats 🧪

Test Results
Failed 0
Passed 1923
Skipped 3
Total 1926

Trends 🧪

Image of Build Times

Image of Tests

@andresrc
Copy link
Collaborator

@leehinman I understand we are using this as an "input" package (/cc @ruflin )

@ruflin
Copy link
Member

ruflin commented Mar 17, 2021

I would consider this an input package. Even though this is a bit special as winlog is already very structured.

@leehinman
Copy link
Contributor Author

@andresrc or @ruflin sorry, I sounds like "input package" has special meaning, but I'm not aware of it. Could you clarify? or point me to issue/docs? I just want to make sure I'm not supposed to be doing something different.

@jamiehynds
Copy link

jamiehynds commented Mar 22, 2021

@leehinman users may be unsure what to populate the dataset name with. Does the value default to generic or do we need to pre-populate with a default value, e.g. windows.custom

@leehinman leehinman marked this pull request as ready for review March 22, 2021 21:20
@elasticmachine
Copy link

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

required: false
multi: true
show_user: false
- name: custom
Copy link
Member

@andrewkroh andrewkroh Mar 23, 2021

Choose a reason for hiding this comment

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

Does this need show_user: false? Checking the spec, this supposedly defaults to false true. https://github.com/elastic/package-spec/blob/a8fe73d82b25806f6b96582b99993a63b67ef130/versions/1/data_stream/manifest.spec.yml#L83

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No not needed, I can remove.

I often include it because it controls inclusion in "Advanced" items, so while developing it makes it easy to toggle.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry 🤦 , I was really unclear in that comment. What I meant was, does custom need to add show_user: false because it's an advanced setting? And I misquoted the link, the spec says the default is show_user: true, so without it I assume custom is always being shown to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you found a Fleet GUI bug. So if show_user: false or show_user is missing, then "Custom Configurations" showed up in the "Advanced Settings". If show_user:true, then it is always shown to the user. Weird.

Anyway, I'm adding show_user:false to "Custom Configurations".

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.

👍

@leehinman leehinman merged commit 97c25e4 into elastic:master Apr 2, 2021
@leehinman leehinman deleted the 317_new_custom_winlog branch September 28, 2021 20:22
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
* Custom Windows event logs
@hogemo
Copy link

hogemo commented Apr 4, 2024

Is it possible to listen for multiple channels for one integration? Or do i specifically need one integration per channel?

@andrewkroh andrewkroh added Integration:winlog Custom Windows Event Logs New Integration and removed Integration:windows Windows labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration:winlog Custom Windows Event Logs New Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Windows] Custom Event Channel support
8 participants