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.

Sign up for GitHub

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

Jump to bottom

[windows] Fix mapping/pipelines for time_created #5384

Merged
merged 15 commits into from
Feb 28, 2023
Merged

[windows] Fix mapping/pipelines for time_created #5384

merged 15 commits into from
Feb 28, 2023

Conversation

nicpenning
Copy link
Contributor

@nicpenning nicpenning commented Feb 25, 2023 •
edited
Loading

  • Bug
  • Enhancement

What does this PR do?

Align with other Windows integrations and the winlog.time_created improper mapping and adjusting the error handling for said pipelines.

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.

Related issues

Link related issues below. Insert the issue link or reference after the word "Closes" if merging this should automatically close it.

@nicpenning nicpenning requested review from a team as code owners February 25, 2023 00:25
@nicpenning nicpenning requested review from faec and leehinman February 25, 2023 00:25
@elasticmachine
Copy link

elasticmachine commented Feb 25, 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-02-26T21:52:03.418+0000

  • Duration: 16 min 14 sec

Test stats 🧪

Test Results
Failed 0
Passed 129
Skipped 0
Total 129

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@nicpenning
Copy link
Contributor Author

I got a little wild trying to find a replace all areas where the winlog.time_created existed and accidently removed the target field of event.created that this should go into which is interesting because most places it does not do that. There seems to be more than what meets the eye with this time_created debacle. @efd6 👋

@nicpenning
Copy link
Contributor Author

Also, there is this issue:
image

Any idea where in the code this can get addressed for the Forwarded logs index template?

efd6
efd6 reviewed Feb 26, 2023
View reviewed changes
packages/windows/data_stream/forwarded/elasticsearch/ingest_pipeline/powershell.yml Outdated
@@ -31,13 +31,21 @@ processors:
ignore_empty_value: true
ignore_failure: true
if: ctx?.winlog?.level != ""
- date:
- date:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- date:
- date:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye

@efd6
Copy link
Contributor

efd6 commented Feb 26, 2023

/test

@elasticmachine
Copy link

elasticmachine commented Feb 26, 2023 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (4/4) 💚
Files 87.5% (7/8) 👎 -9.002
Classes 87.5% (7/8) 👎 -9.002
Methods 83.516% (76/91) 👎 -7.375
Lines 92.634% (5093/5498) 👍 1.287
Conditionals 100.0% (0/0) 💚

@efd6
Copy link
Contributor

efd6 commented Feb 26, 2023 •
edited
Loading

/test

efd6
efd6 reviewed Feb 26, 2023
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

I can't see field definitions for winlog.time_created in powershell, powershell_operational or sysmon_operational. It should be there too, no?

@nicpenning
Copy link
Contributor Author

I can check. In some cases the pipelines remove that field entirely.

@nicpenning
Copy link
Contributor Author

Confirmed, each of those remove this field so it should not exist.

@efd6 efd6 merged commit d4fd194 into elastic:main Feb 28, 2023
@elasticmachine
Copy link

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

@nicpenning nicpenning deleted the patch-5 branch February 28, 2023 03:43
agithomas pushed a commit to agithomas/integrations that referenced this pull request Mar 20, 2023
@nicpenning @agithomas
[windows] Fix mapping/pipelines for time_created (elastic#5384)
9f7079b 
Fix mapping of winlog.time_created in the forwarded data stream and improve
error handling for date processor failures throughout.
agithomas pushed a commit to agithomas/integrations that referenced this pull request Mar 21, 2023
@nicpenning @agithomas
[windows] Fix mapping/pipelines for time_created (elastic#5384)
24eb2a4 
Fix mapping of winlog.time_created in the forwarded data stream and improve
error handling for date processor failures throughout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efd6 efd6 efd6 approved these changes

@faec faec Awaiting requested review from faec faec is a code owner automatically assigned from elastic/elastic-agent-data-plane

@leehinman leehinman Awaiting requested review from leehinman leehinman is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nicpenning @elasticmachine @efd6