elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.69k stars 8.24k forks source link

[Fleet] Decouple agent logs LogStream config from space-specific log source configuration #107920

Open weltenwort opened 3 years ago

weltenwort commented 3 years ago

:memo: Summary

The agent logs shown on Fleet's agent page embed the <LogStream> component provided by the Logs UI. While doing so it doesn't specify a sourceId, which means it defaults to the user-configurable source configuration for the currently active space. This in turn means the agent log display might break when the user changes the log source configuration to not include the indices containing the agent logs. To avoid that the fleet plugin could define an internal source configuration, which is space-independent can not be changed by the user.

relates to beats/issues#27282

:heavy_check_mark: Acceptance criteria

:warning: These ACs are only a proposal and should be adjusted during refinement.

:hammer_and_wrench: Background information

The agent details page embends the <LogStream> here:

https://github.com/elastic/kibana/blob/58054c332508b29d119ff84fe7b622eaefeec2df/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_details_page/components/agent_logs/agent_logs.tsx#L348-L353

To avoid the aforementioned interference with the user-configurable log source configuration, the component can take a sourceId property, which can refer to an "internal" immutable source injected into the configuration using the defineInternalSourceConfiguration function provided by the setup contract of the infra plugin.

The process is documented in https://github.com/elastic/kibana/blob/58054c332508b29d119ff84fe7b622eaefeec2df/x-pack/plugins/infra/public/components/log_stream/log_stream.stories.mdx#with-a-source-configuration. Several example usages can be found in the Kibana code base, such as the following:

https://github.com/elastic/kibana/blob/58054c332508b29d119ff84fe7b622eaefeec2df/x-pack/plugins/enterprise_search/server/plugin.ts#L170-L176

elasticmachine commented 3 years ago

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

elasticmachine commented 3 years ago

Pinging @elastic/fleet (Team:Fleet)

hop-dev commented 3 years ago

I believe I need to add infra as a requiredPlugin of fleet so I can call infra.defineInternalSourceConfiguration however that causes the following cyclical dependency:

fleet --required-> infra --optional-> osquery --optional-> fleet

Going to start looking at my options. Looks like infra only introduced the osquery dependency a week ago in this PR https://github.com/elastic/kibana/pull/104272/files

hop-dev commented 3 years ago

@jen-huang I think this may end up being quite a lot of work unless there's a simple solution to the cyclical dependency that I'm missing.

I would've liked to add it to the agenda for one of our engineering meetings but the next one isn't until the 14th Sept, I will try link up with @nchaulet when he gets back he may have some ideas. I am moving to blocked for now.

joshdover commented 3 years ago

For circular dependencies in Kibana, we generally have two options:

If we want to try inverting one of the dependencies here to avoid the circular dependency, there's really 3 options:

  1. Invert the dependency between fleet and infra
    • Have the infra plugin depend on fleet plugin instead. We'd need to expose an API from fleet (eg fleet.registerInternalSourceConfigurationProvider that infra could register itself with so that we can access infra's defineInternalSourceConfiguration when it's available.
    • I think this may not work by itself though because we currently have a requiredBundle on the infra plugin for the UI code for the LogStream plugin. We may need to invert this dependency as well and have this component provided at runtime via a new API exposed by the Fleet public plugin such as fleet.registerLogStreamComponent to allow the infra plugin to call this manually.
    • To do this, we'd also need to handle the case where the infra plugin is disabled and have some sort of fallback UI (maybe just a warning like "Enable the infra plugin to view logs"). This problem will likely be eliminated in 8.0 if we don't allow the infra plugin to be disabled any more (see #89584).
  2. Invert the dependency between infra and osquery
    • It appears infra plugin already has logic for when the osquery plugin is not enabled / available so this may be the easiest dependency to invert.
    • infra would need to expose an API like registerOsQueryActionComponent and have the osquery plugin call this on the infra plugin.
  3. Invert the dependency between osquery and fleet
    • TBH I wouldn't explore this option because it feels architecturally wrong to have Fleet depending on a specific integration app like osquery.

Option (1) feels like a good architectural direction as it avoids adding more dependencies to Fleet (actually removes one), however it does require we handle the case where the infra plugin is not available/enabled (which is going away soon) or there is a bug where infra doesn't call the Fleet plugin API for some reason.

Option (2) probably has the fewest required changes, but it requires that we touch the infra plugin and I'm not sure if there are future integrations between osquery and infra that this change would make harder. If you choose to go this route, we'd probably want to do the infra and osquery changes as a separate PR first and then add the new fleet -> infra dependency in the next pass.

Long-term, I think it probably makes sense for these logging components and services to be extracted from the infra plugin entirely into something more fine-grained to avoid this problem across the project as more teams consume these logging bits.

hop-dev commented 3 years ago

Thanks @joshdover this is incredibly useful!

Hi @patrykkopycinski I see that you did the osquery work for https://github.com/elastic/kibana/pull/104272, I wondered if you had any thoughts on option 2 above, and whether it would hinder any future work you have planned.