[SecuritySolution] [Bug] Hide timeline for priv mon onboarding

[machadoum]

Summary

Fix the bug that caused the timeline to overlap with the onboarding components on the privileged user monitoring page.
Since the timeline is not required on an onboarding page, we decided to hide it.

For now, I decided to implement the simplest change to fix the bug, which is adding a URL param (hideTimeline) exclusively for hiding the timeline. I also considered updating the existing timeline URL parameter, but that seems too complicated for a bug fix, especially since it would require updating the timeline Redux store, which already has a show property used for other purposes. If this use case becomes widely used, we can refactor it to be part of the timeline core implementation.

Checklist

• Unit or functional tests were updated or added to match the most common scenarios
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

[elasticmachine]

Pinging @elastic/security-solution (Team: SecuritySolution)

[elasticmachine]

Pinging @elastic/security-entity-analytics (Team:Entity Analytics)

[PhilippeOberti]

Just my 2 cents but adding more logic to the already too complex useUrlState isn't a great idea... We're actually currently investigating an issue with this code (see this ticket), and recently some changes were made because things were not happening in the right order.
I'm just saying that adding even more logic to this is maybe risky?

Also I think we need to be careful when wanting to add url parameters. These will stay for a very long time, and when we want to remove them we still need to keep code for months or potentially years as we need to support previously customer saved or bookmarked urls.
To me it seems that adding a new url parameter just for one page isn't the right move. I'm not familiar with the privileged user monitoring page, but it does not look like it's a page that would be as commonly used as the rules page, the alerts page or discover for example?

Also, we already have a way to hide Timeline from a page, via the LinkItem. Couldn't this be leveraged here? To my knowledge, it is used on every page in Kibana.

[tiansivive]

code LGTM

[semd]

Hey @PhilippeOberti, I understand your concerns.

I already discussed this with Pablo before. Some context:

They have this page in the /entity_analytics_privileged_user_monitoring path (link config), for which they do want the Timeline bottom bar to be visible when the page is ready. However, they have a state when the engine is not ready, in which they render onboarding content, and in that scenario, it does not make sense to render the timeline bottom bar, it may be confusing for the user, so they want to hide it.

We already have a way to hide Timeline from a page, via the LinkItem. Couldn't this be leveraged here? To my knowledge, it is used on every page in Kibana.

They can not use the LinkItem config because that would always hide the Timeline for that path. They need a way to toggle the Timeline visibility programmatically.
I suggested using a query parameter, since we are already using the URL to determine the timeline visibility via pathname + link config. Adding an additional control via search looked like a low-friction solution.

I was not aware of the problems with the urlState, but I see this entry is not adding any action. Actually, we could accomplish the same thing without using useState, just using navigation, but urlState abstracts that work.

I think we need to be careful when wanting to add URL parameters. These will stay for a very long time, and when we want to remove them, we still need to keep code for months or potentially years as we need to support previously customer saved or bookmarked urls.

Agree, but this is not a search parameter or anything critical about the page behaviour, like we have on Alerts, Flyout.... It's just hiding something that does not make sense to show. Deprecating and ignoring this parameter would not cause a big issue.

I think this approach is simple and just works, it solves the problem. It's not fancy, but practical and reusable.

Another solution could be creating a new Redux action and a new state prop to force the timeline bar not to render. But that is more complex and would imply more changes.

wdyt?

[PhilippeOberti]

They can not use the LinkItem config because that would always hide the Timeline for that path. They need a way to toggle the Timeline visibility programmatically. I suggested using a query parameter, since we are already using the URL to determine the timeline visibility via pathname + link config. Adding an additional control via search looked like a low-friction solution.

Thanks for the explanation, I did not know that!

wdyt?

I don't like it, but I also do not have an alternative to suggest 😆

• I don't like the fact that all our current url parameters are used to store data and this new one isn't.
• I don't like the fact that now that it's there in the code, other people might be tempted to use it, especially if they're not aware of the current hideTimeline option on the LinkItem
• I don't like the fact that we're going to now check the url for every page change to see if we want to show Timeline or not, just for a single page in the entire Security Solution application

I'm not going to block this PR, but I'll let someone else in my team approve if that's ok with y'all 😄

[semd]

@PhilippeOberti
This is not very constructive 😅, but I get your point. Let me try to find an alternative approach

[PhilippeOberti]

@PhilippeOberti This is not very constructive 😅, but I get your point. Let me try to find an alternative approach

I know and I'm sorry. I cannot think of an alternative... I just wanted to raise my concerns that's it. If that's the best option we have, then let's merge it!
Otherwise, I'm happy to brainstorm together on this!

[logeekal]

I think one problem with hideTimeline search parameter is that it introduces a new approach to show/hide timeline in addition to LinkItem. It has a potential to be confusing in future (which of them takes priority and when over other).

Another solution could be creating a new Redux action and a new state prop to force the timeline bar not to render. But that is more complex and would imply more changes.

I think problem with this approach is that if a state is changed to hideTimeline: true, then user navigated to a different page, there will be a need to revert this state which i do not like. Today, we do something similar with timeline like this and it is already causing problems.

I can think of one alternative which does not touch timeline at all.

Can't we have 2 different routes:

• /entity_analytics_privileged_user_monitoring/initializing/ - when engine is not ready.
• /entity_analytics_privileged_user_monitoring - when engine is ready.

@semd , @machadoum , do you think this is viable?

[kqualters-elastic]

Hacky but simpler:

  useEffect(() => {
    const element = document.querySelector(
      '[data-test-subj="timeline-bottom-bar-container"]'
    ) as HTMLElement | null;
    const hideTimeline = ['onboarding', 'initializingEngine'].includes(state.type);
    if (element) {
      if (hideTimeline) {
        element.style.display = 'none';
      } else {
        element.style.display = 'block';
      }
    }
    return () => {
      if (element) {
        element.style.display = 'block';
      }
    };
  }, [state.type]);

in EntityAnalyticsPrivilegedUserMonitoringPage

[kqualters-elastic]

