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

[ML] Transforms: Add performance journey for transform wizard source index loading. #160837

Merged

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Jun 29, 2023

Summary

Part of #160712.

Add a performance journey for transform wizard source index loading.

node scripts/run_performance.js --journey-path x-pack/performance/journeys/many_fields_transform.ts

Journey showing up in APM:

image

image

Journey showing up in Performance Metrics:

image

Checklist

@walterra walterra added :ml release_note:skip Skip the PR/issue when compiling release notes Feature:Transforms ML transforms v8.10.0 labels Jun 29, 2023
@walterra walterra self-assigned this Jun 29, 2023
@walterra walterra requested review from a team as code owners June 29, 2023 07:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Great job adding in our first performance journey! LGTM

x-pack/performance/journeys/many_fields_transform.ts Outdated Show resolved Hide resolved
Comment on lines +243 to +244
const span = this.currentTransaction.startSpan(name, type ?? null);

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the fix. I need to run a few pipelines to make sure we are able to extract APM transactions for scalability test simulation

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
transform 359 363 +4

Async chunks

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

id before after diff
transform 404.2KB 404.5KB +325.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 413 417 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 492 496 +4
total +6

History

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

cc @walterra

const loadIndexDataDuration = window.performance.now() - loadIndexDataStartTime.current;

// Set this to undefined so reporting the metric gets triggered only once.
loadIndexDataStartTime.current = undefined;
Copy link
Member

Choose a reason for hiding this comment

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

nice one! Having a single value for each metric per run seems the best way to go at the moment.

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

Dima commented he is going to validate the apm transaction thing.

The rest lgtm on the operations side

Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

LGTM. Great work!

@walterra walterra merged commit a8e07e8 into elastic:main Jun 30, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 30, 2023
@walterra walterra deleted the 157980-ml-transform-performance-journey branch June 30, 2023 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Transforms ML transforms :ml release_note:skip Skip the PR/issue when compiling release notes v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants