Skip to content

[7.x] Convert event log's duration from number to string in Kibana (keep as "long" in Elasticsearch)#144837

Merged
mikecote merged 6 commits intoelastic:7.17from
mikecote:backport-event-log-duration-fix
Nov 10, 2022
Merged

[7.x] Convert event log's duration from number to string in Kibana (keep as "long" in Elasticsearch)#144837
mikecote merged 6 commits intoelastic:7.17from
mikecote:backport-event-log-duration-fix

Conversation

@mikecote
Copy link
Copy Markdown
Contributor

@mikecote mikecote commented Nov 8, 2022

Fixes #143562.

Backport of #130819 to 7.x.

Steps to verify

  1. Fake alert event log durations to be 1 year by changing the following code

https://github.com/elastic/kibana/pull/144837/files#diff-27ff694dc534e9e509d43b00c563eabada722db2371c9bcf016ff3c8b929c5fbR706
const duration = millisToNanos(31556926000);

  1. Create a rule that creates an alert consistently (ex: index threshold)

  2. Let the rule run a few times

  3. See event log now capturing long durations successfully

curl -XGET 'https://elastic:changeme@localhost:9200/.kibana-event-log*/_search?pretty' -k

@mikecote mikecote added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Feature:EventLog Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework v7.17.8 labels Nov 8, 2022
@mikecote mikecote self-assigned this Nov 8, 2022
@mikecote
Copy link
Copy Markdown
Contributor Author

mikecote commented Nov 9, 2022

@elasticmachine merge upstream

@mikecote
Copy link
Copy Markdown
Contributor Author

mikecote commented Nov 9, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution / Alert details with unmapped fields "before each" hook for "Displays the unmapped field on the JSON view"
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution / Detection rules, machine learning Creates and activates a new ml rule
  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution / Detection rules, override Creates and activates a new custom rule with override option

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
eventLog 84 92 +8
Unknown metric groups

API count

id before after diff
eventLog 84 92 +8

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mikecote

@mikecote mikecote marked this pull request as ready for review November 10, 2022 12:34
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

Copy link
Copy Markdown
Contributor

@ersin-erdal ersin-erdal left a comment

Choose a reason for hiding this comment

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

Added a comment about changing duration, sequence and severity type mapping.

sequence: ecsNumber(),
severity: ecsNumber(),
sequence: ecsStringOrNumber(),
severity: ecsStringOrNumber(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you change these manually? I don't see any change on mappings.js

Copy link
Copy Markdown
Contributor Author

@mikecote mikecote Nov 10, 2022

Choose a reason for hiding this comment

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

With the change, the data type in Elasticsearch will remain the same long type (hence no change there), but I had to change the generation script and output files to indicate the accepted and returned value may be a string or a number (previously just number).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks :)

Copy link
Copy Markdown
Contributor

@ersin-erdal ersin-erdal left a comment

Choose a reason for hiding this comment

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

LGTM,
Tested locally and was able to see the duration field as string.

@mikecote mikecote merged commit a1730fd into elastic:7.17 Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Feature:EventLog release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.17.8

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ResponseOps] backport to 7.17 fix for: "event.duration" must be a safe number

5 participants