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

[infra] hosts table metrics #173708

Merged

Conversation

klacabane
Copy link
Contributor

@klacabane klacabane commented Dec 20, 2023

Summary

Closes #172475

  • record performance event of the host table data loading
  • infra synthtrace now installs system package and can produce all datasets queried by host view
  • lens viz (internal)

Testing

  • run journey with PERFORMANCE_ENABLE_TELEMETRY=1 node scripts/run_performance.js -v --journey-path x-pack/performance/journeys/infra_hosts_view.ts
  • check data is ingested in lens

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@klacabane
Copy link
Contributor Author

/ci

@klacabane
Copy link
Contributor Author

/ci

@klacabane klacabane marked this pull request as ready for review December 27, 2023 11:48
@klacabane klacabane requested review from a team as code owners December 27, 2023 11:48
@botelastic botelastic bot added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Dec 27, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@klacabane klacabane added Feature:Metrics UI Metrics UI feature release_note:skip Skip the PR/issue when compiling release notes labels Dec 27, 2023
@dmlemeshko
Copy link
Member

Hi @klacabane

Great work, good to see the new performance journey 🔥 Code LGTM and thanks for adding monitoring chart, I added it to the dashboard and will setup alerting to make sure we got notified whenever journey metrics are changing.

Hope you don't mind a small addition I pushed in 066ae0b
I'm not sure if you have future plans to extend the journey with more performance metrics collected (e.g. metric charts render time), but since we also collect APM data I thought it is good to wait for both table and charts are rendered, not only the first row to be displayed. We can later check if journey run time is getting faster/slower over time.

In the past with the help of perf journey we spotted that suggestion charts in Lens Editor have inconsistent rendering time (and customers were reporting it too), so I think it might it useful to track it.

@klacabane
Copy link
Contributor Author

Hi @dmlemeshko, thanks for the change makes sense to already have the journey baseline include charts. Adding charts load time is planned, do you know if there are any examples of that ?

@dmlemeshko
Copy link
Member

Adding charts load time is planned, do you know if there are any examples of that ?

You can use Lens Editor EBT instrumentation #163089 as example.

I had a quick look and there are few ways to do it: report duration for each LensWrapper + Metric Chart Wrapper individually (I'm sure I put the startTime incorrectly, but just as an example) and end up with 5 separate events or somehow handle loading state completion for the whole KPIGrid (1 event, optionally with details for each lens metric).

Each team has its own preference regarding events structure and level of details, we only suggest split it into time-to-data & time-to-render to better understand the trends (e.g. slow trend is in data fetching, rendering time remains consistent)

@klacabane
Copy link
Contributor Author

@elastic/obs-ux-logs-team @elastic/obs-ux-infra_services-team still looking for review :)

@jennypavlova jennypavlova self-requested a review January 10, 2024 13:47
Copy link
Contributor

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 I tried it locally, thank you for adding that!

await kibanaPage.waitForCharts({ count: 5, timeout: 60000 });
});

export function generateHostsData({
Copy link
Contributor

Choose a reason for hiding this comment

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

Really nice 🤩 I am happy to see that we can use synthrace for host test data generation 💯

x-pack/performance/journeys/infra_hosts_table.ts Outdated Show resolved Hide resolved
telemetry.reportPerformanceMetricEvent(
'infra_hosts_table_load',
duration,
{ key1: 'data_load', value1: duration },
Copy link
Contributor

Choose a reason for hiding this comment

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

After I checked the telemetry cluster again I saw that the duration is available there (passed on line 77) Do we need the value1 in this case (or do we want it because we want to use the data_load key)? I see the same data in the Time Series chart selecting value1 and duration
image

Copy link
Contributor Author

@klacabane klacabane Jan 11, 2024

Choose a reason for hiding this comment

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

Right now we only record the data loading duration, I've set it as key1 if we want to extend the event with more metrics (ie render time) but atm using either duration or value1 is similar.

Looking at metrics again I can't find the actual duration of the journey, we get different events associated with the journey (getAllTag, kibana_loaded..) but I'm not sure if one of these actually represents the journey execution time @dmlemeshko do you know how we can get the journey duration ?

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1342 1346 +4

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
@kbn/apm-synthtrace 34 44 +10

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.3MB 1.3MB +483.0B

Page load bundle

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

id before after diff
infra 99.7KB 99.8KB +169.0B
Unknown metric groups

API count

id before after diff
@kbn/apm-synthtrace 34 44 +10

History

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

@klacabane klacabane merged commit a1cd44c into elastic:main Jan 15, 2024
20 checks passed
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 15, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
## Summary

Closes elastic#172475

- record performance event of the host table data loading
- infra synthtrace now installs `system` package and can produce all
datasets queried by host view
- [lens
viz](https://telemetry-v2-staging.elastic.dev/s/kibana-performance/app/visualize#/edit/f88d3180-9f10-11ee-b4ef-7533a045abfc?_g=h@97e8101&_a=h@a8f21d5)
(internal)

### Testing
- run journey with `PERFORMANCE_ENABLE_TELEMETRY=1 node
scripts/run_performance.js -v --journey-path
x-pack/performance/journeys/infra_hosts_view.ts`
- check data is ingested in lens

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Dzmitry Lemechko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:skip This commit does not require backporting Feature:Metrics UI Metrics UI feature release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[asset_manager] Benchmark usage in Hosts view