[discuss] Provide a single context service for different handlers

[joshdover]

With the current implementation of Handler Contexts, each Core service or plugin that wants to expose context must have its own IContextContainer which requires that each context provider register with each context container in the system.

In effect, this means N context providers must register with M context containers. This can easily lead to situations where the services available in one context are not available in another.

Many of these usages of context overlap. For instance, on the frontend, it's likely that all contexts would want to make available the same services. Rather than forcing all context providers to register with all services and plugins that want to expose context to handlers, we could have a single set of context providers for the entire frontend.

Examples

Before

class MyPlugin {
  constructor(private readonly initContext) {}

  public setup(core, plugins) {
    core.application.registerMountContext(
      this.initContext.opaqueId,
      'search',
      provider
    );

    core.tasks.registerTaskContext(
      this.initContext.opaqueId,
      'search',
      provider
    );

    plugins.management.registerMountContext(
      this.initContext.opaqueId,
      'search',
      provider
    );
  }
}

After

class MyPlugin {
  constructor(private readonly initContext) {}

  public setup(core, plugins) {
    // The same set of context providers could be used across services & plugins
    core.context.register(
      this.initContext.opaqueId,
      'search',
      provider
    );
  }
}

Considerations

• There may be different types of context providers depending on the situation. For instance, on the server, we may want to have a user-scoped context (for use in things like HTTP route handlers) and a generic context (for use in things like background tasks). We may need to provide two different contexts to support this. For example we could have a core.context.registerScopedProvider and a core.context.registerProvider for these two different situations.

[elasticmachine]

Pinging @elastic/kibana-platform

[epixa]

It'd be nice to see the context pattern used more in practice before expanding it's scope and usage like this. I suspect something like this is necessary, but it's going to add layers of complexity to an already-complex concept that we've mostly just reasoned about on paper.

If you do go down this route, we should consider a mechanism that allows plugins to compose contexts together rather than the interface of the context service becoming a generic registry. This would allow a plugin to say stuff like "I'd like to create a new context for my extension point that has this extra feature composed onto the features of the HTTP request context", which is different than hardcoding the notion of "scoped" into the API of the context service itself.

[joshdover]

On the frontend, we already have quite a few different places that either have context or should have context in the future:

• App mounting
• Chrome help extensions rendering
• Chrome nav control extensions rendering
• Toast message rendering
• Banner rendering
• Modal rendering
• Flyout rendering
• Embeddables
• Management section rendering

Right now, if one plugin wanted to add a new context property, they'd need to register with each of these.

In addition, if a 3rd party plugin wanted to provide context to its own handlers, they'd need to set up registering each context provider themselves.

All in all, I just think that we're trying to solve the service discovery problem in a way that may be more complicated than we need it to be.

[timroes]

Thanks for opening that issue. As discussed yesterday that's something we would highly appreciate.

It'd be nice to see the context pattern used more in practice before expanding it's scope and usage like this.

This might be a hen egg situation here, because we currently would rather avoid using context because of the above described issue. It would actually solve some of the setup/start lifecycle issues and allow us to register in setup, but at the cost of needing a registry for each of the places and the mentioned N:M relation.

I would also not necessarily agree with the system getting "more complex" by that change, rather the opposite. In our discussion yesterday we also tried to figure out how many different contexts there might be, and couldn't come up with any other example then the 2 on the serverside (bound to user/unbound) and one on the client side (always bound).

[mshustov]

Right now, if one plugin wanted to add a new context property, they'd need to register with each of these.

OTOH, what if a plugin wants to provide API only to a specific extension point?

and one on the client side (always bound).

Does AppMountContext expose something bound to a user?

[timroes]

@restrry Do you have an example for that case? Because providing something into a context is anyway an API of that plugin, I currently don't see any reason why we the plugin providing that API would want to limit that for a specific consumer (context extension point), especially given that available context extension points will change over time, so meaning every provider must decide with every new extension point if it would want to provide it there or not.

Does AppMountContext expose something bound to a user?

Maybe not right now, this was more meant on a "theoretical" basis. I meant, that everything running in the browser HAS a user, so in contrast to the backend side, there isn't at the same point in time a difference between contexts that are user bound and ones that are not. Sorry I am having a hard time describing that exactly, maybe @joshdover can help out here with better words :D

[epixa]

