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

[Monitoring] Opening links in new tabs could not work in multi-cluster monitoring #52937

Closed
chrisronline opened this issue Dec 12, 2019 · 7 comments · Fixed by #63891
Closed
Labels
bug Fixes for quality problems that affect the customer experience Team:Monitoring Stack Monitoring team

Comments

@chrisronline
Copy link
Contributor

We rely on cluster_uuid in global state to persist the "active" cluster_uuid to ensure that when users click links to other parts of stack monitoring, we show them data from the "active" cluster.

This relies on a couple things:

  1. State persisted in the url (?g=)
  2. In-memory state (GlobalState)

When we construct links within the stack monitoring UI, we don't specify any global state (one example) so we don't need to worry about number 1 from above.

These links currently work because of number 2. Our routing is all driven through our browser-based router which enables seamless (no page reloading) routing so the in-memory global state is properly persisted and read when we load the next route.

However, this does not work if you open a link in a new tab. Number 2 goes away, as the in-memory state only persists for the specific tab. And since we do not actively use number 1, there is nothing ensuring the right cluster is used when the new tab link loads. We hope the server returns the clusters in the same order on the next page load, because we just pick up the first one.

@igoristic any insight into how this might be better in new platform world?

@chrisronline chrisronline added bug Fixes for quality problems that affect the customer experience Team:Monitoring Stack Monitoring team labels Dec 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@igoristic
Copy link
Contributor

igoristic commented Dec 16, 2019

@chrisronline AFAIK localStorage should still persist across tabs/windows if in the same browser (unless in incognito mode).

I think this can easily be fixed by first getting the url state via: globalState.fetch() and then syncing any changes if they are still relevant: globalState.$inheritedGlobalState = true and globalState.save()

@igoristic
Copy link
Contributor

I could sneak this fix into the NP branch and link this issue, since I'll already have basic shim for globalState anyways

@chrisronline
Copy link
Contributor Author

@igoristic Where you are seeing that globalState uses local storage? Looking through the code, it doesn't seem to persist it anywhere, other than memory.

@igoristic
Copy link
Contributor

@chrisronline Sorry I miss understood the question/problem

I guess we can make our resolvers in routes a bit smarter and check to see if the cluster uid in the url is already in the globalState; if it's not then maybe re-fetch it and sync it with globalState?

Have you tried re-syncing the globalState manually?

globalState.fetch();
globalState.$inheritedGlobalState = true;
globalState.save();

I haven't tested it in NP yet, but looks like it should do exactly that

@chrisronline
Copy link
Contributor Author

@igoristic

Have you tried re-syncing the globalState manually?

I'm not sure what you mean. Where would you add this code?

To reiterate the problem, this is problematic when you open a link in a new tab. The link itself doesn't contain any global state and global state isn't persisted across tabs (as it's just stored in memory, versus local/session storage)

@chrisronline
Copy link
Contributor Author

Bumping this up.

I think we need to solve this sooner than later, to support deep linking from alert emails.

I'm thinking we can introduce a quick solution where we use a custom url endpoint that will support setting globalState.cluster_uuid, something like monitoring#/_cluster/{clusterUuid}/{route}.

So monitoring#/_cluster/1abc2/elasticsearch/nodes/56yui89 will resolve to: monitoring#/elasticsearch/nodes/56yui89?_g=(cluster_uuid:1abc2).

We'll want to ensure this works for CCS links, since those add another global state param (ccs:{remotename}).

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:Monitoring Stack Monitoring team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants