-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Pluggable URL Service (Direct Access Link Service) #25247
Comments
Pinging @elastic/kibana-platform |
Pinging @elastic/kibana-app |
This looks interesting but I don't fully understand it and I'm wondering if it's at all related to the linking use-case I'm thinking of? I'll describe that here for comment. :) When APM wants to link to ML, or Infra to SIEM, etc etc, the app that wants to render a link (source app) needs to consider a few things:
In observability we've suggested one solution to this problem could be having each plugin define a set of "linking functions" that provide link hrefs for anything other apps may want to link to If apps use their own link functions to generate links within their own app, they would be guaranteed to stay accurate even if paths, query params, etc. change. Again, not sure if this problem is related to this issue here, but this is the problem we see most often right now. |
Thanks for the input Jason, I do think it's related and should be thought about when something like this is designed. It's not just links to saved objects but links to application pages, and they might not be backed by a saved object. So my original proposal which was written up a good bit ago should probably be rethought. But I agree it's an issue I keep see coming up, and will again when we want the user to be able to customize drilldown links. |
After thinking about this, it probably makes more sense to start with a client side only solution, which is easier and I think would still cover most of the use cases. Ignore the short url use cases for an initial phase. I've started thinking the action framework might work well. Something like:
Attach this action to the "APPLY_FILTER_TRIGGER" and drilldown links work. Been meaning to try to come up with a POC for this, but haven't gotten around to it yet. |
cc @streamich - I am thinking of taking this idea a bit further. I think it will be useful for alerts, background searches and drilldowns. As you dig into drilldowns, think about how you could use such a service, e.g. a dashboard drilldown uiAction that under the hood called something like: directAccessLinks.generateUrl('dashboardAppLinkGenerator', state: { query?, filters?, dashboardId?, version: string }); I think this will be more useful in the end than using UI actions, since we can do things like handle on the fly migrations, server side URL creation, deprecation warnings, and any object that wants to reference a url can do so by storing the Maybe if we do this we can have the background search team handle migrations by seeing this deprecation warning and updating the state. |
@stacey-gammon, some thoughts:
this can be wrapped in a plugin, which apps just need to require. And as directAccessLinkPlugin are already configured by the apps, this redirect plugin will have all needed information to do the migrations. |
Yea I think so
That's a really cool idea! Although with my current thoughts, class GoogleSearchGenerator implements DirectAccessLinkGenerator<typeof GOOGLE_SEARCH_URL_V2> {
readonly id = GOOGLE_SEARCH_URL_V2;
isDeprecated = () => false;
generateUrl = async (state: GoogleSearchUrlStateV2) =>
`www.google.com/search?q=${state.query}&${state.withinTimeRange !== 'none' ??
`as_qdr=${state.withinTimeRange ?? 'd'}`}`;
}
class LegacyGoogleSearchGenerator
implements LegacyAccessLinkGenerator<typeof GOOGLE_SEARCH_URL_V1, typeof GOOGLE_SEARCH_URL_V2> {
readonly id = GOOGLE_SEARCH_URL_V1;
generateUrl = async (state: GoogleSearchUrlStateV1) => `www.google.com/search?q=${state.query}`;
isDeprecated = () => true;
migrate(
stateV1: GoogleSearchUrlStateV1
): { generatorId: typeof GOOGLE_SEARCH_URL_V2; state: GoogleSearchUrlStateV2 } {
return { generatorId: GOOGLE_SEARCH_URL_V2, state: { ...stateV1, withinTimeRange: 'none' } };
}
} I'm not sure though if it makes sense to do it this way... I need to dig in a little more. I do think we'll need a way to type the old states though. And also what happens if you migrate a url multiple times, do you go from v1 to v2 to v3 or straight from v1 to v3? Probably we'll hit the same issues our migration system handles. Using separate generator ids I think might end up more manageable, but, I have yet to get a good enough feel for it. |
Most common pain point with regards to URLs from users:
** whichever solution we have we need to provide a migration plan to not break all existing urls |
I'll recap one key motivation for this kind of service which was not made very clear. This service is important because it handles migrations of URLs in saved objects - whether it be background searches, threshold alerts, or dashboards with drilldown configured actions. The plugin author who created the saved objects does not know where the URLs being stored in them originated from and should not be in charge of migrating them. This is especially true when in the context of external developers. For example, plugin developer Joe wrote an app that uses a dashboard embeddable which has panels that expose "create threshold alert" functionality (built by plugin developer Sue). The URL for viewing where this alert was generated from will point to Joe's app. What happens if Joe changes the app's URL structure? Joe doesn't know about threshold alerts, and Sue doesn't know about this app. Who will migrate the data saved in the saved object? One solution that was brought up in the meeting was to not migrate it in saved objects but only migrate on the fly, handled by the app. The problem is that we end up with a slew of old saved objects that contain old style URLs, that can't be migrated. Joe's app will have to support these old style URLs for a long time, if he doesn't want to break all his user's existing alert links. This is a situation we ran into with reporting and watches that referenced reporting URLs. Even though we technically could break these urls in a major version we chose not to because it would have been very disruptive to all the users with a ton of old watches. If we have a generic system for migrating URL references, we can go through a period of 'this is a legacy URL, it must be migrated, it will no longer be supported in version xyz'. Then our solution teams have a chance to update any saved objects with URL references in them by using the generic url migration system: migrateMySavedObjects(oldSavedObj) {
const generator = directAccessLinkPlugin.getGenerator(oldSavedObj.urlRef.generatorId);
if (generator.isLegacy()) {
const { newGenId, newState } = generator.migrate(oldSavedObj.urlRef.state);
oldSavedObj.urlRef = { generatorId: newGenId, state: newState };
}
return oldSavedObj; This way the user who created the saved object doesn't need to know anything about what URLs its storing, it'll get that information from the link generator along with the migration information. The app can also use this same functionality for migrating URL state on the fly for the period of time that the URL will be deprecated but still supported. We can use telemetry to determine how often old references are still around and when we can move from deprecated to not supported. |
Okay after spending more time thinking about this, here's a brain dump 😄 I think what has gotten me hung up is referring to everything as "URL State", since in the new platform the single source of truth for application state resides with the app itself as explained in #39855. If we can no longer rely on URLs always being there, then the service (and approach to migration) needs to be designed to only deal in state objects. Whether that state is synced to the URL is purely an implementation detail of a particular app. This means each app is responsible for:
The direct access link service is responsible for:
And solutions plugins using the service are responsible for:
With that in mind, I have a few questions:
I'm wondering if something along these lines would work: Phase 1 - client side onlyReplicate the existing URL-based behavior by creating a simple app that takes an For the sake of example, suppose we call this // mySolutionPlugin.ts
const state = encodeURIComponent(JSON.stringify({ foo: 'bar' }));
// url format is `${host}/app/goTo/${appId}${optionalPath}?_=${state}`
const url = `localhost:5601/app/goTo/dashboard/some/path?_=${state}`;
// goTo app then parses the appId / state and does a redirect:
await core.application.navigateToApp('dashboard', {
path: '/some/path',
state: { foo: 'bar' },
});
// which redirects you to:
`localhost:5601/app/dashboard/some/path`
// dashboardAppPlugin.ts
// At this point, the app is mounted and must pick up the state
// from `window` or react-router props:
const { state } = window.navigation.history;
// The app will optionally update URL via state sync utilities:
`localhost:5601/app/dashboard/some/path?_a=(foo:bar)` Each app would also expose a public interface of their state object: export interface DashboardPublicAppState {
version: number;
dashboardId: string;
filters?: Filter[];
query?: Query;
timerange?: TimeRange;
} Users who don't want to manipulate the URL and just want to do this programmatically from their own plugin can do this already with const state: DashboardPublicAppState = {...};
await core.application.navigateToApp('dashboard', { state }); As for migrations, apps could support these by statically exporting a simple reducer, perhaps with an // exported from direct access link service
interface AppStateMigrator<AppState> {
migrate: (legacyState: legacyState) => AppState;
isLegacy: (state: unknown) => boolean;
}
// dashboardAppPlugin.ts
export const stateMigrator: AppStateMigrator<DashboardAppState> = {
migrate: oldState => ({ version: 2, migrated: true }),
isLegacy: oldState => oldState.version < 2;
} Then solutions plugins could apply these to some state they have stored in a saved object: // mySolutionPlugin.ts
import { stateMigrator } from 'src/plugins/dashboard/server';
const { isLegacy, migrate } = stateMigrator;
const currentState = { version: 1, migrated: false };
const state = isLegacy(currentState) ? migrate(currentState) : currentState;
// Update saved objects with new `state` Later if we needed to, we could stick this in a registry, but as long as these are pure functions they can be statically imported on the client/server by other plugin devs, which solves the original goal of folks applying migrations from outside of a plugin. Phase 2 - serverGenerate short URL hashes which are stored with corresponding state in a permalink saved object. This would be a replacement for the existing shorturls implementation. Creating a new short url involves posting the state and other metadata to a REST API similar to the one in this issue's description: > POST api/short_url/generate // or whatever it is called
{
id?: string, // this is how you customize the short code
appId: string, // app you are redirecting to
path?: string,
state: Record<string, any>,
expires?: string,
}
< HTTP 200
{
id: '123abc',
url: 'localhost:5601/app/goTo/123abc',
...etc,
} Then navigating to one of these shortcodes would take you to our The main difference in this approach is that there isn't a generator ID, only a single app ID, and versioning lives in the state object (which is where we actually care about versions). So my question is - does something along these lines solve our use cases as well, and would it be simpler? |
Going to partially answer my own question here after having further discussions with @stacey-gammon -- this approach should work for solving the shorturls issue & passing state through a URL. But the more we have looked into it, the migration use case becomes increasingly complex, because the implications are widespread and affect several services outside of this new proposed access link service. With that in mind, #56166 has been opened to discuss these migration challenges in greater detail. I still think it'd be worth considering a service like this in the meantime, and simply continue to use on-the-fly migrations for now until a longer term strategy has been identified. |
An interesting idea was also raised in #4338 regarding providing direct access using URL parameters. This is somewhat similar to the "client side redirect app" idea I mention above in terms of ergonomics:
|
Want to just drop in here another use case for this service: global search/navigation (#56232). Saved objects will show up in this search and will need links for how to open them. This is an interesting use case, we may need to expose a registry, mapping saved object type to generator id. If we coupled them, then we couldn't migrate using the id itself. |
I added a basic implementation here: #57496 There is no client side redirect app, it's just a register of generators that have At least for the ML replacement, in it's current form, it would need the usual URL because they show it to the user: which has the migration issues but changing it now would be a breaking change. The client side redirect app I think is still a good idea for apps that want to take state that isn't in the URL. I think we could have both. Phase 1: interface DirectAccessLinkGenerator {
createUrl(state: S) => string;
}
interface DirectAccessLinkPluginSetup {
registerDirectAccessLinkGenerator: (id: string, generator: Generator<id>);
}
interface DirectAccessLinkPluginSetup {
getDirectAccessLinkGenerator: (id: string): Generator<id>;
} Phase 2 (if needed/when important): interface DirectAccessLinkGenerator {
createUrl(state: S) => string;
}
interface DirectAccessLinkPluginSetup {
// loading up the `http://.../redirectApp?id="123";state={...}` would grab the
// right state handler instance, call stateHandler.instantiateState(state), which might
// in turn use window.href.location = generator(id).createUrl(urlState);
registerRedirectHandler: (id, stateHandler: (state: S) => void);
registerDirectAccessLinkGenerator: (id: string, generator: Generator<id>);
}
interface DirectAccessLinkPluginSetup {
getDirectAccessLinkGenerator: (id: string): Generator<id>;
} The more I think about that though, I don't know if I see the benefit of this redirect app - the state is still in a url, just a different url. Either way, what do you think about moving forward with an initial, simple implementation like in #57496? |
That's right, we're thinking the initial version would allows users to navigate to apps and saved objects, and it would like something like this: The Core UI team's roadmap is still being shaped, but this feature is very near the top. |
I'm a little late to the party here, but I have a question to make sure I understand the proposals here. In these proposals are we giving any weight to making sure links are actually The way I read @lukeelmers ' comment it sounds like you'd click through to the app, and only after clicking through the app can decide to update its URL as optional next step. Is that a correct understanding? |
I think we'd want them to be href links so users can open in new tab, also they might be created server side and used externally, like in a "share to slack" action. |
I don't believe Global Search is quite blocked on this. We have an existing API that plugins can use to specify how to build a link to a SavedObject. It is currently in use by the Saved Objects UI in Management. This needs to be migrated to the Platform, but right now legacy plugins can specify the kibana/src/legacy/core_plugins/kibana/index.js Lines 139 to 143 in c6f5fdd
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
closed with #57496 |
Direct Access Link Service
Goal
Provide an pluggable API that generates direct access links for applications with state.
As a bonus, conditionally store it as a short url with an optional expiration attribute.
Why is this important?
We can still more forward with many features without this service, so why should we prioritize it?
Reason 1: Migrations for URLs in saved objects
Without a migration plan, we can end up with a lot of URLs stored in various saved objects that are broken and deprecated. We hit this issue with reporting - Watcher configurations had a lot of legacy style reporting URLs. Even though we could deprecate those URLs in a major, we chose not to for awhile because it would be so disruptive to our users - breaking all of there watches. Now we have multiple features in the midst of development that will also potentially require storing URL references. We'll end up with the same issues but even more widespread.
Consider the situation with threshold alerts, wanting a reference to the URL of wherever the embeddable resided. One workaround without this service is taking whatever the current URL is and storing that with the alert, but that will result in us having no idea of what URLs are being stored and referenced and what URL changes would cause a BWC - no tests around it.
Reason 2: Clear division of responsibilities
Rather than having hacked together URLs handled by multiple teams, it should be the responsibility of the application that is rendering in that URL.
Considerations
Server side functionality
We will likely want at least the URL generation services to be available server side for certain alerts (for example, you can imagine wanting a link to a dashboard that you sent to "run in background" (#53335) , or a threshold alert that you created based off of a visualization in a dashboard (#49908).
Since I think we should rely on our new URL state sync utilities, I think that means we should move some of that functionality into
common
folder and ensure we do not directly rely on client side specific functionality (e.g. defaulting a variable towindow.location.href
).Migrations
Some features may end up storing references to urls via this system, by storing a generatorId and state. Which team should be in charge of migrating these saved objects if the state stored has been deprecated? I believe we can solve the problem by having generators supply a
migrateState
function. Then the feature authors managing that specific saved object will be in charge of noticing deprecated url state stored, looping through the saved objects and runningmigrateState
in it to store the new state.API
Plugin
Storage
When the Generate Access Link service is used with
permalink: true
then a saved object of typepermalink
willbe created in the .kibana index with all the information needed to recreate the non-permalink version of the link.
Management
Current issues with our short url service:
Short urls never get deleted from es
We plan to store these new short url access links as saved objects which means we can easily
expose them in the saved object management interface for management capabilities, such as delete, with little additional effort.
With the capability of detecting parent-child object relationships, broken reference links could be
identified.
Additional feature requests
Human readable links
Solved via the
permalinkOptions.id
attribute.How this works with Embeddables
Many actions that are tied to embeddables would like to use URLs. For instance, if a dashboard wanted to store all it's queries in a background saved object, and alerts. In each case, there is a desire to store a URL that contains the specific embeddable, but the dashboard doesn't know where it came from. We may be able to pass in an optional object to Embeddables to supply this information to actions based on where they are embedded.
Migrations
implementors of the system can handle migrations "on the fly", using the version string, or register a saved object migration.
Registries
Server/Common:
Routes:
Embeddable threshold alert integration
Threshold alerts integration could use this service like so:
Chat ops integration
A chat ops integration could use this service like:
Make it slow integration
Background searches could use this integration as well, if we passed along a linkGenerator to the embeddable to get access to the url/state of the context it exists in, so when we click "view searches" we can send the user back to the original URL, but also ensure that it's set to reference the search ids in the collector cache
The text was updated successfully, but these errors were encountered: