[system + winlog] Fix mapping for time_created to date #5350

nicpenning · 2023-02-21T17:51:22Z

Bug

What does this PR do?

This sets the winlog.time_created field (currently keyword) to a date type. Even though that I have no data that has this field any longer (in Winlogbeat or Elastic Agent System or Custom Windows event log integrations), I see that it exists in the sample json files provided in the pipelines directory for testing, so instead of removing it, I am updating to a date as it should be.

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.

elasticmachine · 2023-02-21T17:54:36Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-02-23T01:49:52.859+0000
Duration: 15 min 25 sec

Test stats 🧪

Test	Results
Failed	0
Passed	196
Skipped	0
Total	196

🤖 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 · 2023-02-21T21:47:14Z

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

efd6

I'm not sure we can do this safely; in system, if the date processor fails we have a field that may fail ingest and in winlog there is no pipeline, so we have no control of the format of the value in the field and so again risk an ingest failure. The system case could be made more safe, but winlog has no processor by design.

Also, what are the implications for existing installations with a mapping change?

nicpenning · 2023-02-21T22:48:02Z

Today, it seems that this field doesn't exist anyways. In the System System, and Application mappings, this field does not exist so they are not in conflict with the properly mapped Winlogbeat mappings.

Do you have any modern examples with the Elastic Agent and of the System or Custom Windows logs that this field exists?

None the less, the other option is to remove this field mapping entirely.

Right now it is inconsistent between the System Integrations, Windows and Custom Windows integrations.

What would you recommend?

efd6 · 2023-02-21T23:05:20Z

Thanks, the mapping in winlogbeat supports this change.

I think I'd suggest leaving the winlog integration as you have it; the responsibility I guess lies on the user to provide an appropriate pipeline to handle the data stream. For the system/security it could be made safe by adding an on_failure option to the date processor (and modifying the pipeline on_failure option).

Something like

  - date:
      tag: "time_created_date"
      field: winlog.time_created
      formats:
        - ISO8601
      if: ctx.winlog?.time_created != null
      on_failure:
        - remove:
            field: winlog.time_created
            ignore_failure: true
        - append:
            field: error.message
            value: "fail-{{{ _ingest.on_failure_processor_tag }}}"
        - fail:
            message: "Processor {{ _ingest.on_failure_processor_type }} with tag {{ _ingest.on_failure_processor_tag }} in pipeline {{ _ingest.on_failure_pipeline }} failed with message: {{ _ingest.on_failure_message }}"
on_failure:
  - set:
      field: event.kind
      value: pipeline_error
  - append:
      field: error.message
      value: "{{ _ingest.on_failure_message }}"

Could also consider a rename instead of a remove in the case of a failure.

nicpenning · 2023-02-21T23:23:21Z

I like the fail safe in the pipeline. I am good with adding that.

I am wondering if the sample document came from Splunk or a third party ingestion tool because of the 10k winlogbeat endpoints and many event channels, time_created doesn't appear to exist. We only have about 30 Elastic agents with the reapective integrations but no matches yet. I see the sample doc was from 2019 so there could have been a change in the last 3-4 years for this source.

nicpenning · 2023-02-22T00:04:19Z

I went ahead and adapted your feedback for adjusting the pipeline.

efd6 · 2023-02-22T00:07:29Z

/test

efd6 · 2023-02-22T00:09:26Z

packages/system/data_stream/security/elasticsearch/ingest_pipeline/default.yml

@@ -61,8 +61,17 @@ processors:
      field: winlog.time_created


We do need the tag here.

Okay, I will add it in.

elasticmachine · 2023-02-22T00:21:49Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`3/3`)	💚
Files	100.0% (`4/4`)	💚
Classes	100.0% (`4/4`)	💚
Methods	60.976% (`50/82`)	👍 35.976
Lines	99.535% (`2780/2793`)	👎 -0.465
Conditionals	100.0% (`0/0`)	💚

nicpenning · 2023-02-22T02:13:22Z

Corrected, please test.

nicpenning · 2023-02-22T02:33:25Z

/test

efd6 · 2023-02-22T02:38:05Z

