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] Move to domain-based directory structure #71566

Closed
1 task
joshdover opened this issue Jul 13, 2020 · 37 comments
Closed
1 task

[discuss] Move to domain-based directory structure #71566

joshdover opened this issue Jul 13, 2020 · 37 comments
Labels
discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team

Comments

@joshdover
Copy link
Contributor

joshdover commented Jul 13, 2020

This issue is to discuss moving the Kibana repository to domain-based file directory structure.

Goals

  • Create a new repository structure that can grow with our codebase and organization
  • Make exploring the codebase simpler
  • Group related code in the same area of the codebase
  • Improve the discoverability of shared services and other foundational building blocks
  • Promote encapsulation between modules and reduced coupling across the codebase
    • Related: allow code sharing between related modules without sharing to the whole codebase
  • Pave the way for a new, modular toolchain based on distinct NPM packages
  • Support the Kibana Development Principle, "Organize by domain"

Non-goals

  • Match directory structure to team structure or responsibilities
  • Find a perfect taxonomy that fits every situation

Proposal

Currently, the bulk of Kibana's source code is organized in a flat structure inside the src/plugins and x-pack/plugins directories, where each directory is a plugin. Building on the capabilities added in #68811, I propose that we create a new, hierarchical directory structure that is domain-centric rather than layer-centric. If today's structure describes what layer of code (eg. a plugin) is contained in a directory, this new structure would describe what domain of responsibility (eg. alerting) is contained in a directory.

This proposal involves:

  • Eliminating the plugins directories
  • Creating a new, hierarchical structure, based on domains
  • Moving all code that is not shipped in our distributable out of the src directory

The domains:

  • 3+1 solutions are all grouped as their own domains (analyze, observability, security + elasticsearch)
  • application_services is a domain of services used to build other applications
  • core is a domain of services that define or significantly augment how Kibana works
  • alerting appears complex enough to be its own domain. It is both an application service and an application itself
  • management appears to be similar to alerting in that it is a service and an application, though this could probably be moved into application_services if desired

Proposed structure

  • dev (build tooling that does not ship in the distributable)
  • src (all code that ships in the distributable)
    • analyze
      • dashboard
      • discover
      • visualizations
        • charts
        • vis_type_*
        • visualize
    • cli
      • kibana
      • keystore
      • plugin
    • dev_tools
      • console
      • ui (the current dev_tools plugin)
    • home
      • newsfeed
      • ui (the current home plugin)
    • management
      • advanced_settings
      • saved_objects_management
      • ui (the current management plugin)
    • platform
      • core (as in the "core platform" -- always present)
      • application (as in the "application platform")
        • embeddables
        • inspector
        • ui_actions
        • kibana_react
        • kibana_utils
      • data (as in the "data platform")
        • data (should probably be broken up into separate plugins / domains?)
        • expressions
        • es_query
        • bfetch
      • telemetry (as in the "telemetry platform")
        • usage_collection
        • telemetry
        • telemetry_collection_manager
        • telemetry_management
    • test
      • utils
      • fixtures
      • es_archiver
  • x-pack
    • alerting
      • alerting
      • alerting_builtins
      • alerts
      • watcher
    • analyze
      • dashboard_enhanced
      • dashboard_mode
      • discover_enhanced
      • graph
      • lens
      • canvas
      • ml
      • maps
    • cloud
    • cluster_data_management
      • cross_cluster_replication
      • index_lifecycle_management
      • index_management
      • remote_clusters
      • rollup
      • snapshot_restore
    • platform
      • application
        • advanced_ui_actions
        • drilldowns
        • embeddable_enhanced
        • reporting
        • licensing
        • license_management
      • audit_trail
      • auth (FKA security plugin)
      • data
        • data_enhanced
      • encrypted_saved_objects
      • features
      • global_search
      • spaces
      • task_manager
      • telemetry
        • oss_telemetry
        • telemetry_collection_xpack
      • translations
      • upgrade_assistant
    • dev_tools
      • grok_debugger
      • console_extensions
      • searchprofiler
      • painless_lab
    • ingest
      • beats_management
      • ingest_manager
      • ingest_pipelines
      • logstash
    • observability
      • apm
      • infra
      • uptime
    • security (as in Security Solution)
      • case
      • siem
      • endpoint
      • lists

Execution

Once we have a consensus on a general structure, we should start moving to this incrementally. I propose that we start with the application_services domains. We can use this first domain to figure out any kinks in moving that much code and use what we learn to tackle the next domains.

We can then delegate the domains to individual teams to make the final moves.

Outstanding questions

  • How should we adapt our no-restricted-paths ESLint rule to be compatible with this new structure?
@joshdover joshdover added discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Jul 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@joshdover
Copy link
Contributor Author

cc @peterschretlen

@joshdover joshdover added Team:AppArch Team:Operations Team label for Operations Team labels Jul 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@joshdover
Copy link
Contributor Author

joshdover commented Jul 14, 2020

cc @epixa because I know "organize by domain" was a point you were previously passionate about :)

@pgayvallet
Copy link
Contributor

pgayvallet commented Jul 15, 2020

Eliminating the plugins directories

Is this something we already know we'll be doing, or only a proposal? TBH, I'm not that big of a fan for a couple reasons:

  • core get mixed at the same level as the domain folder, but they are very different in term of behavior (even in domain directory structures, we are just collecting plugins, only thing that changes is that we are not doing it in nested folders). Also that would kinda force to have [root]/src as a pluginSearchPaths, which is not that great as some 'non-domain' folder would be scanned unnecessarily.

  • As you stated, that forces to move everything 'not shipped' out of src to dev. Even if I won't disagree that it theoretically makes senses, that will be some some additional heavy refactoring / moving all the things that could be avoided (or at least handled in another issue, later) by preserving the plugin folder.

In short, that's only my opinion, but I think that having the suggested domain structure, while keeping it under the existing src/plugin and x-pack/plugins folders would really help us perform all the moving we want incrementally while respecting all the goals you listed.

@stacey-gammon
Copy link
Contributor

I like the goals of this effort and the outcome, but I wonder if we could alleviate some of @pgayvallet's concerns by tweaking the execution plan.

What about these steps:

  1. Migrate everything inside src/plugins/* into the hierarchy suggested, but keep inside src/plugins
  2. Migrate everything "not shipped" outside of "src".
  3. Incrementally migrate the top level folders out of src/plugins/* and into src/* (probably still wouldn't be able to do that in one go).

I think step 3 will be faster, and we avoid @pgayvallet's concern of having 'non-domain' folders scanned unnecessarily.

Also, @spalger just pulled this PR together for me: #71731, which relies on these plugin folders.

@peterschretlen
Copy link
Contributor

@pgayvallet has some good practical points and I'd love to hear other opinions from the team and contributors outside the team too.

This is already implied in the proposal, but to call it out directly: regardless of what we adopt, at some point moving away from layer-centric is a scale imperative. What does our repository structure look like in 2 years if we do nothing vs adopt a different structure? Reading the goals section, I think all the goals have a common theme of supporting a larger codebase and many more contributors than we have today.

Take x-pack: 2 years ago, x-pack folder had 17 plugins. Today master has 63, so a bit less than 4x. 2 years from now we're easily 100+ (yikes!) and if the growth trend continues as-is it'll be significantly higher. At some point layer-centric organization becomes impractical, so thank you @joshdover and the team for the clear proposal and trying to get ahead of this.

@joshdover
Copy link
Contributor Author

Eliminating the plugins directories

Is this something we already know we'll be doing, or only a proposal?

Nothing is certain yet, but we know we need a solution to scale our source code repository.

core get mixed at the same level as the domain folder, but they are very different in term of behavior (even in domain directory structures, we are just collecting plugins, only thing that changes is that we are not doing it in nested folders). Also that would kinda force to have [root]/src as a pluginSearchPaths, which is not that great as some 'non-domain' folder would be scanned unnecessarily.

The idea (though not explicitly stated above) is that directories should be able to contain all of the code that is related to a single domain. That code may include plugins, packages, types, documentation, etc. So the idea of moving out of the plugins directory is based on the premise that not everything in these domains will be plugins. Plugins may be the primary way these domains are integrated into the system, but under the hood they may be composed of other types of layers. And those other layers may also be exposed to other parts of the system. For example, the Telemetry domain may expose a usage_collection package that is not a plugin to be consumed by other domains.

As you stated, that forces to move everything 'not shipped' out of src to dev. Even if I won't disagree that it theoretically makes senses, that will be some some additional heavy refactoring / moving all the things that could be avoided (or at least handled in another issue, later) but preserving the plugin folder.

I actually don't think part is required in order to implement the rest. Depending on how we go about this, instead we could:

  1. Explicitly specify which directories out of src and x-pack should be copied into the build and used for plugin discovery; or
  2. Compile from src directly, outputting to build so that only the final artifacts make it into the build.

(1) sounds like more manual work than it is since I don't expect these domains to change often.

However I do think that having a strict separation between what goes in the build vs. what doesn't may be helpful in other contexts. But I agree it's not necessary right now.

In short, that's only my opinion, but I think that having the suggested domain structure, while keeping it under the existing src/plugin and x-pack/plugins folders would really help us perform all the moving we want incrementally while respecting all the goals you listed.

What about these steps:

1. Migrate everything inside `src/plugins/*` into the hierarchy suggested, but keep inside `src/plugins`

2. Migrate everything "not shipped" outside of "src".

3. Incrementally migrate the top level folders out of `src/plugins/*` and into `src/*` (probably still wouldn't be able to do that in one go).

This seems like a perfect workable plan to me, but it does require we move plugin code twice. I'm not sure what is more effort though. Maybe @elastic/kibana-operations can shed light on what adjustments we would need to make to the build system.

@kobelb
Copy link
Contributor

kobelb commented Jul 16, 2020

I share the opinions that @joshdover expressed in this comment.

I'd also like to add that not tying the directory structure to the concept of plugins gives us flexibility in defining what should be a plugin, without having to drastically restructure the code again. The only thing that absolutely has to be a plugin at this point is "x-pack", because it has to be conceptually installed and separate from the OSS build. We've historically used plugins at a much more granular level, and used them to enforce encapsulation when this isn't absolutely necessary.

@mshustov
Copy link
Contributor

application_services is a domain of services used to build other applications

It is relevant to the UI applications only? How it's different from the core? The Core cannot be disabled, maybe?

core is a domain of services that define or significantly augment how Kibana works

Is core === platform + Security plugin + Spaces plugin? If so, we might want to call it Platform. There are no other references to the core term in our codebase. Introducing a new team always adding more mental overloading.

  • test
    • utils
    • fixtures
    • es_archiver

The testing part is not fully covered in the proposal. I'd argue the all the functional tests that we have in /test & /x-pack/test folders and (probably src/fixtures as well) should be a part of an appropriate domain.
It’s essential for DX that packages can run any sort of domain-specific tests (unit, integration, functional) to get a quick feedback loop.
The same true for type check & build so that we can improve DX if our tooling support domain-specific development mode without waiting for new build chain to be ready. @elastic/kibana-operations Do you think it's possible to support it with the @kbn/optimizer.

x-pack

@joshdover How restructuring codebase affects the plugin IDs? Right now we have the same domains in OSS and x-pack folders. Do we want to formalize plugin naming schema?

@spalger
Copy link
Contributor

spalger commented Jul 20, 2020

It’s essential for DX that packages can run any sort of domain-specific tests (unit, integration, functional) to get a quick feedback loop.
The same true for type check & build so that we can improve DX if our tooling support domain-specific development mode without waiting for new build chain to be ready. @elastic/kibana-operations Do you think it's possible to support it with the @kbn/optimizer.

Maybe @elastic/kibana-operations can shed light on what adjustments we would need to make to the build system.

There isn't anything stopping us from supporting domain-specific development mode in the @kbn/optimizer. I just need more concrete tasks at this point. I don't think it will be complicated to support any of this proposal, including the updates to the build system, even if we need to make them multiple times I'm here to help.

I just expect that we will try to make the change and fix the things that are broken... I'm not sure how much will break, but based on my experience I don't think that anything could break that would be too much effort to fix.

I have a lot of confidence that once we have consensus on where we want to go we can get there by just starting a branch and moving forward. Sorry, I know that's not fantastic for planning exactly how many steps this will take but it's the best I can commit to.

@peterschretlen
Copy link
Contributor

@constancecchen @sqren @XavierM @nreese @cjcenizal would be curious to hear some opinions from other teams on the merits or potential issues of domain-based folders for your domains.

@nreese
Copy link
Contributor

nreese commented Jul 21, 2020

I think the proposal makes a lot of sense. I really like the idea of application_services folder as having all of these services in a single directory will make it easier to explore what's available.

@cee-chen
Copy link
Member

Thanks for the tag! 👍 / no strong objections here - from the Enterprise Search team's POV, this isn't a significant change from our current planned plugin directory structure. Based on the list in the top comment, sounds like we'd fall under x-pack like so:

  • x-pack
    • enterprise_search
      • overview
      • app_search
      • workplace_search
      • shared (shared components/helpers between AS and WS - is sharing code between plugins still OK to manage this way?)

Hopefully that looks sane.

@cjcenizal
Copy link
Contributor

We spent some time thinking about domains while reorganizing the Management apps and I think we arrived at some pretty good ones:

image

For the apps owned by ES UI, I think they'd make more sense grouped into alerting, data, stack, and ingest domains. The elasticsearch domain is too vague to me (everything depends on ES eventually! 😄).

  • alerting
    • alerting
    • alerting_builtins
    • alerts
    • watcher
  • data
    • cross_cluster_replication
    • remote_clusters
    • rollup
    • snapshot_restore
    • index_lifecycle_management
    • index_management
  • stack
    • license_management
    • upgrade_assistant
  • ingest
    • beats_management
    • ingest_manager
    • logstash
    • ingest_pipelines

application_services also sounds a little too vague, like we're leaving domain-world and edging back into layer-world. What defines an application service? Josh defined it as "a service used to build other applications", but I expect plugins to publish functionality that other plugins can consume and integrate with, as we seek out and identify more power plays. I worry that the boundaries of the application_services domain aren't clear enough. I wonder if can we move its contents into other domains or carve out new ones? Just my 2 cents.

@sorenlouv
Copy link
Member

sorenlouv commented Jul 22, 2020

@constancecchen @sqren @XavierM @nreese @cjcenizal would be curious to hear some opinions from other teams on the merits or potential issues of domain-based folders for your domains.

I think a domain-based structure makes sense and having all observability code in a single location is a good thing. I don't foresee any problems migrating to this structure.

@kobelb
Copy link
Contributor

kobelb commented Jul 23, 2020

During previous discussions, we decided that core provides low-level primitives, which every plugin in Kibana is dependent upon, and cross-cutting to all of Kibana. Where-as, application_services are a bit higher-level shared-services which are more for building applications, which tend to be UI focused. The differentiation between core and application_services is rather loose, and it's subject to change long-term. Collapsing both of these to be part of a platform domain seems completely reasonable, while exacerbating the naming of certain teams 😉. However, keeping them separate for the time-being minimizes the impact of the proposed changes.

You brought up a good point @restrry about the tests not being covered. I agree that we should ideally make these part of the appropriate domains. It might take us a bit to get here, but it makes sense to me.

@cjcenizal, I agree that the domains you all came up for the Management sections are great. @joshdover and I discussed whether or not it'd make sense to introduce a stack_management domain, which all of these management sections would be a part of, but it introduced some complexities and we decided against proposing it. There are quite a few other separate domains which also have their own management sections: Index Patterns, Saved Objects, Spaces, Advanced Settings, Security. These management sections are rather tightly coupled to the other code within their primary domains, so trying to make them all part of stack-management becomes complicated. In general, we're trying to group similar domains under the same top-level domain as it's our hope that this will facilitate code-sharing and better private and public APIs. Grouping the alert and actions plugins along with watcher feels wrong. Conceptually they're providing similar features, but from the code perspective, their architectures are entirely different.

Perhaps we should consider renaming the elasticsearch domain to elasticsearch_management and creating the sub-domains that you've suggested within it? Conceptually, all of these plugins seem to be related to managing Elasticsearch constructs. Long-term this will make it easier for you all to share code among all of these plugins.

@timroes
Copy link
Contributor

timroes commented Jul 27, 2020

Looks good to me from the Kibana App plugins. I've closed our Kibana App disucssion ticket (#70452) in favor of this one. The only thing missing from my side in the above list is the visualizations plugin. But since we're planning to merge the visualize plugin (containing the visualize application, with the listing page, the frame around the editors, etc.) with the visualizations plugin (the visualization infrastructure: vis type registry, etc.) I'd put it also inside the visualizations folder inside the analyze folder for now (even though for the infrastructure you could argue it might also be an application service).

@joshdover
Copy link
Contributor Author

Combining a few suggestions & discussions above, I've tried to regroup the "core" and "application_services" domains under a single "platform" domain which contains a few sub-domains.

  • src
    • platform
      • core (as in the "core platform" -- always present)
      • application (as in the "application platform")
        • embeddables
        • inspector
        • ui_actions
        • kibana_react
      • data (as in the "data platform")
        • data (should probably be broken up into separate plugins / domains?)
        • expressions
        • es_query
        • bfetch
  • x-pack
    • platform
      • application
        • advanced_ui_actions
        • drilldowns
        • embeddable_enhanced
        • reporting
        • licensing
      • data
        • data_enhanced
      • audit_trail
      • auth (FKA security plugin)
      • encrypted_saved_objects
      • features
      • global_search
      • spaces
      • task_manager
      • translations

There's still quite a few cross-cutting domains in the x-pack/platform directory that don't really seem to group well with other domains. I'm not sure we need to group them further though.

What I like about having a "platform" domain is that it makes it quite clear where most shared services live, whether they are plugins or not. Of course other domains may expose APIs, but the primary place for purely shared APIs can live under "platform".

The testing part is not fully covered in the proposal. I'd argue the all the functional tests that we have in /test & /x-pack/test folders and (probably src/fixtures as well) should be a part of an appropriate domain.
It’s essential for DX that packages can run any sort of domain-specific tests (unit, integration, functional) to get a quick feedback loop.

Agreed, this was not very clear. I intend for the test domain to be where testing utilities live. This could include tooling, fixtures, setup utilities, etc, but should not include anything that (1) tests something that belongs to another domain or (2) does not provide generally useful testing utilities. Anything that is specific to a domain should live in that domain (ideally under a test sub-domain). Also, if we are moving all non-production code out of src we should consider moving the test domain to the top-level as well (which would replace the existing test directory of e2e tests).

I agree also that we need to be able to move the e2e tests out of the top-level test directory and into the domains to which the tests are relevant. Maybe there is some very high-level integration tests done at the top-level domain, but these should be more or less only testing Kibana's integration layer, which is platform/core.

Grouping the alert and actions plugins along with watcher feels wrong. Conceptually they're providing similar features, but from the code perspective, their architectures are entirely different.

Perhaps we should consider renaming the elasticsearch domain to elasticsearch_management and creating the sub-domains that you've suggested within it? Conceptually, all of these plugins seem to be related to managing Elasticsearch constructs. Long-term this will make it easier for you all to share code among all of these plugins.

I tend to agree here. I think the UI groupings make sense from a user-perspective, but that the domains in our codebase do not necessarily have to map to what the user sees.

To take a step back, the goal of this proposal is to scale our codebase to more developers. Our premise is that using good software design practices will help accomplish that.

By organizing by domain, we can facilitate appropriate code sharing as well as proper encapsulation between domains. How we build software, organize our teams, and communicate across the company will be heavily influenced by this structure (and vice versa). This proposal allows teams to have their own "mini-platform" that is specific to their domain without sharing prematurely with the rest of the Kibana ecosystem. Sharing prematurely can lead to new, unintentional dependencies across the codebase on components that were not intended to be shared. These dependencies become a maintenance burden that can slow down new development.

It's also important that as our codebase grows, new and old developers alike can find services relevant to the task at hand. Having clearly labelled domains (and sub-domains!) is incredibly helpful, especially when we have a growing list of 100+ plugins.

The "data" grouping in the screenshot doesn't really map well to Kibana's codebase. It's too broad a term to describe what is actually inside of it, which are plugins related to user data indices. I also agree with @cjcenizal's point that "elasticsearch" may be too broad as well. We should consider labels that both describe what is inside the domain and how it is related to the rest of Kibana from an architecture standpoint (not necessarily a user standpoint). elasticsearch_management, data_indices_management, user_indices, user_es_data etc. would all make sense to me. However, I'm also working under the assumption that all of these plugins share some code, which may not be true.

The other domains you listed, CJ, make sense to me, with Watcher being the wild card here. I don't feel too strongly about where that goes. I agree it's architecturally distinct from Kibana's Alerting platform, however it is related from a domain perspective. If doesn't share much code with anything else, it seems fine to put it into alerting, but if it shares more code with the ES management UIs, then maybe it makes sense to put it in that domain. All it really does is CRUD against ES APIs anyways, so it seems related in that way to the user_es_data domain (or whatever we end up calling it).

@kobelb
Copy link
Contributor

kobelb commented Jul 28, 2020

When discussing with @cjcenizal outside of this GitHub issue, he brought up a good point that it's not immediately obvious why the proposed domain-oriented hierarchy satisfies the goals which were originally outlined. Below, I've expounded upon why I think the proposed hierarchy satisfies these goals, in an effort to provide more context.

Create a new repository structure that can grow with our codebase and organization

Currently, there are three primary categories of Kibana code: a plugin, a core subsystem, and a @kbn package. If code is a plugin, it must be placed within src/plugins/ or x-pack/plugins/. If code is a core subsystem, it must be placed within src/core/. And if something is a @kbn package it must be placed within packages/.

This is a rather rigid structure, that limits us from making certain changes like redefining what should be modeled as a plugin or what to model as a @kbn package. A lot of times these decisions end up being made for pragmatic reasons; for example, something becomes a @kbn package so it can be statically imported everywhere, including in developer tools or tests. These decisions require that the code be placed in specific folders, and incur other trade-offs.

Short-term, a majority of code will still be modeled as a plugin. However, long-term, I'd like to eliminate the differences between something being a core sub-system, a plugin or a @kbn package. No longer requiring plugins to be placed within a specific folder is a step in this direction.

Make exploring the codebase simpler and improve the discoverability of shared services and other foundational building blocks

The domain-oriented hierarchy should make it much easier to develop a mental model about Kibana's architecture. Grouping all of the plugins which contain shared-services as part of the platform domain, like @joshdover suggested here, makes it obvious that these plugins are accessible to all plugins and are always available.

Where-as grouping all plugins for a specific Solution within the same top-level domain will make it obvious which plugins are Solution-specific, and should only be used from within that Solution.

When a "component" is large-enough and isolated from everything else, it should be a "top-level domain", for example Alerting. If there are a bunch of smaller "components" which are all rather isolated, but conceptually are for a similar purpose, they should be grouped under another "top-level domain", for example all of the CLIs.

Group related code in the same area of the codebase

Ideally, all code with a sub-domain should exhibit high-cohesion. There are many different types of cohesion; however, a domain-oriented hierarchy like the one that's proposed in this issue allows us the freedom to structure our modules and plugins in this manner. This does depend on us getting the hierarchy right, and there will likely need to be some adjustments made over time.

Promote encapsulation between modules and reduced coupling across the codebase

This is primarily a long-term goal, which we won't necessarily realize by just putting the plugins into a folder-hierarchy. Long-term, we should provide more control over which plugin APIs and static-code should be accessible publicly vs internal to a sub-domain.

Exposing public APIs comes at a maintenance cost for both the "producer" and the "consumer". Whenever a public API changes, it either has to be done in a backward-compatible manner, which requires some forethought from the producer; or it is a breaking change, and requires changes to all consumers. Therefore, from the maintenance perspective, it's generally best to minimize what has to be a public API.

Currently, this encapsulation is generally done using a plugin. However, as soon as a plugin exposes a method on its setup or start contracts it becomes public and any other plugin can then depend on it. This comes at a disadvantage in situations where certain plugins are highly integrated with each and rely on different APIs than other types of consumers. Therefore, plugins that would benefit from having internal plugin APIs should be grouped together within the domain hierarchy. This way once we figure out the exact method of facilitating this access controls between plugins in sub-domains, it can be done non-invasively.

Pave the way for a new, modular toolchain based on distinct NPM packages

At this time, it's very difficult to implement a modular build. The lack of isolation between plugins and the frequent static code sharing makes this incredibly challenging. We've used parallelization as much as possible to make the build fast, but at some point, this just doesn't work any longer.

If we get the isolation between the various domains and sub-domains right, we should commonly see the code within a subset of the domain hierarchy changing at the same time when implementing a feature. This will allow the future modular build toolchain to only have to rebuild the parts of the code that changed and improve the developer experience.

Nothing in the proposed domain-hierarchy jumps out at me as something that changes in complete isolation, or would frequently change at the same time as another subdomain. However, there's likely something that I'm overlooking. Getting the domain-hierarchy right from the start isn't a necessity, and we can figure it out over-time.

@cjcenizal
Copy link
Contributor

cjcenizal commented Jul 28, 2020

Thanks for sharing your thoughts @joshdover and @kobelb! This is great context and I love this direction. I spoke with @alisonelizabeth and we consolidated our thoughts a bit.

Analysis/explanation of es_ui_shared

es_ui_shared contains code that's shared among most/all of the plugins owned by ES UI. If it contains code that isn't generalized beyond its ES UI consumers, this would be a strong indicator of a domain and a strong argument that the plugins that consume es_ui_shared belong in a single elasticsearch-type domain.

Alison and I took a look at this code through this lens and found that the majority of this code (with the exception of one or two small components) is generalizable and is intended to support general application development. In other words, if there were a process for contributing code to platform, we would enter this code into that process. With this in mind, we decided to ignore es_ui_shared, because it doesn't indicate a domain.

We do think es_ui_shared highlights a problem that's possibly beyond the scope of this proposal. Before code can be contributed to platform, it'll begin as code encapsulated within a domain. In order to test its generalizability, its maintainers might want to share it with other domains before contributing it to platform (or equivalent). What's the path for doing this? We weren't aware of one, which is why we created es_ui_shared -- we wanted to signal that the code inside is meant to be shared, but it's not yet canon. If we can define a formal path for sharing code more broadly to test its generalizability we can get rid of es_ui_shared.

Lack of cohesion within elasticsearch

I examined the plugins that would fall inside of the elasticsearch domain with the criteria from the cohesion Wiki definition in mind. As an exercise I looked at Ingest Node Pipelines, Snapshot Restore, and License Management. Is there...

  • ...logical cohesion? No, they don't do anything similar.
  • ...temporal cohesion? No, they don't execute any functionality at the same point in time, either within the UX or over the course of Kibana's execution.
  • ...procedural cohesion? No, they don't form any type of workflow, either in the UX or in terms of composable building blocks.
  • ...communicational cohesion? No, they don't consume or operate on the same entities.
  • ...sequential cohesion? No, they don't expose any functionality either in the form of UIs or modules that can be composed in any way.
  • ...functional cohesion? No, there is no single well-defined task that connects them.

From this process, I wasn't able to identify any defining characteristics for an elasticsearch domain.

Strong cohesion within cluster_data_management

OTOH, Alison and I felt there's clear cohesion among the apps that are coincidentally grouped in the Data section of the nav: cross_cluster_replication, remote_clusters, rollup, snapshot_restore, index_lifecycle_management, and index_management. Some signs of cohesion:

  • Functional cohesion: cross_cluster_replication, rollup, and index_lifecycle_management inject functionality and behavior into index_management and they all contribute to the task of managing the cluster data on a fundamental level.
  • Sequential cohesion: we use cross-linking and deep-linking to support inter-app workflows, e.g. cross_cluster_replication -> remote_clusters, snapshot_restore -> remote_clusters, and snapshot_restore -> index_management.
  • Temporal cohesion: at some point in the future we can imagine combining these apps so that functionality that users are likely to want to access at the same time will be grouped together, e.g. cross_cluster_replication and remote_clusters, index_lifecycle_management and rollup.

Note: rollup injects functionality into index_patterns. Will this still work if we put it into a domain?

Strong cohesion within ingest_management

We think ingest_pipelines has strong cohesion with the other plugins in the ingest_management domain that I suggested:

  • Functional cohesion: all of the apps contribute to the task of ingesting data into the cluster.
  • Functional cohesion: ingest_pipelines will export components and library code for consumption by ingest_manager.
  • Sequential cohesion:ingest_pipelines and ingest_manager will cross-link between one another.

Leftovers

With the above domains capturing their respective plugins, we're left with watcher, upgrade_assistant, and license_management. We don't feel strongly about where these end up. 😄 They don't seem to have any cohesion among them, so I don't think they should share a domain. Here are a couple possibilities that made sense to us, but we're also open to whatever y'all think is best:

  • Leave them all at the root-level. The simplest option.
  • Leave watcher at the root, and move the other two into platform. I think there's a strong argument that license_management and upgrade_assistant offer fundamental support for application development. Any app that is gated by a license higher than Basic will want to direct users to License Management, and could even surface a license warning via a component published by that plugin. Every plugin should consider its upgrade story and should coordinate the upgrade UX with Upgrade Assistant, which can involve cross-linking and sharing code.
  • Same as above, but move watcher into alerting. Due to practical considerations, Kibana Alerting and Watcher will likely never share code. But in an ideal world, we'd have a deprecation/migration story for moving users from Watcher to Kibana Alerting (maybe we do already?). As part of this story, I can imagine Kibana Alerting wanting to borrow a UI component or some library code to save dev time and to surface consistent UI/behavior to users that are migrating from one to the other. In this case, they'd extract the common code into a shared place within the alerting domain.

@stacey-gammon
Copy link
Contributor

We do think es_ui_shared highlights a problem that's possibly beyond the scope of this proposal. Before code can be contributed to platform, it'll begin as code encapsulated within a domain.
In order to test its generalizability, its maintainers might want to share it with other domains before contributing it to platform (or equivalent). What's the path for doing this?
We weren't aware of one, which is why we created es_ui_shared -- we wanted to signal that the code inside is meant to be shared, but it's not yet canon. If we can define a formal path for sharing code more broadly to test its generalizability we can get rid of es_ui_shared.

I believe the answer here is that you shouldn't. Code shouldn't be shared before it's contributed to the domain it belongs in. If the domain owners don't have the time to support and maintain the code, then the code should be kept private.

There are two aspects here:

  1. What is the process for sharing services before they are "GA".
  2. What is the process for contributing code to another domain.

It seems the situation with es_ui_shared was a mixture of both: the services are considered "beta" or "experimental", and they were built by a team that probably shouldn't be the final owners and maintainers of such a library.

We don't have a process for 1, though I do think it will be important to establish one.

The process for 2 involves cooperation between the two teams, and if the domain owners can't take it on, then the code should be kept private.

I think this is covered inside https://github.com/elastic/engineering/blob/master/kibana_dev_principles.md#dont-share-code-prematurely

Public APIs have overhead to maintain, and risks to consume. We evaluate third party packages because of these risks. Will bugs be addressed? How many maintainers of the package are there? How many users? What are the chances this package will stop being maintained?

Even if the code is great, our public services should have enough people to support and maintain it to give consumers confidence in those answers.

@cjcenizal
Copy link
Contributor

@stacey-gammon I agree with you. I think we're all generally aligned with these principles. At the same time, I think we need some processes, conventions, or tooling to support them. For example, what's the right way to share code between two teams while preventing a third team from also deciding to try it out? People want to do the right thing, and there are things we can do to help them. Even if's as simple as a convention like having a consistently labeled "Shared code" section in a plugin's or domain's root-level README. I'm conscious that we're wandering off the original topic of this issue though -- let me know if you'd like me to create a new one to keep the topics separate.

@stacey-gammon
Copy link
Contributor

For example, what's the right way to share code between two teams while preventing a third team from also deciding to try it out?

I think what I'm trying to get at is, there isn't. You shouldn't be sharing code between two domains, unless it's the prescribed, supported solution, owned by the appropriate domain owners.

I'm saying "domain" instead of team, because if two teams are working within a single domain, we want to support sharing it.

I do agree we need some tooling for sharing code within a domain, or subdomain, without sharing it publicly. This "organize by domain" folder structure should help us, if we can come up with a way to let code be exported from a plugin but not a domain. @joshdover - is there an issue for this yet?

As we continue to grow, we'll continue to reflect and iterate on our tooling and recommendations. Currently the stance is that all code shared from a plugin is public. Now we are seeing, based on how often very small plugins are created, we should support code that is "public only within a domain". The original thinking was that we'd have a handful of large plugins that covered domains, each which contained various nested services. As it turned out, that just isn't how developers are using the system. Instead, we are seeing many, very small plugins. We reflected, and now are iterating.

I hope that the es_ui_shared situation was a one-off, but if similar situations, with legit use cases, continue to come up, then we may need to re-evaluate. But for now I think the goal should be to avoid that situation by not sharing code prematurely.

@sebelga
Copy link
Contributor

sebelga commented Aug 3, 2020

Thanks @stacey-gammon. You answered my comment #72034 (comment).

You shouldn't be sharing code between two domains, unless it's the prescribed, supported solution, owned by the appropriate domain owners.

If we take the form lib for example. There might be 2/3 teams doing many forms that would benefit from it. It seems that there will never be an appropriate domain owner as it does not fall in the "useful for all" category.

If I hear you well, you are suggesting in this concrete scenario to copy/paste the whole form lib to the other team and let them maintain it (and have 2 copies of it in the repo)?

IMO there might be some middle ground: the way we are currently working with the Siem team. They are consuming the form lib but don't request any changes. If they'd like an improvement, they suggest it but are not blocked by it and let us plan when/if we address it. At the same time, if they find a bug, they communicate it to us and that's awesome because it might be a bug we hadn't noticed.
And thanks to that, we (the producer) are freed from any pressure on maintenance but we get notified on bugs. And they (the consumer) build forms much faster and get free upgrades on the form lib. Win/win 👍

I do agree though that this type of collaboration does require a stable API, tests and docs, which in this example we did not fully follow (I admit).

@stacey-gammon
Copy link
Contributor

in this concrete scenario to copy/paste the whole form lib to the other team and let them maintain it (and have 2 copies of it in the repo)?

Since we are already in this situation with the forms library, and multiple teams are using it, I think it's okay to leave exposed and owners as the elasticsearch team (all of this is about creating recommendations, not rules).

I see three better spots for it instead of es-ui-shared:

  • Put inside the new elasticsearch folder. Not very discoverable, but since it's in "maintenance mode" and not under active development, maybe that's a good thing?

  • Add new top level react-form-lib folder. Not really the most intuitive place for it (I think that would be kibana-react), but it's more intuitive than "es-ui-shared", and you can continue to be the sole domain owners.

  • Nest under kibana-react, and we'll break the recommendation of "single owner per plugin" in this case (kibana-react isn't a plugin anyway, just static code). In the README, specify that everything in this folder with the exception of the forms lib is owned by app arch, and forms lib is owned by es-ui team.

I leave up to you and the App Arch team to decide what to do with it ultimately.

There might be 2/3 teams doing many forms that would benefit from it. It seems that there will never be an appropriate domain owner as it does not fall in the "useful for all" category.

I think App Arch would be the clear domain owner of this. App Arch services and utilities don't have to be "useful for all" to be something they want to take on. The heuristic, imo, should be something like "Will maintaining and supporting this library improve Kibana developer experience and efficiency enough to offset the costs, while taking into account other priorities?". The answer would take into account:

  • How many devs is it useful for?
  • How much time can it save each dev?
  • What are the maintenance costs?
  • Are there other things we could be focusing on instead, for a greater payoff?

@joshdover
Copy link
Contributor Author

@cjcenizal I think the domains you came up with make more sense to me now. And +1 to using cohesion as a strong signal that components should belong to the same domain.

As for upgrade_assistant and license_management, I think those both make sense to belong to the platform domain. In fact, we should also probably consider combining the license_management and licensing plugins. Though they don't share any code that I am aware of, they are clearly part of the same domain and should probably be thought of as an atomic unit (and have a single owner).

For watcher, I am leaning strongly towards putting that in the alerting domain. Though it works completely differently, it is part of the same area of concerns and, like you pointed out, will likely want to share UI code at some point in the future as we push towards the new alerting solution.

If there are no objections, I will update the top-level description to reflect the new Elasticsearch domains CJ outlined above.

This "organize by domain" folder structure should help us, if we can come up with a way to let code be exported from a plugin but not a domain. @joshdover - is there an issue for this yet?

No issue, but I'm not sure there needs to be. The way domains expose code to other domains is no different than other NPM modules. They should define an index.ts file that defines which exports are available to other parts of the tree and we should not allow deep imports. Plugins inside a domain may share whatever they choose with other plugins within that domain without exporting that code from the domain's top-level index.

Since we are already in this situation with the forms library, and multiple teams are using it, I think it's okay to leave exposed and owners as the elasticsearch team (all of this is about creating recommendations, not rules).

These suggestions regarding the forms lib make sense to me as well. Let's get that code in the right long-term home and iterate from there. If the team does not have the bandwidth to accomdate all requests for changes from other teams, then other teams should either feel empowered to make the changes themselves (which introduces the danger of the forms lib becoming incoherent) or to make a copy ("fork") of the lib inside their own plugin.

Obviously having many copies can get out of hand, but there are downsides to trying to make a one-size-fits-all solution as well. Making practical decisions is the goal here, not to have a universal rule.

@shahzad31
Copy link
Contributor

Love this initiative, will be really helpful in improving developer experience.

Also big ++ to moving integration/functional tests at domain level. It's really hard to naviagte around code in current structure.

@walterra
Copy link
Contributor

walterra commented Aug 5, 2020

Great initiative! I'd like to add another example similar to es_ui_shared for reference. The ML team maintains the ml and transform plugin. We developed some React components (e.g. an extended version of EuiDataGrid) that are used by both plugins and are complex enough where we thought code duplication would be a burden to maintain. Similar to es_ui_shared, we broke some conventions to be able to share that code between plugins. Note this is only about statically shared code, the plugins are completely independent during runtime. As a team, we maintain and feel responsible for the code, but it's completely different domains. What would be the right way to move forward with this and how does it fit in the proposed concept? My thinking would be that these static components would not be exposed by either plugin, but the plugins themselves would import the components from an independent place. But on the other hand these components might not be intended to be completely "public" as in part of the platform or used by others right away. Don't get me wrong, we'd love to hear if those components would also be useful for others, but making something like that "public" would then require another level of effort (e.g. better documentation, supporting other teams using the components, more tests for generic/unknown use cases) and could be quite an overhead compared to maintaining the code as an implementation detail as a team and could stand in the way of delivering other things on time. In general I really like where this is going, so please consider this just as input how practicalities or everyday coding might get in the way of a theoretical concept :).

Maybe we can think of ways how team responsibilities, code ownership and expected target audiences of code can be part of the concept. Some very rough thoughts about the previously mentioned code sharing between plugins:

  • A SpecialDataGrid component could live somewhere in a generic components folder src/components.
  • src/components/special_data_grid would be assigned to the ML team in github's CODEOWNERS.
  • Linting rules would allow importing the component only into plugins/ml and plugins/transform. Note the intention would not be to completely prohibit the use by other teams, but to create awareness about it's intended scope of use: Similar to CODEOWNERS changing the linting rules to consume the component by another team/domain would intentionally trigger a discussion around maintenance and support responsibilities.

Hope these thoughts make sense and are useful 😅

@lukeelmers
Copy link
Member

The way domains expose code to other domains is no different than other NPM modules. They should define an index.ts file that defines which exports are available to other parts of the tree and we should not allow deep imports. Plugins inside a domain may share whatever they choose with other plugins within that domain without exporting that code from the domain's top-level index.

@joshdover Somehow in the discussion so far I had overlooked this implication. So this means that the top-level directory of each domain would represent an API boundary for sharing static code, e.g.:

// src/platform/public/index.ts
export * from '../core/public';
export * from '../data/public';
...etc

// somewhere in my plugin
  import { CoreStart, RandomThingFromDataPlugin } from 'src/platform/public';
  import { RandomThingFromKibanaUtils } from 'src/platform/kibana_utils/public';

I think there a few questions we'd need to answer around how this is implemented in practice, however that's probably a discussion for a different thread.

@joshdover
Copy link
Contributor Author

Somehow in the discussion so far I had overlooked this implication. So this means that the top-level directory of each domain would represent an API boundary for sharing static code

I think this pattern is a separate consideration, but this proposal definitely is designed with an eye towards improving module encapsulation. Having clearer boundaries between modules & domains would definitely help and I think linting that would make sense. It may also be needed to migrate to the new Bazel toolchain.

If we decided to go down this route, I think we'd probably need to introduce this gradually and it will most likely be a forcing function to refactor some modules to better fit with this pattern. IMO, those cases are probably code smells anyways and this exercise would probably improve the overall quality of the codebase.

@TinaHeiligers
Copy link
Contributor

@joshdover the Kibana Telemetry team had planned to start grouping our plugins into a structure proposed in the description of this issue but hadn't considered moving the combined grouping to platform. Should we still go ahead with our plans for the grouping in the existing plugin directory or wait?

@joshdover
Copy link
Contributor Author

@joshdover the Kibana Telemetry team had planned to start grouping our plugins into a structure proposed in the description of this issue but hadn't considered moving the combined grouping to platform. Should we still go ahead with our plans for the grouping in the existing plugin directory or wait?

Let's wait for us to decide this issue first to avoid any unnecessary churn in the codebase.

That said, I'd like to bring this proposal to a "final comment period" of sorts. It seems like we have a consensus on this general direction and the high-level organization of these domains. If anyone has any fundamental flaws or concerns with this plan, please raise them now.

If no fundamental flaws are raised by EOD Friday September 4th, I propose we close this issue and begin planning the work to make this a reality.

@TinaHeiligers
Copy link
Contributor

@joshdover thanks for the info.
I don't see any telemetry-related plugins in the proposal for the x-pack structure.
We'll need to decide where to put:

  • x-pack/plugins/oss_telemetry
  • x-pack/plugins/telemetry_collection_xpack.
    The naming of these plugins needs some work but they allow us to use the plugins in src.

@joshdover
Copy link
Contributor Author

joshdover commented Aug 24, 2020

We'll need to decide where to put:

  • x-pack/plugins/oss_telemetry

  • x-pack/plugins/telemetry_collection_xpack.
    The naming of these plugins needs some work but they allow us to use the plugins in src.

I think we should put these under x-pack/platform/telemetry to mirror the OSS structure.

EDIT: I updated the structure above to include these

@joshdover
Copy link
Contributor Author

With no objections, I'm closing this issue. The Platform team will coordinate an execution plan in a separate follow up issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team
Projects
None yet
Development

No branches or pull requests