Comparing this branch with main, this linkUpdater causes some more re-renders of the SecuritySideNav and related hooks, and given how fragile the current logic is to render ordering (see: this bug: #225650 , and happy to talk about others if you want), we should imo use the css approach with the useEffect above to not introduce any other unintended side effects. Hooks based architecture where everything is tied into the component lifecycle is brittle enough as is, using observables from a nested useEffect makes everything more brittle, and since this is just being added to fix a css bug, should be fixed as close to that as possible.

[machadoum]

Comparing this branch with main, this linkUpdater causes some more re-renders of the SecuritySideNav and related hooks

This sounds quite surprising to me, because Sergi's changes shouldn't impact any rerender unless the links config is explicitly updated, which only happens on the Privileged User monitoring page.

I ran some tests and was unable to reproduce the increase in rerenders. I tested the render method of 2 components when the security solution is fully loaded, and we navigate from a landing page to any of the following pages:

---

1. x-pack/solutions/security/plugins/security_solution/public/common/components/navigation/security_side_nav/security_side_nav.tsx

• BEFORE CHANGE
  landing and dashboards 3
  host, user - 5
  cases 5
  alerts 6
  detection rules 8

• AFTER CHANGE
  landing and dashboards 3
  host, user - 5
  cases 5
  alerts 6
  detection rules 8

---

2. x-pack/solutions/security/packages/side-nav/src/solution_side_nav.tsx

• BEFORE CHANGE
  Once for every page

• AFTER CHANGE
  Once for every page

@kqualters-elastic, please let me know how you tested it. I want to investigate why it happens.

[kqualters-elastic]

@machadoum ya I would expect any navigation to a new route to be the same, what is different is when you are on the same route, and update the state.type that the effect depends upon via one of the reducer actions, doesn't matter how.

[kqualters-elastic]

@machadoum made this branch: https://github.com/kqualters-elastic/kibana/tree/use-effect-observable-fun

added 2 buttons to the top, but just have an onClick prop that's a useCallback. 1 is the logic in the effect that calls the appLinks observable, 2 is the css based approach.

This is a gif of the profiler when the updater ran, you can see that it was in fact caused by that, and then subsequently all sorts of potentially impactful things happen, history changes, state syncing hooks, etc.

[machadoum]

@kqualters-elastic I see. Indeed, the CSS is much quicker for hiding and showing the timeline.

However, it also has some drawbacks, such as being an additional source of truth for hiding or displaying the timeline, which could make debugging more difficult when an error occurs. And be more brittle, since it depends on internal timeline CSS selectors, and in the unlikely scenario of an unexpected error, it could hide the timeline for all pages.

I am okay with both solutions; each one has its pros and cons. I will leave the decision for the @elastic/security-threat-hunting-investigations team. Please let me know which one you prefer.

[lgestc]

Maybe we could utilize route state here? No extra url params, just in memory.

@machadoum https://v5.reactrouter.com/web/api/Link/to-object

[kqualters-elastic]

such as being an additional source of truth for hiding or displaying the timeline, which could make debugging more difficult when an error occurs.

@machadoum that applies to any solution where we are showing/hiding the timeline from a component instead of in the link definitions.

And be more brittle, since it depends on internal timeline CSS selectors, and in the unlikely scenario of an unexpected error, it could hide the timeline for all pages

it's a single static top level attribute that all the tests rely on that's always rendered when timeline is, and changing the styles of the timeline is not going to create any side effects, works fine with class based emotion, whereas the observable approach triggers a whole cascading chain of updates, in components that are known to be very codependent on rendering order. In my opinion it's an easy choice, but it's up to you really. Is there an issue or screenshots with the original css issue?

[machadoum]

I will keep this PR open until the team responsible for the timeline makes a call. Since I am not responsible for maintaining the timeline and won't be on call for SDHs, I don't feel comfortable making the decision.

[machadoum]

I'm replying here to ensure my point is clear. I don't mind implementing the CSS approach if that is the recommendation from the investigation team.

such as being an additional source of truth for hiding or displaying the timeline, which could make debugging more difficult when an error occurs.

@machadoum that applies to any solution where we are showing/hiding the timeline from a component instead of in the link definitions.

I disagree with the statement, because if we hide the timeline using the same approach, it is already being used on other pages (like the config), the hook useShowTimeline will work as expected, and every place that depends on it too. However, if we use an approach that makes useShowTimeline return an incorrect value, it could create bugs in areas that rely on the hook.

Only three places depend on useShowTimeline. But with time, more places could use the hook.

• Security template which hides the timelines
• The assistant,
• The side nav

[machadoum]

Hacky but simpler:

  useEffect(() => {
    const element = document.querySelector(
      '[data-test-subj="timeline-bottom-bar-container"]'
    ) as HTMLElement | null;
    const hideTimeline = ['onboarding', 'initializingEngine'].includes(state.type);
    if (element) {
      if (hideTimeline) {
        element.style.display = 'none';
      } else {
        element.style.display = 'block';
      }
    }
    return () => {
      if (element) {
        element.style.display = 'block';
      }
    };
  }, [state.type]);

in EntityAnalyticsPrivilegedUserMonitoringPage

I tested this approach, and it doesn't work consistently. I haven't delved much into it, but it is probably due to the React rendering order. If the timeline is rendered after we switch its style property, it will overwrite the changes.

Screen.Recording.2025-07-07.at.09.24.22.mov

Please let me know how to proceed.

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: fb3f5e9

Failed CI Steps

• FTR Configs #64
• FTR Configs #103

Test Failures

• [job] [logs] FTR Configs #64 / ESQL execution logic API @ess @serverless ES|QL rule type, alert suppression with exceptions should apply exceptions
• [job] [logs] FTR Configs #103 / home app sample data installing should install ecommerce sample data set

Metrics [docs]

Async chunks

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

id	before	after	diff
securitySolution	9.8MB	9.8MB	+638.0B

Page load bundle

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

id	before	after	diff
securitySolution	94.4KB	94.8KB	+402.0B

History

• 💚 Build #313861 succeeded 5066249
• 💚 Build #313491 succeeded 8b0ab72

cc @machadoum

[kibanamachine]

Starting backport for target branches: 9.1

https://github.com/elastic/kibana/actions/runs/16286635677

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	9.1	Backport failed because of merge conflicts You might need to backport the following PRs to 9.1: - [Investigations][DataViews] - Remove non-performant toSpec usage (#225726) - [Security Solution] Fix initial data view flash (#225675)

Manual backport

To create the backport manually run:

node scripts/backport --pr 225442

Questions ?

Please refer to the Backport tool documentation

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 225442 locally
cc: @machadoum

[machadoum]

💚 All backports created successfully

Status	Branch	Result
✅	9.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation