Filters are no longer being picked up by `useData` in the log pattern
analysis flyout, this looks like it might be a race condition caused by
the `useEffect` which loads the state data.
This state data will never change while the flyout is open and so rather
than using a initial render `useEffect` the state can be loaded and used
when `aiopsListState` is initialized.
Steps to view problem
* install sample data set
* Open lens visualization
* Open inspector. Notice console errors
<img width="300" alt="Screen Shot 2023-05-05 at 11 03 25 AM"
src="https://user-images.githubusercontent.com/373691/236521366-d8fb9302-e93b-4047-a0bf-d7c09dcc3ffb.png">
https://github.com/elastic/eui/pull/6566 removed `closeButtonAriaLabel`
prop from [EuiFlyout](https://elastic.github.io/eui/#/layout/flyout) EUI
75.0.0 (Effecting 8.8 and 8.9). FlyoutService spreads options into
`EuiFlyout`, resulting in `closeButtonAriaLabel` getting added to dom
and causing error.
`OverlayFlyoutOpenOptions` type added by
https://github.com/elastic/kibana/issues/37894. I replaced
`OverlayFlyoutOpenOptions` with `EuiFlyoutProps` to make it more clear
what props are accepted and provide stronger typing that stays in sync
with EUI typings
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Fixes the React dev mode warning "Warning: Each child in a list should
have a unique "key" prop. Check the render method of EuiBasicTable" by
putting the `key` attribute for a group item on the outer level.
Adding ML's log pattern analysis results into a Discover by triggering a
flyout from the stats menu.
Only text fields contain the run pattern analysis button.
A filter can then be applied from one or more patterns which will cause
Discover to only show documents which match that category.
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
- Moves code from `x-pack/plugins/ml/common/util/errors` that was shared
via `x-pack/plugins/ml/public/shared.ts` to `@kbn/ml-error-utils`.
- `data_visualizer` and `aiops` plugins now use that package instead of
code duplication.
Fixes an issue there the global state `_g` and app state `_a` would get
out of sync and overwrite each other. For example, a click on Refresh in
the date picker (global state) could reset the search bar (app state) to
empty.
The issue was that in `x-pack/packages/ml/url_state/src/url_state.tsx`
the `searchString` could become a stale value in `setUrlState`. This PR
fixes it by using the approach already used in `usePageUrlState`: The
`searchString` is passed on to be stored via `useRef` so that the
`setUrlState` setter can always access the most recent value.
- Adds an action to the analysis results filter to copy a KUERY filter
for each row to the clipboard.
- Consolidates duplicate code for the Discover action into a custom
hook.
- Adds a `Filter fields` popover selector inspired by EUI's data grid
column picker to toggle inclusion of fields into grouping.
- Moves the `Group results` switch and the `Filter fields` popover on
the same level as the progress controls.
- Adapts the `explain_log_rate_spikes` API endpoint to support
retrieving a grouping update only.
- Hides the pagination footer for the results tables if there's less
results than the current page size.
- Refactors individual helpers to manage the `random_sampler`
aggregation into a single reusable wrapper. The helper's factory can be
supplied with either a sample probability right away or a document count
to generate a dynamic sample probability based on it.
- Applies random sampling to the main date histogram chart.
Similar to https://github.com/elastic/kibana/pull/148746, adds execution
context to the AIOps Explain Log Rate Spikes API endpoint.
Tested with an analysis run which produces:
```
"elasticsearch.slowlog.id":"6ad55893-c4de-4571-a4a4-4b843e889669;kibana:application:ml:%2Faiops%2Fexplain_log_rate_spikes;application:aiops:%2Finternal%2Faiops%2Fexplain_log_rate_spikes"
```
The AIOps plugin currently has only one endpoint in total for the
Explain Log Rate Spikes analysis therefor I added the necessary code
just inline into that one route. All other calls are for now done using
the `esSearch` endpoint provided by the `ml` plugin. We are starting an
effort to move those calls to dedicated endpoints
(https://github.com/elastic/kibana/issues/153458). Once we start that
effort for the AIOps plugin we should move both license and this context
code to a shared api guard similar to the ml/transform plugins.
- Makes use of the `filter` option for the `frequent_item_sets`
aggregation introduced in `8.7.0`
(https://github.com/elastic/elasticsearch/pull/92414).
- Several fixes to grouping code, esp. around detection of multiple
non-overlapping groups.
- Improved which items of a group to display in the summary. Previously,
up to 15 items that were unique to a group were displayed. Related to
the above mentioned improvement it would mean that for groups that had
no unique items then no items would show up in the summary. Now we sort
items by duplicates and doc count ascending (a sort of "uniqueness"
score) and show up to 5 items of those.
- The Group button code has been simplified.
👋 Hi all - the biggest breaking change of this PR is around two icon
type changes/renames.
1. ⚠️ **The `alert` icon is now named `warning`**
- <img width="103" alt=""
src="https://user-images.githubusercontent.com/549407/223561599-8913e88c-676f-47cd-aaed-81b64783bd81.png"
align="middle">
- This change should have been automatically converted on your behalf by
the EUI team, **but if for some reason** we missed making this
conversion in this PR and your icon(s) are now broken, please ping us or
let us know in this PR (or fix yourself after this PR merges).
- In some cases, teams were using this icon for error messages,
alongside the `danger` color. In those cases, we opinionatedly changed
those icon usages to the new `error` icon instead of using the old
alert/warning icon.
2. 🛑 **The `crossInACircleFilled` icon has been removed, and a new
`error` icon added**
- <img width="84" alt=""
src="https://user-images.githubusercontent.com/549407/223561892-4406bdf6-1a55-49ac-85ad-3a11eb7c090d.png"
align="middle">
- The conversion for this breaking change was not straightforward. This
was the path we used to determine what to change `crossInACircleFilled`
usages to:
- If the icon was associated with errors or error messages, we changed
it to the new `error` icon.
- If a "delete" action was associated with this icon, we changed it to
the `trash` icon instead.
- If a "clear" action was associated with this icon, we changed it to
just the `cross` icon, or in some cases `minusInCircleFilled` (if used
alongside `plusInCircleFilled`).
- Again, if we made a mistake during this conversion or missed your
plugin, please feel free to ping us.
## Summary
`eui@75.1.2` ⏩ `eui@76.0.2`
## [`76.0.2`](https://github.com/elastic/eui/tree/v76.0.2)
**Bug fixes**
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([#6640](https://github.com/elastic/eui/pull/6640))
## [`76.0.1`](https://github.com/elastic/eui/tree/v76.0.1)
**Bug fixes**
- Fixed broken icons on all `isInvalid` form controls
([#6629](https://github.com/elastic/eui/pull/6629))
## [`76.0.0`](https://github.com/elastic/eui/tree/v76.0.0)
- Added `pivot` glyph to `EuiIcon`
([#6605](https://github.com/elastic/eui/pull/6605))
- Added the `displayHeaderCellProps` API to `EuiDataGrid`'s columns,
which allows passing custom props directly to column header cells
([#6609](https://github.com/elastic/eui/pull/6609))
- Added the new `headerCellProps`/`footerCellProps` APIs to
`EuiDataGrid`'s control columns, which allows passing custom props
directly to control column header or footer cells
([#6609](https://github.com/elastic/eui/pull/6609))
- Added a new `footerCellRender` API to `EuiDataGrid`'s control columns,
which allows completely customizing control column rendering (previously
rendered an empty cell)
([#6609](https://github.com/elastic/eui/pull/6609))
- Updated the styling of nested ordered lists in `EuiText` to align with
GitHub's list style, which is a popular format used in Markdown or MDX
formatting ([#6615](https://github.com/elastic/eui/pull/6615))
- Added a margin-bottom property exclusively to the direct child `ul`
and `ol` elements of the `EuiText` component
([#6615](https://github.com/elastic/eui/pull/6615))
- Fix issue with badges appearing within an `EuiBadgeGroup`, where the
CSS rule to override the `margin-inline-start` was not being applied
correctly due to the order of appearance in the CSS rules
([#6618](https://github.com/elastic/eui/pull/6618))
**Bug fixes**
- Fixed `EuiDataGrid` footer control columns rendering with cell
expansion popovers when they should not have been
([#6609](https://github.com/elastic/eui/pull/6609))
- Fixed an `EuiSkipLink` bug where main content loading in
progressively/dynamically after the skip link rendered was not being
correctly focused ([#6613](https://github.com/elastic/eui/pull/6613))
**Breaking changes**
- Renamed `EuiIcon`'s `alert` to `warning`
([#6608](https://github.com/elastic/eui/pull/6608))
- Removed `EuiIcon`'s `crossInACircleFilled` in favor of `error`
([#6608](https://github.com/elastic/eui/pull/6608))
---------
Co-authored-by: Davey Holler <daveyholler@hey.com>
Co-authored-by: Constance Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Jon <jon@elastic.co>
Quite some code and comments referred to `change points` in the Explain
Log Rate Spikes code. However, the underlying aggregation to get the
corresponding data is based on the `significant_terms` aggregation. The
naming mismatch was originally introduced when porting code from Python
prototypes that included this naming.
To avoid confusion with the recently introduced actual "Change Point
Detection" feature, this PR renames references to changes points in the
Explain Log Rate Spikes code to refer to significant terms.
Note this was only referenced like this in code and comments, no wording
in the UI was used that referred to "Change Points" in the Explain Log
Rate Spikes feature.
## Summary
This PR makes small changes to the strings of helper text in the ML UI.
It also slightly changes the transform UI text. It mostly adds some
missing full stops to the end of the sentences for the sake of
consistency.
Fixes https://github.com/elastic/kibana/issues/149344
This PR migrates all plugins to packages automatically. It does this
using `node scripts/lint_packages` to automatically migrate
`kibana.json` files to `kibana.jsonc` files. By doing this automatically
we can simplify many build and testing procedures to only support
packages, and not both "packages" and "synthetic packages" (basically
pointers to plugins).
The majority of changes are in operations related code, so we'll be
having operations review this before marking it ready for review. The
vast majority of the code owners are simply pinged because we deleted
all `kibana.json` files and replaced them with `kibana.jsonc` files, so
we plan on leaving the PR ready-for-review for about 24 hours before
merging (after feature freeze), assuming we don't have any blockers
(especially from @elastic/kibana-core since there are a few core
specific changes, though the majority were handled in #149370).
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
## Summary
Addresses https://github.com/elastic/kibana/issues/147705 and fix an
issue with ML filter bar in Data Visualizer and AIOps pape not
supporting complex filters correctly in 8.7. Changes include:
- Using the `getSavedSearch` utility to get the `selectedSavedSearch`
instead of the SavedSearchSimpleObj (which we previously manually parsed
the json search source).
- This requires some additional page dependency to be injected.
- Rename `currentSavedSearch` to `deprecatedSavedSearchObj` for clarity.
Note that not all deprecatedSavedSearchObj has been replaced with the
new `selectedSavedSearch`. Areas that still use the
deprecatedSavedSearchObj:
- AD job creator
Potential todos:
- Support for loading the saved time range & refresh interval upon
restoring the saved search
- Support for AggregateQuery (currently, we only support lucene and
kuery query types) #150091
### Checklist
Delete any items that are not applicable to this PR.
- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
### Risk Matrix
Delete this section if it is not applicable to this PR.
Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.
When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:
| Risk | Probability | Severity | Mitigation/Notes |
|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces—unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes—Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |
### For maintainers
- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
The `useEffect` hooks that set up subscriptions related to the
`timefilter` were missing `[]` to be only called when the code is
mounted, without it they were called on every render. This PR adds the
missing `[]` and also combines the two hooks into one.
Improve responsive behaviour of page header with date picker.
- Removes custom breakpoint based code to determine date picker width
and instead use it's native `width` option.
- Adds a `flexGroup` boolean toggle to be able to get back the flex
items only without the flex group if you want to embed the date picker
in an already existing flex group to avoid additional nesting.
- Sets the `fill` option of the refresh button to `false` to avoid the
dark blue "primary".
- In the `aiops` plugin and the `ml_page.tsx` component, migrates away
from EUI's deprecated components for the page layout.
- Adds a `min-width` to page titles to avoid narrow wrapping (e.g.
wrapping after each character on narrow screens).
- Moves duplicates of `DatePickerWrapper` and related code to package
`@kbn/ml-date-picker`. The duplicated components across the code base
have been consolidated and diverging features combined. Each duplicate
has been checked with a diff against the package before deletion.
- Moves duplicates of `query_utils.ts` to a package
`@kbn/ml-query_utils`.
- Some jest test were migrated from enzyme to react-testing-lib.
- `i18n` strings and data-test-subjects have been updated to be prefixes
in line with package names.
- Replaces custom code related to the `compact` flag with EUI's
breakpoints.
Moves multiple copies of `useStorage()` and related code to a package as
a single source. The different copies with hard coded types have been
adapted so `useStorage()` is now based on generics. Also moves
duplicates of `isDefined()` to its own package.
Effort to deduplicate code. Move nested property utilities and url state
to packages.
Boilerplate for the packages was created likes this:
```
node scripts/generate package @kbn/ml-url-state --web --dir ./x-pack/packages/ml/url_state
node scripts/generate package @kbn/ml-nested-property --web --dir ./x-pack/packages/ml/nested_property
```
I consolidated the different `url_state.ts` files. One thing to note:
Each one had its own definition for `pageKey: AppStateKey`. I changed
that and made it just `pageKey: string`, I suspect it's good enough.
Otherwise we'd have a reverse dependency on all consuming code.
Alternative: We could refactor to require overriding a generic to pass
in allowed values.
## Dearest Reviewers 👋
I've been working on this branch with @mistic and @tylersmalley and
we're really confident in these changes. Additionally, this changes code
in nearly every package in the repo so we don't plan to wait for reviews
to get in before merging this. If you'd like to have a concern
addressed, please feel free to leave a review, but assuming that nobody
raises a blocker in the next 24 hours we plan to merge this EOD pacific
tomorrow, 12/22.
We'll be paying close attention to any issues this causes after merging
and work on getting those fixed ASAP. 🚀
---
The operations team is not confident that we'll have the time to achieve
what we originally set out to accomplish by moving to Bazel with the
time and resources we have available. We have also bought ourselves some
headroom with improvements to babel-register, optimizer caching, and
typescript project structure.
In order to make sure we deliver packages as quickly as possible (many
teams really want them), with a usable and familiar developer
experience, this PR removes Bazel for building packages in favor of
using the same JIT transpilation we use for plugins.
Additionally, packages now use `kbn_references` (again, just copying the
dx from plugins to packages).
Because of the complex relationships between packages/plugins and in
order to prepare ourselves for automatic dependency detection tools we
plan to use in the future, this PR also introduces a "TS Project Linter"
which will validate that every tsconfig.json file meets a few
requirements:
1. the chain of base config files extended by each config includes
`tsconfig.base.json` and not `tsconfig.json`
1. the `include` config is used, and not `files`
2. the `exclude` config includes `target/**/*`
3. the `outDir` compiler option is specified as `target/types`
1. none of these compiler options are specified: `declaration`,
`declarationMap`, `emitDeclarationOnly`, `skipLibCheck`, `target`,
`paths`
4. all references to other packages/plugins use their pkg id, ie:
```js
// valid
{
"kbn_references": ["@kbn/core"]
}
// not valid
{
"kbn_references": [{ "path": "../../../src/core/tsconfig.json" }]
}
```
5. only packages/plugins which are imported somewhere in the ts code are
listed in `kbn_references`
This linter is not only validating all of the tsconfig.json files, but
it also will fix these config files to deal with just about any
violation that can be produced. Just run `node scripts/ts_project_linter
--fix` locally to apply these fixes, or let CI take care of
automatically fixing things and pushing the changes to your PR.
> **Example:** [`64e93e5`
(#146212)](64e93e5806)
When I merged main into my PR it included a change which removed the
`@kbn/core-injected-metadata-browser` package. After resolving the
conflicts I missed a few tsconfig files which included references to the
now removed package. The TS Project Linter identified that these
references were removed from the code and pushed a change to the PR to
remove them from the tsconfig.json files.
## No bazel? Does that mean no packages??
Nope! We're still doing packages but we're pretty sure now that we won't
be using Bazel to accomplish the 'distributed caching' and 'change-based
tasks' portions of the packages project.
This PR actually makes packages much easier to work with and will be
followed up with the bundling benefits described by the original
packages RFC. Then we'll work on documentation and advocacy for using
packages for any and all new code.
We're pretty confident that implementing distributed caching and
change-based tasks will be necessary in the future, but because of
recent improvements in the repo we think we can live without them for
**at least** a year.
## Wait, there are still BUILD.bazel files in the repo
Yes, there are still three webpack bundles which are built by Bazel: the
`@kbn/ui-shared-deps-npm` DLL, `@kbn/ui-shared-deps-src` externals, and
the `@kbn/monaco` workers. These three webpack bundles are still created
during bootstrap and remotely cached using bazel. The next phase of this
project is to figure out how to get the package bundling features
described in the RFC with the current optimizer, and we expect these
bundles to go away then. Until then any package that is used in those
three bundles still needs to have a BUILD.bazel file so that they can be
referenced by the remaining webpack builds.
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This breaks out inline code in
`x-pack/plugins/aiops/server/routes/explain_log_rate_spikes.ts` to
functions in separate files and adds jest unit tests for each function.
The mocks used for jest unit tests are used as expected data in the API
integration tests. This allows to make sure that the static mocks are
still up to date should we have to update API integration tests based on
upstream tests.
For groups that have multiple values for the same field, the group
histogram query wasn't able to fetch data because it filters with a bool
`must` and individual `term` aggregations. This PR fixes it by using a
`terms` aggregation if there are multiple values for a field so just
these get treated as `OR`.
Where applicable, update imports to use `@kbn/field-types` package
instead of `@kbn/data-plugin/common` plugin. Should we move some of our
own code to packages, the import via the plugin wouldn't be allowed.
Fixes client side code to transform groups into table rows. Because the
transformation used a dictionary like structure with field names as
keys, we missed if there were multiple values for a field. This changes
the structure to an array of field/value pairs so we can support
multiple values per field.
The field candidate selection for Explain Log Rate Spikes was missing a
check if the supported field type is also aggregatable. For example, a
`keyword` type field could still be non-aggregatable if it was both not
indexed and `doc_values` set to `false`. Additionally, if no groups were
detected, we showed a "Try to continue analysis" button in the UI even
if the analysis was able to finish. In this PR the artificial logs dataset for functional tests was extended
to include a field like that.
The `date_histogram` query was updated to use `min_doc_count: 0` and set
`extended_bounds` to the date pickers time range so it returns buckets
for all necessary time slots necessary for the full date histogram bar
chart.
