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

[Discuss] View Alert in App #56298

Closed
gmmorris opened this issue Jan 29, 2020 · 24 comments · Fixed by #58997
Closed

[Discuss] View Alert in App #56298

gmmorris opened this issue Jan 29, 2020 · 24 comments · Fixed by #58997
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0

Comments

@gmmorris
Copy link
Contributor

gmmorris commented Jan 29, 2020

Following a conversation with @alexfrancoeur I felt it was worth discussing the View in App feature of the Alert Details page.

Broadly this is more than just a link, as it raises question about how alerts are represented across apps, what information we need when linking to them etc.

Things to figure out:

  1. How do we link an Alert to an App?
  2. What information would such a link need to maintain the drill down experience.

#Initial Thoughts

How do we link an Alert to an App?

My current thought process is this:
We should let each solution to specify the permalink to an alert the . way they see fit when defining the AlertType.
To that end, we can add an options function on the AlertType which takes an Alert and returns a permalink that a user can follow to view that Alert within the solution.

Open Questions

  1. Should this exist at Alert level too somehow? If a Solution allows a user to create a Alert based on an AlertType that is build-in or provided by another Solution, should they be able to override the permalink?
  2. Do we have built-in AlertTypes that need a permalink?
  3. If there is no permalink returned for an Alert, should the button be rendered as disabled, or not rendered at all?

What information would such a link need to maintain the drill down experience?

When the user drills down into specific date range on the details page- should we pass that to the AlertType when asking for the PermaLink so that they can provide a different URL?
This feels better than just assuming the URL can handle a DateRange (at all, or using a specific format) and adding it as query params on the URL, as it allows each Solution to handle DateRanges differently.

My current thinking is that if the user has just landed on the details page we use the default permalink provided by the Solution, and once they touch the DateRange field, we ask the AlertType for a new permalink, handing it the DateRange as an argument along with the Alert.

@gmmorris gmmorris created this issue from a note in Make it Action (In Progress) Jan 29, 2020
@gmmorris gmmorris self-assigned this Jan 29, 2020
@gmmorris gmmorris added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0 labels Jan 29, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

This was referenced Jan 29, 2020
@pmuellr
Copy link
Member

pmuellr commented Jan 29, 2020

related: #55622

@peterschretlen
Copy link
Contributor

My understanding is that in new platform, you can navigate to an app and provide state ( Luke provides a good overview here )

core.application.navigateToApp('dashboard', { state });

In theory we should be able to use this to deep link into apps that use alerting. That state would likely be the one the app was in when the alert was created / flyout was invoked, so it could be provided when the alert is created. A quick look at the code though I don't see state used much when calling navigateToApp, so I'm not sure how much/well it is supported today.

We could save the app + state on the alert - with the caveat that migration becomes complicated (see #56166 ). Though the app state has to be persisted somewhere, so whether it lives on the alert or a permalink object the upgrade/migrate problem doesn't go away.

I'm hoping we can avoid storing URLs. It's doable but I'm concerned it will be fragile over the long term.

@pmuellr
Copy link
Member

pmuellr commented Jan 30, 2020

we can add an options function on the AlertType which takes an Alert and returns a permalink that a user can follow to view that Alert within the solution

Should this exist at Alert level too somehow?

Do we have built-in AlertTypes that need a permalink?

The index threshold built-in alertType is a good sounding board for this. Hopefully, this alertType will be able to be used by multiple apps. If so, then you can't really say that all index threshold alerts should go to a single app. It has to be at the alert-level, not at the alertType level.

It seems like we can make use of the consumer SO attribute for this:

"consumer": {
"type": "keyword"
},

Somehow we can allow solutions to register a consumer value with some kind of function that to provide the right state for the core.application.navigateToApp(app, {state}) call. We call that function to get the state from the app, then call core.application.navigateToApp() with it.

We don't actually have a ton of data available in the index threshold API - date, alertInstance (string, eg hostname), value that "exceeded" the threshold value set in the alert, the threshold value set in the alert, alert group. That would be for a single "triggered" alert event.

The function provided by the consumer to return the state would be passed all that, and the alertType (string) itself, and it can make a determination whether it can compute the state for that. If it can't, we don't show the app link.

There's still migration concerns, of course, but it's possible we could handle this more dynamically, by peeking at things like ECS versions, Kibana versions, also available in the event log (don't think we currently write the Kibana version; will open an issue for that).

It's also not immediately clear where we would store things like "the exceeded value" in the event log - hopefully there is a good home for it in ECS, but I worry about more complicated alerts that may have more than a single scalar value that is of interest when the alert is triggered.

@gmmorris
Copy link
Contributor Author

@peterschretlen ,

My understanding is that in new platform, you can navigate to an app and provide state ( Luke provides a good overview here )

core.application.navigateToApp('dashboard', { state });

In theory we should be able to use this to deep link into apps that use alerting. That state would likely be the one the app was in when the alert was created / flyout was invoked, so it could be provided when the alert is created. A quick look at the code though I don't see state used much when calling navigateToApp, so I'm not sure how much/well it is supported today.

We could save the app + state on the alert - with the caveat that migration becomes complicated (see #56166 ). Though the app state has to be persisted somewhere, so whether it lives on the alert or a permalink object the upgrade/migrate problem doesn't go away.

I wasn't aware navigateToApp could take a state, I'll look into it.
I was thinking that state would just be the Alert itself and the Date range specified on the Alerts Details page.
I like the idea using a function on the AlertType to specify the state, but what data would we want to pass to it? My instinct says it should just be the Alert & Date Range, but we could also give it access to an API to the EventLog so that on a per AlertType basis a solution can decide what data to fetch to help it produce the appropriate state.

Anything else we add is essentially us making early assumptions about what a solution might need, and that's probably more limiting in the long run, isn't it?

I'm hoping we can avoid storing URLs. It's doable but I'm concerned it will be fragile over the long term.

Oh, that was never the intention - the idea was you would ask the AlertType for the URL when loading AlertDetails and would rely on the app fetching the alert when it loads its own page, not that we would pass the state on the URL (just Alert ID). It wouldn't be stored anywhere.

@gmmorris
Copy link
Contributor Author

gmmorris commented Jan 31, 2020

@pmuellr

It seems like we can make use of the consumer SO attribute for this:

Isn't the consumer specified by the solution? seems fragile as an implementor might not specify the Application specific name as consumer (perhaps because they don't realise that we use it for that) and so navigating to the app would fail.
If we make this coupling, we should validate that the consumer specified on the Alert is always a valid application name.
Is that feasible?

Somehow we can allow solutions to register a consumer value with some kind of function that to provide the right state for the core.application.navigateToApp(app, {state}) call. We call that function to get the state from the app, then call core.application.navigateToApp() with it.

Yeah, I like this 👍

We don't actually have a ton of data available in the index threshold API - date, alertInstance (string, eg hostname), value that "exceeded" the threshold value set in the alert, the threshold value set in the alert, alert group. That would be for a single "triggered" alert event.

The function provided by the consumer to return the state would be passed all that, and the alertType (string) itself, and it can make a determination whether it can compute the state for that. If it can't, we don't show the app link.

There's still migration concerns, of course, but it's possible we could handle this more dynamically, by peeking at things like ECS versions, Kibana versions, also available in the event log (don't think we currently write the Kibana version; will open an issue for that).

It's also not immediately clear where we would store things like "the exceeded value" in the event log - hopefully there is a good home for it in ECS, but I worry about more complicated alerts that may have more than a single scalar value that is of interest when the alert is triggered.

If the function were handed the Alert ID, a Date Range and access to the Event Log API, would that suffice to allow you to fetch the state you need?

@gmmorris
Copy link
Contributor Author

gmmorris commented Jan 31, 2020

Isn't the consumer specified by the solution? seems fragile as an implementor might not specify the Application specific name as consumer (perhaps because they don't realise that we use it for that) and so navigating to the app would fail.
If we make this coupling, we should validate that the consumer specified on the Alert is always a valid application name.
Is that feasible?

I've given this some further thought @pmuellr , would love your thoughts.

What if we had an API such as an AlertConsumerRegistry and any solution that wishes to consume an Alert can register a handler which is matched up with the value that they would then use as consumer on the Alert and the AlertyType they wish to use.
This handler could either return an application name and state that we then use with core.application.navigateToApp(app, {state}). We could also, perhaps, have the handler actually do the navigation itself, meaning this becomes a pluggable behaviour (showing a flyout, for example), but that would mean it needs to live on the Client side rather than Server and we'd have to see how to do that.

Think of it as a matrix where you have a Consumer on one axis, AlertType in the other, and where they meet is the handler.

This would mean that if you specify a consumer on the alert but there is no handler in the AlertConsumerRegistry that knows how to handle it - we simply omit the link.
This means we don't need to ensure that the consumer matches with an app name, and it also means that the a single solution can actually define different handlers for different consumer names, in case they have a variety of Alerts they wish to handle differently (such as siem-in-siem-app that's displayed in the SIEM app and siem-in-discover which they want to load in Discover and hence they need to navigate to those differently).

Does that make sense?

@pmuellr
Copy link
Member

pmuellr commented Feb 1, 2020

Does that make sense?

yup! Seems like we're driving in a good direction.

I'm guessing we're not going to need to pass a ton of info to the consumer handler - date obviously, and instanceId, and yes, they should have access to the event log. The consumer will likely have lots of their own info available given a date and instance id.

gmmorris added a commit that referenced this issue Feb 3, 2020
This PR adds an Alerts Details page, linked from the Alerts List.

It includes:
Header containing details about the Alert.
Quick Enable / Mute buttons for the Alert
Disabled buttons to the _Edit Alert+ flyout (waiting on #51545), View in App (waiting on #56298) and Activity Log (waiting on #51548)
gmmorris added a commit to gmmorris/kibana that referenced this issue Feb 3, 2020
This PR adds an Alerts Details page, linked from the Alerts List.

It includes:
Header containing details about the Alert.
Quick Enable / Mute buttons for the Alert
Disabled buttons to the _Edit Alert+ flyout (waiting on elastic#51545), View in App (waiting on elastic#56298) and Activity Log (waiting on elastic#51548)
gmmorris added a commit that referenced this issue Feb 3, 2020
This PR adds an Alerts Details page, linked from the Alerts List.

It includes:
Header containing details about the Alert.
Quick Enable / Mute buttons for the Alert
Disabled buttons to the _Edit Alert+ flyout (waiting on #51545), View in App (waiting on #56298) and Activity Log (waiting on #51548)
jfsiii pushed a commit to jfsiii/kibana that referenced this issue Feb 4, 2020
This PR adds an Alerts Details page, linked from the Alerts List.

It includes:
Header containing details about the Alert.
Quick Enable / Mute buttons for the Alert
Disabled buttons to the _Edit Alert+ flyout (waiting on elastic#51545), View in App (waiting on elastic#56298) and Activity Log (waiting on elastic#51548)
@mikecote
Copy link
Contributor

mikecote commented Feb 5, 2020

I was originally on the fence about having scenarios that the UI doesn't have a link to show. I kind of get it now.

I see this view in app link as a way to sort of give the user the ability to "go back into the app where I setup the alert" kind of thing as @peterschretlen mentioned. This would make sense to not show this "view in app" link for alerts that are created from the management app.

I would agree that it would have to be coupled with the consumer field because feature controls will also be depending on the value set there. With such registry, we should also validate the consumer field instead of letting any values being set.

@gmmorris

Think of it as a matrix where you have a Consumer on one axis, AlertType in the other, and where they meet is the handler.

I can see why this would make sense as the developer has to develop the flyout to create an alert of a specific type and this matrix registration becomes another step. I would like to explore options that reduce the amount of steps a developer has to go through to allow creating alerts from their app. I've put some thoughts below.

What if we allow the consumer to store metadata into the alert and enforce a single metadata schema per consumer. This schema could be close to what state / navigation params it allows. This would only force the consumer plugin to register itself and a single function that provides state for navigation (instead of one per alert type).

Another option that I can see working is what @peterschretlen recommended and we just store the state right into the alert with some way to handle migrations.

Side note
The saved objects management UI has a similar feature where you can "view in app" any saved object. It gets the URL via getInAppUrl function (ex: https://github.com/elastic/kibana/blob/master/src/legacy/core_plugins/kibana/index.js#L143-L148). The function is server side but tacks on the result of the function on each object when calling the management find API (here: https://github.com/elastic/kibana/blob/master/src/legacy/core_plugins/kibana/server/routes/api/management/saved_objects/find.js#L96).

majagrubic pushed a commit to majagrubic/kibana that referenced this issue Feb 10, 2020
This PR adds an Alerts Details page, linked from the Alerts List.

It includes:
Header containing details about the Alert.
Quick Enable / Mute buttons for the Alert
Disabled buttons to the _Edit Alert+ flyout (waiting on elastic#51545), View in App (waiting on elastic#56298) and Activity Log (waiting on elastic#51548)
@gmmorris
Copy link
Contributor Author

Following further discussion on the Alerting Design meeting, I'm sketching out bellow a suggestion on how we might want to approach implementation of this feature.
We'd love some feedback from Stack Monitoring & SIEM, as you've already began working in our framework.

First of all, to clarify the goal

We wish to add a View in App link on the Alert Details page which will allow the user to view the specific Alert within the Solution that originally created it.

Within the Alert Details page a use can view details about the Alert, but they will also be able to view all of the Alert Instances that were active within a certain Date Range, filtered by free text search or by specific Instance ID.

So, essentially, we would need to communicate to the Solution, identified by their consumer ID, which Alert needs to be view and what filtering has been applied (Date range, Specific list of Instances as filtered by the page OR the filtering needed to recreate that list of specific Instances).

Suggestion for Implementation

In order to remain flexible in how a specific Solution chooses to handle viewing an Alert we want to stay away from anything that assumes things about where to go, so we want to stick to using the new Kibana Platform navigateToApp api which is used in this manner:

core.application.navigateToApp(consumer, { state });

In which we tell the platform which Solution to Navigate to and a State to pass in which is the parameter that the solution can use to evaluate where to navigate to.

This means that all we already have the ID of the solution, as this is provided to us by the solution when it creates an Alert (this is the consumer field), all we need to do is figure out what the state would be - and this would probably be very specific to each individual solution.

To facilitate this we're proposing a mechanism where by a Solution can register a handler, much like solutions can register their own Alert Types as setup stage. This handler would be passed to the Alerting framework such that it specifies what Solution it represents (the consumer), whether it can support any Alert Type or whether it actually only knows how to display specific Alert Types (you can specify multiple handlers for different Alert Types if different handling is required) and a function which will be called by the framework whenever it wishes to navigate into the Solution whose sole job will be to return the State that it needs in order to navigate correctly.

Let's look at an example

Let suppose the monitoring solution wishes to provide a Solution specific view of the Index Threshold alerts created from within the monitoring app.

It could register a handler using an API like this (this is just an example, the API might be a little different):

plugins.alerting.setup.registerNavigation(
  'monitoring',
  'index-threshold',
  (alert: Alert, alertType: AlertType, alertInstances?: AlertInstances[], context?: { /* TBD */ }) => {
    //...
  }
);

This would tell Alerting that if the user is viewing the Alert Details page and the Alert is has an AlertType of IndexThreshold, and was created with the consumer monitoring then the View in App link is available as the solution should know how to handle a link into the solution to view this specific Alert with these specific Instances and context (filtering and date range).

The way we'll evaluate whether we actually want to enable the link is that when the Alert Details page loads we'll call the handler function, handing it the following arguments:

  1. The Alert
  2. The Alert Type
  3. The list of AlertInstances (probably as Event Log events) AND / OR the Context (meaning the filtering used to search for events in the Event Log which produced that list which can be fetch fresh by calling the Event Log API)

The handler would then evaluate, based on that information whether it actually knows how to display this Alert within it's UI, and if so, it returns an object of any shape that we would pass as the state to the navigateToApp api.

This allows us to be very flexible in how a Solution chooses to handle viewing the alert within it, as long, as it supports navigation via navigateToApp.

notes on 7.7 vs 7.8+

It doesn't look like we'll have the Event Log API ready for version 7.7, so we are current showing the current active AlertInstances rather than a whole history with date range filtering.
This means that the API in 7.7 will inevitably have to return a list of instances (rather than a filtering context), which means the API might have to change a bit between 7.7 and 7.8, but we'll try and keep that to a minimum.


Would love to hear thoughts and concerns, but especially, we'd love to hear from @elastic/siem and @elastic/stack-monitoring on whether you feel this approach would allow you the flexibility to handle this kind of feature well, or whether there's something we're missing.

@chrisronline
Copy link
Contributor

At a high level, this sounds good for our team. It will work for the alerts we are building right now, even though they wouldn't correspond to a unique alert view page.

The one thing to keep in mind is our navigation relies on global state (in particular, that is where we store which cluster is currently being viewed) and our alerts will span across all monitored clusters, so we'll need to ensure that the View in App link is able to handle this sort of navigation. I'm also saying this without a full understanding of how NP affects navigation between plugins.

@gmmorris
Copy link
Contributor Author

gmmorris commented Feb 13, 2020

Hmm interesting, thanks @chrisronline .

At a high level, this sounds good for our team. It will work for the alerts we are building right now, even though they wouldn't correspond to a unique alert view page.

The whole point is to allow you to decide at solution level how you want to handle this link- whether its Alert specific, or whether you just use the context (date range and/or filtering) to navigate the user to a place where the link makes sense, is totally up to you.

The one thing to keep in mind is our navigation relies on global state (in particular, that is where we store which cluster is currently being viewed) and our alerts will span across all monitored clusters, so we'll need to ensure that the View in App link is able to handle this sort of navigation. I'm also saying this without a full understanding of how NP affects navigation between plugins.

Am I right in thinking by global state you mean state in the Kibana instance?
If so, as the handler is called within the Kibana instance that the user is on, wouldn't that be able to access the global state and decide on the fly whether it can handle viewing this Alert or not?

@pmuellr
Copy link
Member

pmuellr commented Feb 13, 2020

I suspect there will likely be cases of wanting a "Show in App" without having to need to get the event log data - and we'll want to provide a way for any consumer of the alerting plugin to query event log data for an event, so I think we can let the consumer do that. Or maybe they can ask for it via an option of some kind. And send the time range they want. And ... hmmm.

Are there different scopes here?

I was thinking that we could have a View in App link for the alert itself, in case there's something special about viewing/editing the alert in general within the app. Without any instance data, or light on the instance data - perhaps just our current alert instance state thing.

And it also seems nice to be able to have a View in App link beside each alert instance (/ events, when they come), which the consumer could use to drill down further into their app (grouping and/or event date).

So maybe there are a couple of different locations we can place a View in App button, the consumer would tell us which they support (don't render the button if not supported), and we'd send that 'scope' in the call to get the state. Each scope probably has different data it sends as well - eg, more detail for View in App on an alert instance than for View in App for the alert in general.

@FrankHassanabad
Copy link
Contributor

This should work for us,

Our URL's work with the alert instance id's like so:

http://localhost:5601/app/siem#/detections/rules/id/a9c65c82-d3fb-4121-9ab2-5d8c3cdc8313

And if there's additional information such as date time that is even better and would be like so:

http://localhost:5601/app/siem#/detections/rules/id/a9c65c82-d3fb-4121-9ab2-5d8c3cdc8313?timerange=(global:(linkTo:!(timeline),timerange:(from:1581547734522,fromStr:now-24h,kind:relative,to:1581634134522,toStr:now)),timeline:(linkTo:!(global),timerange:(from:1581547734522,fromStr:now-24h,kind:relative,to:1581634134522,toStr:now)))

But if we don't have date time information it is not a biggie as the application will default to the user defined setting.

@gmmorris
Copy link
Contributor Author

gmmorris commented Feb 13, 2020

I suspect there will likely be cases of wanting a "Show in App" without having to need to get the event log data - and we'll want to provide a way for any consumer of the alerting plugin to query event log data for an event, so I think we can let the consumer do that. Or maybe they can ask for it via an option of some kind. And send the time range they want. And ... hmmm.

Yeah, could go either way, hence the capital OR there ;)

I was thinking that we could have a View in App link for the alert itself, in case there's something special about viewing/editing the alert in general within the app. Without any instance data, or light on the instance data - perhaps just our current alert instance state thing.

That's fine - it's up to the solution to implement.
If they choose to return a state that specifies that whenever the instance/context is empty (or even when it isn't) it's up to them 🤷‍♂

And it also seems nice to be able to have a View in App link beside each alert instance (/ events, when they come), which the consumer could use to drill down further into their app (grouping and/or event date).

So maybe there are a couple of different locations we can place a View in App button, the consumer would tell us which they support (don't render the button if not supported), and we'd send that 'scope' in the call to get the state. Each scope probably has different data it sends as well - eg, more detail for View in App on an alert instance than for View in App for the alert in general.

Yeah, we might want View in App at instance level as well, which I can see handled by this same approach, as we could make that part of the context or an explicit argument.
The API still needs to be ironed out, but broadly we just want to ensure this approach fits the overall need.

@peterschretlen
Copy link
Contributor

@justinkambic @andrewvc @formgeist see: #56298 (comment)
would be curious to get your thoughts on this proposal for linking alert details to pages in observability apps.

@formgeist
Copy link
Contributor

I would defer the technical implementation to @sqren or @jasonrhodes (@justinkambic is here already), but from a product design perspective, I think the solution sounds reasonable. This means that we'll be able to define the links specifically to the types and if we in the future have new views or states that will make it even easier for the user to investigate a threshold alert, we can register support for that.

As a related question but not within this scope, I'm wondering if you have given any thought to letting the user specify custom drilldowns/links for alert types? In that way, they have the option to register (create) their own actions/links for e.g. case management or support portal ticket creation etc.
Just to illustrate an example, I think there are many different links we can serve the user based on a threshold alert – e.g. going to the logs for the trace, inspecting the metrics – and that might not be our primary "View in app" action. Perhaps this is not the role of the alert detail page specifically, but I'm just wondering how it all might connect, and how we might give the user the best options to continue their investigation as they've received an alert.

And another idea; would it be feasible to create "View in x" links within the action itself in the alert creation - that way the user can immediately jump to e.g. APM and bypass the alert detail page, saving the user a click.

@sorenlouv
Copy link
Member

I think this approach can work really well for APM 👍

@andrewvc
Copy link
Contributor

The only concern I have with navigateToApp is what I wrote here: #25247 (comment)

I just want to make sure we use real hrefs and not abuse onclick etc.

@justinkambic
Copy link
Contributor

We haven't implemented any alert viewing capabilities to the Uptime plugin yet, so it will be useful to us if other teams who have done so, and have their own plans to support this type of linking continue to share concerns and implementation details as we design our solution's functionality.

@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 1, 2020

The only concern I have with navigateToApp is what I wrote here: #25247 (comment)

I just want to make sure we use real hrefs and not abuse onclick etc.

That's a very good point, but I don't think that's something we can (or in fact ,should) solve at this level, as it's really down to what navigateToApp supports.
We could do something like use navigateToApp if a handler returns an object, but allow you to also return a string - in which case we bake that string into a normal href.

This would mean that each solutio ncan decide for itself whether we should use a dynamic navigateToApp or a URL. And then, once navigateToApp provides a way for serialising into a URL, it'll be relatively straight forward to release a patch version that uses URLs in place of dynamic navigations.

Does that sound reasonable?

@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 2, 2020

I've began working on the API for this and realised that this could logically sit in either the client or the server.

The downside to exposing this api on the server side plugin is that it would meanhaving to make an api call to the server, evaluating the navigation state, when the Alert Details page loads and then again on every subsequent change to filtering/sorting where we might want the solution to be aware of these changes.
The advantage of this approach is that all interaction between the alerting plugin and the solution plugins remains in the same layer and so any solution specific state needed between both navigation and registration of Alert Types would be accessible.

On the flip side, if we expose this on the client (AKA public) alerting plugin, you wouldn't have to make so many calls to the server on each change, but it would mean solutions have to know how to evaluate navigation state on the client and if they need anything from the server then they'd have to do that work themselves.
The advantage to this approach is that the handler is supposed to return the state (a JS object) which will then be passed back to the solution as an argument via the navigateToApp api. If these reside on the same layer it means that this state can be passed as is, rather than having to be serialized on server and then deserialized on client. That SerDe process could introduce some pain points, such as dates going from a Date type to a number, or binary types being returned by the handler (a function, a Map, etc.) which would get lost in this serialization process.

I'm leaning towards exposing this on the server, using Typescript to limit the types that a handler can return on the state (serializer friendly types) and handling the client's need ourselves, but wanted to float this for feedback in case anyone has any thoughts or concerns.

@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 9, 2020

I've been working on a prototype of this API so we could test out how this API would be used.

This has uncovered two this:

  1. My prototype implementation (which can be seen here: [Alerting] Adds navigation by consumer and alert type to alerting #58997) is implemented on the server side as it was the fastest way to get it done. I am now leaning towards this API actually belonging on the client as the Serailisation/Deserialisation steps add complexity and, tbh, seem an unnecessary hinderance to adoption.

  2. it seems the navigatetoApp support for state is limited to New Platform. This doesn't just mean plugins using navigation would have to be on NP, it also means this wont work for anyone as the Alerting UI is booted by the Management app which is still on Legacy.

I think, with this in mind, we might need to postpone support for state based navigation (unless someone has an idea how we can support this without introducing an Alerting specific way of doing it) and instead rely on the plugins returning a string URL which the Alerting UI would link to directly. This also addresses @andrewvc 's concern, as these would all be regular links.

@pmuellr
Copy link
Member

pmuellr commented Mar 12, 2020

As mentioned in sync, I had been doing some thinking about whether this belonged on the server, or client. Initially it felt to me like it needed to be on the server, but the more I thought about it, realized it doesn't make too much sense there. Client-side seems like a better approach to me now.

@mikecote mikecote moved this from In Progress to Done (Ordered by most recent) in Make it Action Mar 19, 2020
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0
Projects
No open projects
Make it Action
  
Done (Ordered by most recent)
Development

Successfully merging a pull request may close this issue.