Skip to content

[Logs Shared] Move LogStream and LogView into new shared plugin#159279

Closed
tonyghiani wants to merge 360 commits intoelastic:mainfrom
tonyghiani:159128-split-log-stream-into-plugin
Closed

[Logs Shared] Move LogStream and LogView into new shared plugin#159279
tonyghiani wants to merge 360 commits intoelastic:mainfrom
tonyghiani:159128-split-log-stream-into-plugin

Conversation

@tonyghiani
Copy link
Copy Markdown
Contributor

@tonyghiani tonyghiani commented Jun 8, 2023

📓 Summary

Closes #159128

Due to a dependencies issue when disabling a plugin in serverless mode, the LogStream feature and related logic were disabled for every consumer.

We decided to split this shared component and endpoint into their own plugin of shared logs utilities, reducing to the minimum the required dependency that could disable the plugin.

What we moved can be summarized with:

  • infrastructure-monitoring-log-view saved object definition and registration
  • LogViews server/client services (exposed with start contract) + related endpoints
  • LogEntries server service + related endpoints
  • LogEntriesDomain logic (exposed with start contract)
  • <LogStream /> component
  • <ScrollableLogTextStreamView /> component and related logic
  • LogView state machine
  • Containers/Hooks to consume the moved APIs.
  • Common types/utils definition, now exported and consumed as a dependency from the infra plugin.

🤓 Review hints

Most of the changes are just renaming and moving stuff into the new plugin, but for some operations was required to implement new logic, which may deserve a more critical review:

  • server/public plugin.ts files for the infra and logs_shared plugins. The new plugin now registers the fallback actions to retrieve a source configuration if there's no stored log view. It also set the configuration for the message field and registers the log view saved object.
  • the logEntriesDomain has also been moved inside the new plugin, but is also used by the logs-analysis endpoints, so it is exposed by the logs_shared plugin and consumed by infra.

👣 Following steps

We currently are still using the observability plugin for consuming the CoPilot feature on our LogsStream flyout.
The plugin dependency is marked as optional, so disabling the observability plugin in a serverless environment won't disable also the exposed features in this new plugin, but it'll affect only the CoPilot feature, which won't be loaded.

In future, would be nice to extract the CoPilot feature into its own package/plugin, so that also serverless projects can consume it without depending on `observability.

@ghost
Copy link
Copy Markdown

ghost commented Jun 8, 2023

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@tonyghiani tonyghiani changed the title 159128 split log stream into plugin [Logs Shared] Split LogStream and LogView into new shared plugin Jun 8, 2023
@tonyghiani tonyghiani changed the title [Logs Shared] Split LogStream and LogView into new shared plugin [Logs Shared] Move LogStream and LogView into new shared plugin Jun 8, 2023
@tonyghiani tonyghiani added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes labels Jun 15, 2023
@tonyghiani tonyghiani marked this pull request as ready for review June 19, 2023 14:02
@tonyghiani tonyghiani requested review from a team as code owners June 19, 2023 14:02
@tonyghiani tonyghiani requested review from a team June 19, 2023 14:02
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

Copy link
Copy Markdown
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upgrade assistant changes lgtm

@botelastic botelastic bot added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Fleet Team label for Observability Data Collection Fleet team labels Jun 19, 2023
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/apm-ui (Team:APM)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/fleet (Team:Fleet)

licenseManagement: 41817
licensing: 29004
lists: 22900
logsShared: 281060
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we looked into loading parts of this plugin async?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jbudz, I was looking into this today and hopefully I'll be able to reduce this bundle size 👌

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lazy loaded all the components I can, but most of the exported logic lives in utilities and state machines imported by the infra plugin, so the bundle size gain is minimum :(

Copy link
Copy Markdown
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fleet change LGTM

Copy link
Copy Markdown
Contributor

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apm changes lgtm

@tonyghiani tonyghiani requested a review from a team June 23, 2023 09:09
Copy link
Copy Markdown
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AO changes LGTM

achyutjhunjhunwala and others added 3 commits June 26, 2023 19:41
…ic#159990)

## Summary

Closes elastic#157479

This PR adds a check in Service Groups to display overflow bucket if
Service metrics have been overflow


![image](https://github.com/elastic/kibana/assets/7416358/714ff445-0c10-4239-b4b6-0064019178ca)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@elastic/charts](https://github.com/elastic/elastic-charts) |
[`57.0.1` ->
`58.2.0`](https://renovatebot.com/diffs/npm/@elastic%2fcharts/57.0.1/58.2.0)
|
[![age](https://badges.renovateapi.com/packages/npm/@elastic%2fcharts/58.2.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@elastic%2fcharts/58.2.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@elastic%2fcharts/58.2.0/compatibility-slim/57.0.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@elastic%2fcharts/58.2.0/confidence-slim/57.0.1)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>elastic/elastic-charts</summary>

#
[58.2.0](elastic/elastic-charts@v58.1.0...v58.2.0)
(2023-06-23)


### Bug Fixes

* `Chart` component `children` type
([elastic#2071](elastic/elastic-charts#2071))
([525c782](elastic/elastic-charts@525c782))
* **deps:** update dependency @elastic/eui to v82
([elastic#2074](elastic/elastic-charts#2074))
([69a655f](elastic/elastic-charts@69a655f))


### Features

* **flame:** expose search field text and search text change listener
([elastic#2068](elastic/elastic-charts#2068))
([c339947](elastic/elastic-charts@c339947))
* support native chart title and description
([elastic#2002](elastic/elastic-charts#2002))
([341a990](elastic/elastic-charts@341a990))

#
[58.1.0](elastic/elastic-charts@v58.0.0...v58.1.0)
(2023-06-08)


### Features

* **flame:** expose search control
([elastic#2064](elastic/elastic-charts#2064))
([011b56b](elastic/elastic-charts@011b56b))

#
[58.0.0](elastic/elastic-charts@v57.0.1...v58.0.0)
(2023-06-06)


### Bug Fixes

* **axis:** reduce number of y axis ticks on linear scale
([elastic#2005](elastic/elastic-charts#2005))
([0ef828b](elastic/elastic-charts@0ef828b))
* **deps:** update dependency @elastic/eui to v81
([elastic#2052](elastic/elastic-charts#2052))
([4c55e01](elastic/elastic-charts@4c55e01))


### BREAKING CHANGES

* **axis:** the default number of desired ticks in the Y-Axis was
changed from `10` to `5`

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/elastic/kibana).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTAuMCIsInVwZGF0ZWRJblZlciI6IjM1LjExMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: nickofthyme <nicholas.partridge@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
@tonyghiani tonyghiani requested review from a team as code owners July 4, 2023 08:01
@tonyghiani tonyghiani requested a review from a team July 4, 2023 08:01
@tonyghiani tonyghiani requested review from a team as code owners July 4, 2023 08:01
@tonyghiani
Copy link
Copy Markdown
Contributor Author

Closing and reopening as #161151

@tonyghiani tonyghiani closed this Jul 4, 2023
@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Jul 4, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Threat Intelligence Tests #2 / Invalid Indicators verify the grid loads even with missing fields should display data grid despite the missing fields
  • [job] [logs] Threat Intelligence Tests #2 / Timeline should verify add to timeline and investigate in timeline work from various places
  • [job] [logs] Threat Intelligence Tests #2 / Timeline should verify add to timeline and investigate in timeline work from various places

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1474 1380 -94
logsShared - 179 +179
total +85

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
infra 46 41 -5
logsShared - 256 +256
total +251

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
logsShared - 10 +10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 2.0MB 2.0MB -56.1KB
logsShared - 56.8KB +56.8KB
total +746.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
infra 14 11 -3
logsShared - 27 +27
total +24

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
apm 36.2KB 36.2KB +5.0B
enterpriseSearch 38.0KB 38.0KB +5.0B
fleet 133.8KB 133.8KB +5.0B
infra 119.7KB 93.8KB -25.9KB
logsShared - 218.7KB ⚠️ +218.7KB
total +192.8KB
Unknown metric groups

API count

id before after diff
infra 49 44 -5
logsShared - 269 +269
total +264

async chunk count

id before after diff
infra 29 27 -2
logsShared - 8 +8
total +6

ESLint disabled in files

id before after diff
infra 10 9 -1
logsShared - 3 +3
total +2

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
infra 52 51 -1
logsShared - 6 +6
securitySolution 410 414 +4
total +11

References to deprecated APIs

id before after diff
infra 65 60 -5
logsShared - 5 +5
total -0

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
infra 62 60 -2
logsShared - 9 +9
securitySolution 489 493 +4
total +13

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@tonyghiani tonyghiani deleted the 159128-split-log-stream-into-plugin branch July 13, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Fleet Team label for Observability Data Collection Fleet team Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split log stream into a separate plugin