Heads up: I haven't forgotten about this and do intend to respond, but I need more time to think it through. @joshdover what sort of timeframe are you looking to make a decision in?

[joshdover]

@epixa I think it'd be good for us to make this change early in the 7.6 development phase if we're going to do it. We're expecting that most plugins will be consuming NP services at this point.

[epixa]

Can someone provide me concrete examples of where they think shared context is helpful? I think the challenge I'm having in understanding the value here is that I don't have real world examples. @joshdover listed a bunch of context handlers, but I'm not sure why we'd want to share services between them. A couple example interfaces may help me, or at least some example APIs that we'd want to make available in a bunch of different places.

The quote if one plugin wanted to add a new context property, they'd need to register with each of these is what is standing out to me. To me, context isn't some shared set of things that we want to make readily available in places now and in the future. If it were that, we would have called it "shared services" or something. But we didn't, we called it "context" because it is a set of services specifically crafted for the context of a single API. So in that respect, it's entirely appropriate for a plugin to have to register with each of those services if it wanted to extend their context. That there are many calls to register a thing means there is boilerplate, but not a bug.

The reason I feel something like this might be necessary at some point (maybe now) is for the second problem: if I'm creating a new API that I want to make extensible, how do I leverage the contextual capabilities that are available elsewhere? So for a concrete example of that, if we stop passing Elasticsearch clients to plugin setup, how does a plugin expose a contextual API that includes an Elasticsearch client? e.g. How does the alerting plugin expose an elasticsearch client to alert handlers that is already configured in the context of the user that created the alert?

[joshdover]

I agree that this change would not fix a bug, but would be aimed at reducing boilerplate, duplicated logic, and provide a more consistent development experience.

As it is now, plugin developers are at the mercy of both service owners (core services or plugins that "own" handlers and it's associated context) and context owners (core services or plugins that provide context services) to wire up each N context service to M context owners.

The way I can see this playing out is that devs may frequently run into issues where context service A (eg. data) is available in handlers 1 and 2, but not in handler 3. (eg. available in AppMountContext but not in ManagementSectionMountContext). The consumer would then need to register a context provider itself with the management plugin, possibly duplicating logic or worse, implementing it incorrectly.

Having a single registry of "render context" may help in this situation, since I expect many handlers responsible for rendering UI have the similar semantics and it's likely consumers would expect the same services to be available in each.

However, we haven't had this issue pop up yet, so maybe we're getting a little ahead of ourselves here.

[stacey-gammon]

Can someone provide me concrete examples of where they think shared context is helpful? I

However, we haven't had this issue pop up yet, so maybe we're getting a little ahead of ourselves here.

I think I might be hitting a few concrete use cases where this would greatly simplify my experience.

Situation:

• LivingDocumentation plugin exposes a registerSection extension point, where other plugins can add sections (this will probably be very similar to what will happen to management though I'm sticking with just react since it's a sample plugin, not sure if management will run into situations I am hitting).
  • registerSection extension point takes in an object where one of the fn's is render and that gets passed the context.
  • uiActionsExplorer registers a section on this. It depends on livingDocumentation, uiActions and uiActionSamples plugin (another sample plugin).
  • uiActionSamples registers some sample actions and triggers, and also exposes via AppMountContext a ui component that is hooked up to the new actions and triggers.

So, implementing this right out of the box, I get it wrong. Same situation I hit when developing the search plugin. @joshdover had to walk me through how to set up and expose a custom context container and registerSearchContext fn. For the actions one, I expected that the render = context => {} fn, context would have uiActionSamples but it doesn't because that context is actually being passed from livingDocumentation which doesn't rely on uiActionSamples.

Josh told me the way to fix this is to have livingDocumentation expose a new registerLivingDocumentationContext, and then the uiActionExplorer will use this to hook up the uiActionSamples context. I'm going to try to get this to work... but it seems like it'd be so much easier if all the context was just available everywhere and I could nix these custom container context things, which are a bit difficult to wrap ones head around.

Just my 2 cents. Maybe once the search plugin gets merged, and maybe this living documentation plugin as well (which is a bit of a side endeavor right now, so we'll see what happens with that), we can have some more concrete examples of where this proposal would simplify a lot of plugin interaction.

Oh and I also suspect management will hit the same thing since management page is just like my living documentation plugin.

[stacey-gammon]

Alright, I'm going through using registerLivingDocumentationContext and can explain better why I think this won't work well.

uiActionSamples was originally sharing it's SendMessageForm via core.application.registerMountContext. It's like, hey, anyone want to embed this into their application, here ya go! I imagine we'd do something similar with the TopNavQueryBar component bound to a global time range.

So I start trying to set up the connections in uiActionExplorer, since this is the plugin that knows it wants to showcase and test components from uiActionSamples inside the livingDocumentation plugin. Something like:

setup(core, deps) {
    deps.livingDocumentation.registerDocumentationContext<'uiActionSamples'>(
      this.initializerContext.opaqueId,
      'uiActionSamples',
      () => deps.uiActionSamples
    );
}

But uiActionExplorer plugin can't do this because deps.uiActionSamples doesn't export anything from its setup method. So I think the only way to do this is either:

1. Expose SendMessageForm from uiActionSamples setup contract. I think this is bad though because it's a react component and that will slow plugin initialization?

or

2. Have uiActionSamples register it's own stuff inside livingDocumentationContext. But that requires knowledge that it shouldn't have. It doesn't have a dependency on uiActions, nor livingDocumentation. It doesn't know that it's being used in that context.

I think this is the main point @timroes was making. The consumers of this context are relying on the producers of this context to know how it will be consumed. I don't think this will play well when we introduce third party development into the mix.

Anyway, maybe there is still a work around I don't know about, but I will sync back up with ya'll on Monday and see if we can figure it out!

Related PR with this exploration is here: https://github.com/elastic/kibana/pull/47337

it's not in a compilable state though, I just pushed my interim changes of trying to use registerDocumentationSectionContext.

[streamich]

I would be curious to see an example which involves multiple plugins. Like a 3 plugin example: (1) management app plugin that allows to register management app sections; (2) canvas plugin that wants to render a “Canvas” section in management app and in that section render a preview of a Canvas element, which is rendered using Expressions service; (3) expressions plugin provides an expression executor/renderer, but that executor/renderer is available only in start lifecycle; whereas Canvas would register its section in management app already in setup life-cycle.

[epixa]

My concern with this proposal remains unchanged: I think this is contrary to the purpose of context. The per-API nature of context was intentional, and despite trade-offs I see it as a valuable thing.

That said, my concerns are mostly theoretical and philosophical, and I'm not actively involved in building or supporting the context service, so if there's a strong consensus between the platform team and the consumers of this service about how it should evolve, I accept that.

I only have one deal-breaking stipulation: it is imperative that plugins only get access to features from other plugins they explicitly depend on regardless of whether context is constructed per API or whether it is shared across APIs. It's not clear to me whether anyone's proposed otherwise, but it's important enough that I wanted to mention it just in case.

[timroes]

This topic came up in a discussion between @flash1293 and me yesterday, and after talking about the current state and this suggestion, Joe brought up another imho legit question I wanted to raise here:

Leave aside the "user bound context" on serverside, but just giving the client side and serverside (non user bound) context. Is there a specific reason why we at all separate context and plugin API still? Couldn't we just fill the context with all the plugin APIs contracts instead? I find that also worth discussing.

I only have one deal-breaking stipulation: it is imperative that plugins only get access to features from other plugins they explicitly depend on regardless of whether context is constructed per API or whether it is shared across APIs. It's not clear to me whether anyone's proposed otherwise, but it's important enough that I wanted to mention it just in case.

I find that an interesting topic. And I think the context should only contain "dependable" API... but my question is: depending by whom? The plugin that "offers" the context or the one that consumes it? So assuming we have a plugin visualizations which will offer an API to register visualizations on, and in the callback for the visualization we will give you a context, would that context contain only the (context) APIs from plugins that the visualizations plugin depend on, or my plugin depends on?

My personal feeling is that the latter would make more sense, since if it just contains APIs the visualizations plugin depends onto, the consumer of the context is kind of dependent onto internal implementation details of the visualizations plugin. Since the visualizations plugin could potentially always change their dependencies based on what IT needs (and not it consumers might need). So for me it would make more sense the context contains only dependencies the visualization registering plugin depends on.

[epixa]

Edit: This is responding to the second half of Tim's comment.

@timroes I don't have a comprehensive answer in the context (no pun intended) of a shared expression service, but I can respond in terms of the original goals of context and hopefully that is still relevant enough to be useful.

Without shared context

The visualizations plugin owns the context that it exposes to other plugins via its public plugin contracts. It creates a base interface for that context with the varying services it provides for that context and also any of the services it depends on that it thinks are appropriate for that particular context. So perhaps context looks something like this:

interface VisContext {
  visualizations: VisApi;
  elasticsearch: CoreElasticsearch;
}

The visualizations_enhanced plugin extends the context of that API through something like a visualizations.extendContext setup API, which it has access to because it declared visualizations a requiredDependency. Roughly speaking, the new context looks like:

interface EnhancedVisContext extends VisContext {
  enhancedVisualizations: EnhancedVisApi;
}

The foovis plugin depends on visualizations but not enhanced_visualizations, so when it accesses the API's context, it does not see any capabilities of visualizations_enhanced on the context. For foovis, the context is VisContext.

The barvis plugin depends on visualizations and enhanced_visualizations, so it sees of those capabilities on its context. For barvis, the context is EnhancedVisContext.

With shared context

With shared context, the visualizations plugin wouldn't manually construct things like CoreElasticsearch and instead would rely on them being available automatically from the shared context it references? So roughly speaking the vis context interface becomes:

interface VisContext extends SharedContext {
  visualizations: VisApi;
}

When only considering core services (which are always available to everyone), then the only real change here is that there's less work for the visualizations plugin to create the original VisContext. With plugins, SharedContext would need to be computed based on the plugin that accessed it.

So let's assume for a moment that enhanced_visualizations didn't extend the vis context directly but instead modified the shared context for some reason. The SharedContext that was computed for foovis would need to omit the enhancedVisualizations API, whereas the SharedContext that was computed for barvis would include it.

[joshdover]

After seeing this play out in practice in a few different areas, I think we're seeing context on the client-side being used very differently than on the server-side.

Server-side context

On the server-side, we're using context providers to:

1. Simplify consumption of an API. For instance, provide the most recent emitted value of an Observable rather than the Observable itself to a HTTP route.
2. Bind a service to the current request. This allows us to avoid having to pass the request object all over the place just to get user-scoped access to Elasticsearch.

This makes the context providers on the server a bit more complex (and useful) than just the raw APIs they're built on top of. It also means they're more specific to their usages. For instance, a context provider for HTTP routes would be different than a context provider for a background task.

Client-side context

On the client-side, things are a bit different:

1. We're really just injecting services into functions, unchanged from their raw form
2. Exposing "fixed" versions of a service don't quite make sense for many of the rendering contexts because it's quite likely that we'd want the UI to adapt to Observables. This is different than HTTP routes on the backend because rendering contexts are long-lived.
3. It's hard to imagine fundamentally different types of usage patterns like we might see on the server. Almost all client-side code is ultimately concerned with showing something to the user. Even the "search" context mentioned by Stacey above should probably adapt to Observables changing so that UI that depends on these search APIs adapt to an setting change.

Ultimately, many of the motivations in the original RFC don't apply to the use-cases we see on the client-side.

Since we're using context providers in such a simple way on the client, it does feel like a lot of unnecessary complexity just to get access to some core APIs.

Alternative: Service binder

This leads me to believe that maybe what we really need on the client is a simple way to bind Core and Plugin APIs to a function:

interface CoreSetup {
  bindServices<T extends (...args: any[] => any)>(
    callback: (core: CoreStart, plugins: object) => T
  ): T;
}

class Plugin {
  setup(core) {
    core.application.register({
      id: 'myApp',
      mount: core.bindServices(
        (core: CoreStart, plugins: MyPlugins) => ({ element }) => {
          // normal app mounting code here
        }
      ),
    });
  }
}

This bindServices method would still solve one of the core problems that context currently solves on the client: getting access to APIs available only during the start lifecycle for functions registered during setup.

Advantages of this approach:

1. Neither registries nor calling plugins would necessarily need to know about bindServices at all.
2. This eliminates the potential bug highlighted in the context service documentation where a plugin could use the context service incorrectly, resulting in handlers getting the wrong dependencies.
3. Can be used deeply inside a UI or service tree to get access to Core APIs without having to pass all of them down.

Basically this bindServices function would be a very simple dependency injector that would respect the calling plugin's dependencies and does not introduce any magic to get any dependency in the system.

Disadvantages:

1. We lose the ability to expose "fixed" services such as resolving Observables. However, as noted in the difference (3) listed above, this is not something we generally want to do on the client.
2. We cannot tailor APIs to specific contexts. However, I have yet to see a use-case for this on the client.
3. This may limit what we could remove from CoreStart as proposed by a pending RFC though that RFC is primarily focused on the server-side.

Questions:

1. Are there use-cases that couldn't be solved well by the service binder approach?
2. If we move forward with this approach, should it only be available on the client?

[mshustov]

I have several concerns regarding the Service Binder .

• We blur the lines between different lifecycle stages. Are both coreSetup&coreStart contracts available in the binder function body?
• When you see the mount code from your example, it's hard to reason about execution order. Is it executed at the start stage? Or "after start" stage? Because all plugins already started.
• Another additional concept to learn. On top of plugin lifecycles & context.

Can we get by with existing means? Say use late binding.

class Plugin {
  setup(core) {
    core.application.register({
      id: 'myApp',
      mount: this.onMount,
    });
  }
  start(core, deps){
    this.startContract = { core, deps };
  }
  onMount = ({element}) => {
    const startContract = this.startContract;
    if(!startContract) throw new Error('not started yet');
    // normal app mounting code here
  }
}

[flash1293]

@restrry what you suggest is what I'm currently doing in https://github.com/elastic/kibana/pull/47469. Essentially it's the same thing service binder would be doing but you have to implement it yourself in each app plugin. I see this more as a small convenience function than anything else. We could also hide this behind a specialized Plugin class - but of course classical inhertiance has its own problems. I definitely prefer the binder-approach.

class MyPlugin extends AppPlugin {
  // will be used automatically in the register call done in super.setup
  appId: string = 'myApp';
  setup(core, deps) {
    // this will call application.register and save the deps on a local property
    super.setup(core, deps);
  }
  start(core, deps) {
    // this will save the deps on a local property
    super.start(core, deps);
  }
  onMount(element, params, startCore, startDeps) {
  }
}

[eliperelman]

You could potentially combine the two approaches to re-expose the start contract to the bound setup handler as well.

class ExamplePlugin implements Plugin<ExampleSetup, ExampleStart> {
  setup(core: CoreSetup, plugins: SetupDeps) {
    core.application.register({
      id: 'myApp',
      mount: core.bindServices(
        (core: CoreStart, plugins: StartDeps, example: ExampleStart) => ({ element }) => {
          // normal app mounting code here
          example.startMethod();
        }
      ),
    });

    return { setupMethod() {} };
  }

  start(core: CoreStart, plugins: StartDeps) {
    return { startMethod() {} };
  }
}

This would also make the assumption that a registered application wouldn't mount until it's start has returned.

[flash1293]

@eliperelman That would have the side effect that startMethod is exposed to other plugins which is probably undesirable.

[joshdover]

• We blur the lines between different lifecycle stages. Are both coreSetup&coreStart contracts available in the binder function body?

My proposal is that only the start contracts would be passed in by this API, but it is true that the enclosing function could access the setup contracts via the closure. This is true today already with context as well.

• When you see the mount code from your example, it's hard to reason about execution order. Is it executed at the start stage? Or "after start" stage? Because all plugins already started.

I think we would need to ensure that the ApplicationService does not execute until after all plugins have executed their start functions. Maybe we need an Core-only running phase?

As it is now, this is already true because Core's RenderingService does not start executing until all plugins have finished start. We would probably need to make the function returned by core.bindContext throw an Exception if called before PluginsService#start has completed.

• Another additional concept to learn. On top of plugin lifecycles & context.

I am proposing that we replace context on the client-side with this concept. I believe it is easier to use and find than the manual onMount pattern in your example.

Other benefits of this approach over the manual onMount pattern:

• We can directly enforce that no handlers depending on bindServices are executed before all plugins have had a chance to run their start methods.
• It's easier for plugin developers to find, rather than wondering "how in the world do I do this?" and having to lookup documentation and examples, the bindServices method is easy to discover via the CoreSetup type.

[joshdover]

We would probably need to make the function returned by core.bindContext throw an Exception if called before PluginsService#start has completed.

Alternatively, we can make bindServices return a Promise that won't resolve until after PluginsService#start has completed.

[joshdover]

Today we decided to move forward with the bindServices proposal above for the client-side only. Renaming this issue and updating it's description to reflect the decision. Opened new issue: #49691