packages/system/data_stream/security/elasticsearch/ingest_pipeline/default.yml

+            field: error.message
+            value: "fail-{{{ _ingest.on_failure_processor_tag }}}"
+        - fail:
+            message: "Processor {{ _ingest.on_failure_processor_type }} with tag {{ _ingest.on_failure_processor_tag }} in pipeline {{ _ingest.on_failure_pipeline }} failed with message: {{ _ingest.on_failure_message }}"
 on_failure:
  - set:


I just noticed this. Please make this an append, otherwise the fail tag will be lost. Also set the event.kind as shown above.

I'd prefer that these were set in the global on_failure. This way if we add others in the future, they are already handled.

nicpenning · 2023-02-22T02:52:24Z

How is that?

efd6 · 2023-02-22T03:22:33Z

packages/system/data_stream/security/elasticsearch/ingest_pipeline/default.yml

+            field: error.message
+            value: "fail-{{{ _ingest.on_failure_processor_tag }}}"
+        - fail:
+            message: "Processor {{ _ingest.on_failure_processor_type }} with tag {{ _ingest.on_failure_processor_tag }} in pipeline {{ _ingest.on_failure_pipeline }} failed with message: {{ _ingest.on_failure_message }}"
 on_failure:
  - set:


I'd prefer that these were set in the global on_failure. This way if we add others in the future, they are already handled.

efd6 · 2023-02-22T03:23:16Z

packages/system/data_stream/security/elasticsearch/ingest_pipeline/default.yml

+        - set:
+            field: event.kind
+            value: pipeline_error
+        - append:


Please revert eab6083.

I couldn't find a way to revert here on github.com so I applied changes to align to what it was before this commit.

efd6 · 2023-02-22T03:25:29Z

packages/system/data_stream/security/elasticsearch/ingest_pipeline/default.yml

 on_failure:
  - set:
      field: error.message


The global on_failure should be

on_failure: - set: field: event.kind value: pipeline_error - append: field: error.message value: "{{ _ingest.on_failure_message }}"

This way local formatting of the failure message is preserved in conjunction with any failure tags.

Do I remove this then?

- set: field: error.message value: |- Processor "{{ _ingest.on_failure_processor_type }}" with tag "{{ _ingest.on_failure_processor_tag }}" in pipeline "{{ _ingest.on_failure_pipeline }}" failed with message "{{ _ingest.on_failure_message }}"

Yep. If you take the suggestion I started with and replace all of the text in the file from the date processor on with that, then you have what we need.

nicpenning · 2023-02-22T03:49:47Z

I think I am getting closer. Sorry this one has been a rough go :D

nicpenning · 2023-02-22T04:13:06Z

Sorry pushing the wrong buttons on my phone. Ready to test?

efd6 · 2023-02-22T04:39:42Z

Can you amend the global on_failure option to set event.kind and use an append in place of a set for error.message?

nicpenning · 2023-02-22T05:04:34Z

Sorry I forgot to put those back in as intended.

Please give that a go.

efd6 · 2023-02-22T05:27:59Z

packages/system/data_stream/security/elasticsearch/ingest_pipeline/default.yml

      value: |-
        Processor "{{ _ingest.on_failure_processor_type }}" with tag "{{ _ingest.on_failure_processor_tag }}" in pipeline "{{ _ingest.on_failure_pipeline }}" failed with message "{{ _ingest.on_failure_message }}"


Suggested change

value: |-

Processor "{{ _ingest.on_failure_processor_type }}" with tag "{{ _ingest.on_failure_processor_tag }}" in pipeline "{{ _ingest.on_failure_pipeline }}" failed with message "{{ _ingest.on_failure_message }}"

value: "{{ _ingest.on_failure_message }}"

The errors are already formatted in the fail processor.

Understood. Updated.

efd6 · 2023-02-23T01:49:38Z

/test

efd6

Thanks

nicpenning · 2023-02-23T02:42:00Z

You are welcome! Thanks for the help!

elasticmachine · 2023-02-23T08:48:32Z

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

elasticmachine · 2023-02-23T08:48:33Z

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

Update winlog.yml

31a9e9b

nicpenning requested a review from a team as a code owner February 21, 2023 17:51

nicpenning requested review from belimawr and leehinman February 21, 2023 17:51

Update changelog.yml

03dc374

nicpenning added 5 commits February 21, 2023 11:55

Update manifest.yml

373477f

Update changelog.yml

a7fc66c

Update manifest.yml

8aca662

Update README.md

fb4a41f

Update winlog.yml

3841bc6

nicpenning requested a review from a team as a code owner February 21, 2023 17:58

nicpenning added 3 commits February 21, 2023 12:00

Update changelog.yml

acd985f

Update manifest.yml

6a32023

Update README.md

e8317a6

nicpenning changed the title ~~[system] Fix mapping for security datastream time_created to date~~ [system + winlog] Fix mapping for time_created to date Feb 21, 2023

jamiehynds added Team:Security-External Integrations Integration:windows Windows Integration:system System labels Feb 21, 2023

efd6 reviewed Feb 21, 2023

View reviewed changes

Add failure checks in pipeline

c5b2cbd

efd6 reviewed Feb 22, 2023

View reviewed changes

Update default.yml

39b57a6

Update default.yml

92ef74e

efd6 reviewed Feb 22, 2023

View reviewed changes

Update default.yml

eab6083

efd6 reviewed Feb 22, 2023

View reviewed changes

Update default.yml

8541f20

nicpenning requested review from efd6 and removed request for belimawr and leehinman February 22, 2023 04:12

nicpenning added 2 commits February 21, 2023 22:59

Update default.yml

192a022

Update default.yml

8fef282

efd6 reviewed Feb 22, 2023

View reviewed changes

Update default.yml

2aeef45

efd6 approved these changes Feb 23, 2023

View reviewed changes

rdner approved these changes Feb 23, 2023

View reviewed changes

efd6 merged commit 9c89a60 into elastic:main Feb 23, 2023

nicpenning deleted the patch-5 branch February 23, 2023 13:03

nicpenning mentioned this pull request Feb 25, 2023

[windows] Fix mapping/pipelines for time_created #5384

Merged

4 tasks

agithomas pushed a commit to agithomas/integrations that referenced this pull request Mar 20, 2023

[system + winlog] Fix mapping for time_created to date (elastic#5350)

5d5f407

agithomas pushed a commit to agithomas/integrations that referenced this pull request Mar 21, 2023

[system + winlog] Fix mapping for time_created to date (elastic#5350)

6e044c7

		value: \|-
		Processor "{{ _ingest.on_failure_processor_type }}" with tag "{{ _ingest.on_failure_processor_tag }}" in pipeline "{{ _ingest.on_failure_pipeline }}" failed with message "{{ _ingest.on_failure_message }}"

	value: \|-
	Processor "{{ _ingest.on_failure_processor_type }}" with tag "{{ _ingest.on_failure_processor_tag }}" in pipeline "{{ _ingest.on_failure_pipeline }}" failed with message "{{ _ingest.on_failure_message }}"
	value: "{{ _ingest.on_failure_message }}"

[system + winlog] Fix mapping for time_created to date #5350

[system + winlog] Fix mapping for time_created to date #5350

Uh oh!

Conversation

nicpenning commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Checklist

Uh oh!

elasticmachine commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

Uh oh!

elasticmachine commented Feb 21, 2023

Uh oh!

efd6 left a comment

Choose a reason for hiding this comment

Uh oh!

nicpenning commented Feb 21, 2023

Uh oh!

efd6 commented Feb 21, 2023

Uh oh!

nicpenning commented Feb 21, 2023

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

efd6 commented Feb 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌐 Coverage report

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

efd6 Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efd6 Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

efd6 commented Feb 22, 2023

Uh oh!

nicpenning commented Feb 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efd6 commented Feb 23, 2023

nicpenning commented Feb 21, 2023 •

edited

Loading

elasticmachine commented Feb 21, 2023 •

edited

Loading

elasticmachine commented Feb 22, 2023 •

edited

Loading

efd6 Feb 22, 2023 •

edited

Loading

efd6 Feb 22, 2023 •

edited

Loading