Skip to content

[Log threshold] Use dataViewLazy during rule execution#215306

Merged
maryam-saeidi merged 17 commits intoelastic:mainfrom
maryam-saeidi:log-threshld-lazy-data-view
Apr 9, 2025
Merged

[Log threshold] Use dataViewLazy during rule execution#215306
maryam-saeidi merged 17 commits intoelastic:mainfrom
maryam-saeidi:log-threshld-lazy-data-view

Conversation

@maryam-saeidi
Copy link
Copy Markdown
Member

@maryam-saeidi maryam-saeidi commented Mar 20, 2025

Summary

In this PR, we use dataViewLazy, which avoids calling _field_caps API (this logic was introduced in this PR).

Questions

  1. Do we need to call _field_caps API in the log threshold rule executor? If yes, in which scenario?
    No, we don't need to call _field_caps API in rule execution.
  2. How to fix the type issues since DataViewLazy misses some fields that exist in the DataView type.
    We decided to use DataViewLazy everywhere on the server side but convert it to an actual DataView on the client side due to the need for the fields.
Screenshot
Create image
createDataViewLazy image

🧪 How to test

  • Enable APM locally
elastic.apm.active: true
elastic.apm.transactionSampleRate: 1.0
elastic.apm.environment: username
  • Create a log threshold rule and check its execution in traces filtered for your username as the environment. There should be one with your rule name:
    image

    The timing for _field_caps would be more if you replace the createDataViewLazy with the create function.

@maryam-saeidi maryam-saeidi added the release_note:skip Skip the PR/issue when compiling release notes label Mar 20, 2025
@maryam-saeidi maryam-saeidi self-assigned this Mar 20, 2025
@maryam-saeidi maryam-saeidi changed the title [Log threshold] Use dataViewLazy [Log threshold] Use dataViewLazy during rule execution Mar 20, 2025
@maryam-saeidi maryam-saeidi marked this pull request as ready for review April 5, 2025 07:48
@maryam-saeidi maryam-saeidi requested review from a team as code owners April 5, 2025 07:48
@maryam-saeidi maryam-saeidi added the backport:skip This PR does not require backporting label Apr 5, 2025
Copy link
Copy Markdown
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

changes to UA lgtm

Copy link
Copy Markdown
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Data Discovery changes LGTM 👍

Great to see that we are increasing data view lazy usage in rules!

@weltenwort weltenwort self-requested a review April 7, 2025 11:24
Copy link
Copy Markdown
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

LGTM, logs UI code still seem to work well. Nicely done 👏

I left just a few style/architecture questions below 😇

@fkanout fkanout self-requested a review April 8, 2025 11:32
Copy link
Copy Markdown
Contributor

@fkanout fkanout left a comment

Choose a reason for hiding this comment

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

Tested, and I checked the trace, and no field capabilities calls as expected. LGTM
Screenshot 2025-04-08 at 13 57 56

Also, I checked the fields they are all listed

Screen.Recording.2025-04-08.at.14.09.35.mov

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

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
logsShared 229 228 -1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1.2MB 1.2MB +72.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
logsShared 130.6KB 130.8KB +133.0B
Unknown metric groups

API count

id before after diff
logsShared 255 254 -1

History

cc @maryam-saeidi

Copy link
Copy Markdown
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Great performance improvement, thanks a lot!

@maryam-saeidi maryam-saeidi merged commit 1b10f35 into elastic:main Apr 9, 2025
10 checks passed
@maryam-saeidi maryam-saeidi deleted the log-threshld-lazy-data-view branch April 9, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants