[Logs / Metrics UI] Link handling / stop page reloads

[Kerry350]

Summary

This PR fixes #58007. No internal links (metrics <-> metrics, logs <-> logs) will cause a page reload now. Some stray broken links (from the NP migration) have also been fixed. A longstanding bug with the getUptimeLink function has also been fixed (serialized arrays (e.g. host.ip) weren't being handled correctly).

Going into more detail: A useLinkProps hook has been introduced. My hope is that this can become the primary (hopefully only) way we handle links. This hook takes care of using either interal routing via the react-router history instance, or external routing. It will return a href attribute, plus onClick where relevant. This return value can be spread (...) wherever it's needed (generally with EUI components). It is app and basePath aware and will handle all of those niggles.

To acquire the correct href and onClick atrributes we just need to supply a LinkDescriptor. We had a lot of locations handling links, and in a lot of cases they were all doing something slightly different. By consolidating things we should be able to have a lot more trust in our linking (as they're hanled in the same way), and changes will be easier to make when necessary.

The only thing the hook doesn't handle is complex data types and complex encoding for search, e.g. using Rison encoding. This is because some apps use it, and some don't, so I've left this as something for the consumer to handle. It could very well be an option to the hook though, with a default. Ultimately, the hook expects search values to already be strings, or arrays of strings, and in a state that is ready to be handed off to urlUtils.encodeQuery.

Example of an internal link:

// Linking from within the metrics app, to the metrics app
useLinkProps({
  app: 'metrics',
  pathname: '/inventory'
});

This would return a space / basepath aware href, plus an onClick handler that will deal with react-router and pushing to the history instance.

Example of an external link:

// Logs making an ML link (here some Rison encoding is used)

const _a = encode({
  mlTimeSeriesExplorer: {
    entities: { 'event.dataset': partition },
  },
});

useLinkProps({
  app: 'ml',
  hash: '/timeseriesexplorer',
  search: { _a },
});

These changes affect links in:

• Main navigation (inventory, settings etc)
• Node context menu (the menu displayed with a node on the metrics snapshot page)
• Log item actions menu in the log item flyout
• Chart context menu on the metrics explorer page
• Log rate / log categorisation Analyze in ML buttons
• No indices / No data prompt buttons (e.g. "View source configuration")

[Kerry350]

@elasticmachine merge upstream

[elasticmachine]

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

[Kerry350]

Note: #58943 will affect this PR ever so slightly (the one line that handles encoding, and potentially minor test changes). I'm going to try and preemptively make those changes on this branch, without that needing merged first.

[weltenwort]

I really like the useLinkProps hook. It makes all the link generation a lot more readable and I expect it will prevent quite a number of mistakes in the future - great work 👏

One interesting behavior I noticed was that switching between the logs and metrics apps via the sidebar causes no reload, but clicking the "Host logs" entry in the waffle map node context menu does. 🤔 Any idea why that might be?

[Kerry350]

@weltenwort Thanks for the review.

One interesting behavior I noticed was that switching between the logs and metrics apps via the sidebar causes no reload, but clicking the "Host logs" entry in the waffle map node context menu does. 🤔 Any idea why that might be?

The links in the sidebar receive special treatment in new platform allowing the nice no-refresh behaviour when switching between apps. However, with metrics and logs now technically being two separate apps, with independent App IDs, mount functions, and routers, the link rendered in the node context menu for logs is just a plain old <a /> and doesn't receive the same treatment.

If it's okay with you I'd like to look into fixing this when dealing with #58112?

Will start addressing the other feedback now 👍

[weltenwort]

If it's okay with you I'd like to look into fixing this when dealing with #58112?

Certainly, I just wasn't aware of that separate issue 👍

[Kerry350]

@weltenwort I've responded to all of the feedback now, this should be ready for another look. The app prop is now enforced, and the mechanism for differentiating between external links vs internal links has changed to treating links as external, and then enhancing as internal if they're the same (thanks for the suggestion, this worked well).

[Kerry350]

(For #58943 I'm not going to try and preemptively make those changes here due to package / webpack alias changes etc. Instead, as there's only really a one line change (and whatever merge conflicts), I'll help with making them over there once this is merged).

[weltenwort]

Thank you for including the explanatory comments. Together with the external/internal comparison it's quite straightforward to understand now. 👍 Nicely done 👏

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 1342cdd

History

• 💔 Build #31747 failed c942eac
• 💔 Build #31123 failed 4df03bf
• 💔 Build #31099 failed a46990c
• 💚 Build #30346 succeeded 8ac69e7
• 💔 Build #30111 failed 1098833

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