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

[Perfomance] Add is_initial_load meta #206645

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kpatticha
Copy link
Contributor

@kpatticha kpatticha commented Jan 14, 2025

closes https://github.com/elastic/observability-dev/issues/4185

Summary

This PR adds the is_initial_load parameter to the meta field to distinguish whether the onPageReady trigger occurs during the initial load or a page refresh.

You can find the sample document with the is_initial_load metadata in staging telemetry cluster

Link: [DEDUCTED]/s/apm/app/discover#/doc/0d6d7d31-1369-4a53-b36d-fbe97e4e5a0e/backing-ebt-kibana-browser-performance-metrics-000008?id=performance_metric-RMeyhJaNRWamPLvsAPdb4g-2025-01-14T14%3A40%3A50.597Z-kibana%3Aplugin_render_tim

Screen.Recording.2025-01-14.at.19.15.26.mov

How to test

  • Checkout the PR
  • make sure you run yarn kbn bootstrap
  • go to any page that has onPageReady function instrumented (ex services)

@kpatticha kpatticha requested a review from a team as a code owner January 14, 2025 17:26
@kpatticha kpatticha added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:current-major Backport to all previous minor branches backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:current-major Backport to all previous minor branches labels Jan 14, 2025
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Approving from the Core POV because a Set is better than a string[].

I added a comment because I'd love to understand what we're trying to track here.

}

export function measureInteraction() {
performance.mark(perfomanceMarkers.startPageChange);
const trackedRoutes: string[] = [];
const trackedRoutes = new Set<string>();
Copy link
Member

Choose a reason for hiding this comment

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

Q: Do we remove the tracked routes at any point?

Aside from a potential memory leak, what happens if the user goes elsewhere in Kibana and comes back?

Would it be enough if trackedRoutes (Set<string>) is actually currentPathname (string)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The trackedRoutes updates with each route change. If the user navigates to another page (e.g., the services page) and then returns, trackedRoutes will be empty.

The objective is to determine whether the current load is due to navigation (an initial load) or a page refresh. Happy to update it if you have a better approach.

Not sure I understand the following, can you explain please

Would it be enough if trackedRoutes (Set) is actually currentPathname (string)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Tracking currentPathname or plugin url would be better, see comment.

Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

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

Awesome! 🥇

Do we also have a way to distinguish whether the event was related to a page load via inside Kibana navigation or the initial load of Kibana?

@kpatticha
Copy link
Contributor Author

Awesome! 🥇

Do we also have a way to distinguish whether the event was related to a page load via inside Kibana navigation or the initial load of Kibana?

with the current implementation we don't have a way to distinguish this. There is another EBT metric, kibana_started, which we might be able to correlate with these two events using their timestamps. However, this would require additional analysis work.

@elasticmachine
Copy link
Contributor

💔 Build Failed

Failed CI Steps

History

Copy link
Contributor

@awahab07 awahab07 left a comment

Choose a reason for hiding this comment

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

is_initial_load changes with date change because selected time range is persisted via URL state. Also, URL could change even against UI actions which do not cause a data reload.

206645-initial-load-meta-url-issue.mov

See if extracting the plugin url or using pathname is better suited here instead of the full url.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants