From 535edadeaffc585a59db31297ed042c726fd811a Mon Sep 17 00:00:00 2001 From: Ersin Erdal <92688503+ersin-erdal@users.noreply.github.com> Date: Tue, 26 Sep 2023 14:17:37 +0200 Subject: [PATCH 1/6] Remove version from EventLog index name (#166820) Resolves: #158679 This PR removes the stack version from the EventLog index and datastream names. As we already use `kibana-event-log-*` as indexPattern, `kibana-event-log-ds` is used as DataStream name to avoid creating a breaking change. **Changes:** | Name | Old | New | | -------- | ------- | ------- | | Datastream | `kibana-event-log-` | `kibana-event-log-ds` | | IndexPattern | `kibana-event-log-*` | `kibana-event-log-*` (No change) | | IndexTemplate | `kibana-event-log--template` | `kibana-event-log-template` | Backing indices still have `-000001` suffix but i think this is expected. ## To verify: Run Kibana and ES in main with `-E path.data=../local-es-data` to save the data. Create a rule and let it run and create some alerts. See the alerts in the rule details page. Stop ES and Kibana Switch to this PR Run Kibana and ES again with `-E path.data=../local-es-data` See the all old and new alerts in the rule details page. The old index created by the main branch should remain, therefore the both old and the new indices (with and without version) should be visible in the console. --- x-pack/plugins/event_log/README.md | 2 +- .../server/es/cluster_client_adapter.test.ts | 12 ++++++------ x-pack/plugins/event_log/server/es/context.test.ts | 10 ++-------- x-pack/plugins/event_log/server/es/context.ts | 3 +-- x-pack/plugins/event_log/server/es/documents.test.ts | 3 +-- x-pack/plugins/event_log/server/es/names.test.ts | 7 +++---- x-pack/plugins/event_log/server/es/names.ts | 8 +++----- x-pack/plugins/event_log/server/plugin.ts | 1 - 8 files changed, 17 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/event_log/README.md b/x-pack/plugins/event_log/README.md index 35d80d2a2f7c3..ae2117fcd8ad4 100644 --- a/x-pack/plugins/event_log/README.md +++ b/x-pack/plugins/event_log/README.md @@ -44,7 +44,7 @@ prior releases, it was an alias with an initial index set up, with the alias used to deal with rolled over indices from ILM. With the data stream, there's a little less set up, and the bulk writing is slightly different. -The default data stream / alias name is `.kibana-event-log-${kibanaVersion}`. +The default data stream / alias name is `.kibana-event-log-ds`. To search across all versions' event logs, use `.kibana-event-log-*`; it will search over data streams and aliases as expected. diff --git a/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts b/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts index e51108c6e701d..a147d80f6639d 100644 --- a/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts +++ b/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts @@ -35,7 +35,7 @@ beforeEach(() => { clusterClientAdapter = new ClusterClientAdapter({ logger, elasticsearchClientPromise: Promise.resolve(clusterClient), - esNames: getEsNames('kibana', '1.2.3'), + esNames: getEsNames('kibana'), wait: () => Promise.resolve(true), }); }); @@ -50,7 +50,7 @@ describe('indexDocument', () => { expect(clusterClient.bulk).toHaveBeenCalledWith({ body: [{ create: {} }, { message: 'foo' }], - index: 'kibana-event-log-1.2.3', + index: 'kibana-event-log-ds', }); }); @@ -103,7 +103,7 @@ describe('buffering documents', () => { expect(clusterClient.bulk).toHaveBeenCalledWith({ body: expectedBody, - index: 'kibana-event-log-1.2.3', + index: 'kibana-event-log-ds', }); }); @@ -124,12 +124,12 @@ describe('buffering documents', () => { expect(clusterClient.bulk).toHaveBeenNthCalledWith(1, { body: expectedBody, - index: 'kibana-event-log-1.2.3', + index: 'kibana-event-log-ds', }); expect(clusterClient.bulk).toHaveBeenNthCalledWith(2, { body: [{ create: {} }, { message: `foo 100` }], - index: 'kibana-event-log-1.2.3', + index: 'kibana-event-log-ds', }); }); @@ -158,7 +158,7 @@ describe('buffering documents', () => { } expect(clusterClient.bulk).toHaveBeenNthCalledWith(i + 1, { - index: 'kibana-event-log-1.2.3', + index: 'kibana-event-log-ds', body: expectedBody, }); } diff --git a/x-pack/plugins/event_log/server/es/context.test.ts b/x-pack/plugins/event_log/server/es/context.test.ts index 5cb6887af3c9b..19789e0e8dc3c 100644 --- a/x-pack/plugins/event_log/server/es/context.test.ts +++ b/x-pack/plugins/event_log/server/es/context.test.ts @@ -36,7 +36,6 @@ describe('createEsContext', () => { logger, shouldSetExistingAssetsToHidden: true, indexNameRoot: 'test0', - kibanaVersion: '1.2.3', elasticsearchClientPromise: Promise.resolve(elasticsearchClient), }); @@ -51,16 +50,15 @@ describe('createEsContext', () => { logger, shouldSetExistingAssetsToHidden: true, indexNameRoot: 'test-index', - kibanaVersion: '1.2.3', elasticsearchClientPromise: Promise.resolve(elasticsearchClient), }); const esNames = context.esNames; expect(esNames).toStrictEqual({ base: 'test-index', - dataStream: 'test-index-event-log-1.2.3', + dataStream: 'test-index-event-log-ds', indexPattern: 'test-index-event-log-*', - indexTemplate: 'test-index-event-log-1.2.3-template', + indexTemplate: 'test-index-event-log-template', }); }); @@ -69,7 +67,6 @@ describe('createEsContext', () => { logger, shouldSetExistingAssetsToHidden: true, indexNameRoot: 'test1', - kibanaVersion: '1.2.3', elasticsearchClientPromise: Promise.resolve(elasticsearchClient), }); @@ -91,7 +88,6 @@ describe('createEsContext', () => { logger, shouldSetExistingAssetsToHidden: true, indexNameRoot: 'test2', - kibanaVersion: '1.2.3', elasticsearchClientPromise: Promise.resolve(elasticsearchClient), }); elasticsearchClient.indices.existsTemplate.mockResponse(true); @@ -123,7 +119,6 @@ describe('createEsContext', () => { logger, shouldSetExistingAssetsToHidden: true, indexNameRoot: 'test2', - kibanaVersion: '1.2.3', elasticsearchClientPromise: Promise.resolve(elasticsearchClient), }); expect(mockCreateReadySignal).toBeCalledTimes(1); @@ -141,7 +136,6 @@ describe('createEsContext', () => { logger, shouldSetExistingAssetsToHidden: true, indexNameRoot: 'test2', - kibanaVersion: '1.2.3', elasticsearchClientPromise: Promise.resolve(elasticsearchClient), }); context.initialize(); diff --git a/x-pack/plugins/event_log/server/es/context.ts b/x-pack/plugins/event_log/server/es/context.ts index 25d978416e354..7b0936ddf38e5 100644 --- a/x-pack/plugins/event_log/server/es/context.ts +++ b/x-pack/plugins/event_log/server/es/context.ts @@ -38,7 +38,6 @@ export function createEsContext(params: EsContextCtorParams): EsContext { export interface EsContextCtorParams { logger: Logger; indexNameRoot: string; - kibanaVersion: string; shouldSetExistingAssetsToHidden: boolean; elasticsearchClientPromise: Promise; } @@ -54,7 +53,7 @@ class EsContextImpl implements EsContext { constructor(params: EsContextCtorParams) { this.logger = params.logger; - this.esNames = getEsNames(params.indexNameRoot, params.kibanaVersion); + this.esNames = getEsNames(params.indexNameRoot); this.readySignal = createReadySignal(); this.initialized = false; this.retryDelay = RETRY_DELAY; diff --git a/x-pack/plugins/event_log/server/es/documents.test.ts b/x-pack/plugins/event_log/server/es/documents.test.ts index 71b75ee3ca3dc..ec79372420314 100644 --- a/x-pack/plugins/event_log/server/es/documents.test.ts +++ b/x-pack/plugins/event_log/server/es/documents.test.ts @@ -9,8 +9,7 @@ import { getIndexTemplate } from './documents'; import { getEsNames } from './names'; describe('getIndexTemplate()', () => { - const kibanaVersion = '1.2.3'; - const esNames = getEsNames('XYZ', kibanaVersion); + const esNames = getEsNames('XYZ'); test('returns the correct details of the index template', () => { const indexTemplate = getIndexTemplate(esNames); diff --git a/x-pack/plugins/event_log/server/es/names.test.ts b/x-pack/plugins/event_log/server/es/names.test.ts index 63d1ad9d398a7..a01fa5961e5ce 100644 --- a/x-pack/plugins/event_log/server/es/names.test.ts +++ b/x-pack/plugins/event_log/server/es/names.test.ts @@ -14,11 +14,10 @@ jest.mock('../../../../package.json', () => ({ describe('getEsNames()', () => { test('works as expected', () => { const base = 'XYZ'; - const kibanaVersion = '1.2.3'; - const esNames = getEsNames(base, kibanaVersion); + const esNames = getEsNames(base); expect(esNames.base).toEqual(base); - expect(esNames.dataStream).toEqual(`${base}-event-log-${kibanaVersion}`); + expect(esNames.dataStream).toEqual(`${base}-event-log-ds`); expect(esNames.indexPattern).toEqual(`${base}-event-log-*`); - expect(esNames.indexTemplate).toEqual(`${base}-event-log-${kibanaVersion}-template`); + expect(esNames.indexTemplate).toEqual(`${base}-event-log-template`); }); }); diff --git a/x-pack/plugins/event_log/server/es/names.ts b/x-pack/plugins/event_log/server/es/names.ts index 0e48ca911b95a..412bf0824244b 100644 --- a/x-pack/plugins/event_log/server/es/names.ts +++ b/x-pack/plugins/event_log/server/es/names.ts @@ -14,14 +14,12 @@ export interface EsNames { indexTemplate: string; } -export function getEsNames(baseName: string, kibanaVersion: string): EsNames { - const EVENT_LOG_VERSION_SUFFIX = `-${kibanaVersion.toLocaleLowerCase()}`; +export function getEsNames(baseName: string): EsNames { const eventLogName = `${baseName}${EVENT_LOG_NAME_SUFFIX}`; - const eventLogNameWithVersion = `${eventLogName}${EVENT_LOG_VERSION_SUFFIX}`; return { base: baseName, - dataStream: eventLogNameWithVersion, + dataStream: `${eventLogName}-ds`, indexPattern: `${eventLogName}-*`, - indexTemplate: `${eventLogNameWithVersion}-template`, + indexTemplate: `${eventLogName}-template`, }; } diff --git a/x-pack/plugins/event_log/server/plugin.ts b/x-pack/plugins/event_log/server/plugin.ts index 7bd9291ab4091..4b0dcbece34b4 100644 --- a/x-pack/plugins/event_log/server/plugin.ts +++ b/x-pack/plugins/event_log/server/plugin.ts @@ -71,7 +71,6 @@ export class Plugin implements CorePlugin elasticsearch.client.asInternalUser), - kibanaVersion: this.kibanaVersion, // Only non-serverless deployments may have assets that need to be converted shouldSetExistingAssetsToHidden: !plugins.serverless, }); From ce3310965170603ec692cd01cca9f9784e7d7bd0 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 26 Sep 2023 15:18:00 +0300 Subject: [PATCH 2/6] Fixes some types problems (#167228) ## Summary Fixes some types problems in our FTs which are blocking PRs with changes in the functional tests suites. (type diff check is failing) --- test/functional/apps/console/_autocomplete.ts | 3 ++- .../dashboard_elements/controls/common/replace_controls.ts | 2 +- test/functional/apps/visualize/group2/_inspector.ts | 1 - 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/apps/console/_autocomplete.ts b/test/functional/apps/console/_autocomplete.ts index 54e35bff86ee9..dd2e5a131efea 100644 --- a/test/functional/apps/console/_autocomplete.ts +++ b/test/functional/apps/console/_autocomplete.ts @@ -217,6 +217,7 @@ GET _search for (const keyPress of keyPresses) { await PageObjects.console.sleepForDebouncePeriod(); log.debug('Key', keyPress); + // @ts-ignore await PageObjects.console[keyPress](); expect(await PageObjects.console.isAutocompleteVisible()).to.be.eql(false); } @@ -257,7 +258,7 @@ GET _search for (const char of [method.at(-1), ' ', '_']) { await PageObjects.console.sleepForDebouncePeriod(); log.debug('Key type "%s"', char); - await PageObjects.console.enterText(char); // e.g. 'Post ' -> 'Post _' + await PageObjects.console.enterText(char ?? ''); // e.g. 'Post ' -> 'Post _' } await retry.waitFor('autocomplete to be visible', () => diff --git a/test/functional/apps/dashboard_elements/controls/common/replace_controls.ts b/test/functional/apps/dashboard_elements/controls/common/replace_controls.ts index 5c065f7f000f5..d3019a34b8802 100644 --- a/test/functional/apps/dashboard_elements/controls/common/replace_controls.ts +++ b/test/functional/apps/dashboard_elements/controls/common/replace_controls.ts @@ -16,7 +16,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const retry = getService('retry'); const security = getService('security'); - const { dashboardControls, timePicker, common, dashboard } = getPageObjects([ + const { dashboardControls, timePicker, dashboard } = getPageObjects([ 'dashboardControls', 'timePicker', 'dashboard', diff --git a/test/functional/apps/visualize/group2/_inspector.ts b/test/functional/apps/visualize/group2/_inspector.ts index 9baa3b1bc820d..077a37a90c06c 100644 --- a/test/functional/apps/visualize/group2/_inspector.ts +++ b/test/functional/apps/visualize/group2/_inspector.ts @@ -14,7 +14,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const log = getService('log'); const inspector = getService('inspector'); const filterBar = getService('filterBar'); - const monacoEditor = getService('monacoEditor'); const PageObjects = getPageObjects(['visualize', 'visEditor', 'visChart', 'timePicker']); describe('inspector', function describeIndexTests() { From 12f04e7a1014735df7cec50b5c4a653cfd1372e3 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Tue, 26 Sep 2023 14:29:31 +0200 Subject: [PATCH 3/6] Update ZDT update limitation to only bulkUpdate (#167200) ## Summary We've fixed the update limitation (https://github.com/elastic/kibana/issues/152807), so now it only applies to bulkUpdates https://github.com/elastic/kibana/issues/165434 ### 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) --- .../docs/model_versions.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md b/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md index 6e0e0a388c331..8241b361ba872 100644 --- a/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md +++ b/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md @@ -893,9 +893,11 @@ to the `fields` option **were already present in the prior model version**. Othe during upgrades, where newly introduced or backfilled fields may not necessarily appear in the documents returned from the `search` API when the option is used. -### Using `update` with dynamically backfilled fields +### Using `bulkUpdate` with dynamically backfilled fields -The savedObjects `update` API is effectively a partial update (using Elasticsearch's `_update` under the hood), +(Note: this same limitation used to exist for the `update` method but has been [fixed](https://github.com/elastic/kibana/issues/165434). So while they're similar this limitation is only relevant for the `bulkUpdate` method) + +The savedObjects `bulkUpdate` API is effectively a partial update (using Elasticsearch's `_update` under the hood), allowing API consumers to only specify the subset of fields they want to update to new values, without having to provide the full list of attributes (the unchanged ones). We're also not changing the `version` of the document during updates, even when the instance performing the operation doesn't know about the current model version @@ -935,8 +937,14 @@ const newDocAttributes = { Which could occur either while being still in the cohabitation period, or in case of rollback: ```ts -savedObjectClient.update('type', 'id', { - index: 11, +savedObjectClient.bulkUpdate({ + objects: [{ + type: 'type', + id: 'id', + attributes: { + index: 11 + } + }] }); ``` @@ -949,7 +957,7 @@ We will then be in a situation where our data is **inconsistent**, as the value } ``` -The long term solution for that is implementing [backward-compatible updates](https://github.com/elastic/kibana/issues/152807), however +The long term solution for that is implementing [backward-compatible updates](https://github.com/elastic/kibana/issues/165434), however this won't be done for the MVP, so the workaround for now is to avoid situations where this edge case can occur. It can be avoided by either: From 01a784194e60cb5f9fff5169e80af2b6c801662e Mon Sep 17 00:00:00 2001 From: Giorgos Bamparopoulos Date: Tue, 26 Sep 2023 15:35:24 +0300 Subject: [PATCH 4/6] [APM] Add an environment param to the service metadata details endpoint (#167173) Adds a query param for the environment to the `GET /internal/apm/services/{serviceName}/metadata/details` endpoint. ### Before https://github.com/elastic/kibana/assets/5831975/01865a3a-f312-4356-aafd-b21b88045487 ### After https://github.com/elastic/kibana/assets/5831975/e1f2fd00-4b44-4f86-8221-775ec6494913 Closes https://github.com/elastic/kibana/issues/167146 --- .../routing/templates/apm_service_template/index.tsx | 3 ++- .../templates/mobile_service_template/index.tsx | 3 ++- .../components/shared/service_icons/index.test.tsx | 9 +++++++++ .../public/components/shared/service_icons/index.tsx | 9 +++++---- .../shared/service_icons/service_icons.stories.tsx | 10 +++++++++- .../routes/services/get_service_metadata_details.ts | 4 ++++ x-pack/plugins/apm/server/routes/services/route.ts | 5 +++-- .../apm_api_integration/tests/feature_controls.spec.ts | 2 +- .../services/service_details/service_details.spec.ts | 1 + .../service_details/service_infra_metrics.spec.ts | 4 ++++ 10 files changed, 40 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/index.tsx b/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/index.tsx index 73c53dd5fda91..c797550c1617a 100644 --- a/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/index.tsx +++ b/x-pack/plugins/apm/public/components/routing/templates/apm_service_template/index.tsx @@ -90,7 +90,7 @@ function TemplateWithContext({ const { path: { serviceName }, query, - query: { rangeFrom, rangeTo }, + query: { rangeFrom, rangeTo, environment }, } = useApmParams('/services/{serviceName}/*'); const history = useHistory(); const location = useLocation(); @@ -140,6 +140,7 @@ function TemplateWithContext({ diff --git a/x-pack/plugins/apm/public/components/routing/templates/mobile_service_template/index.tsx b/x-pack/plugins/apm/public/components/routing/templates/mobile_service_template/index.tsx index 496010a14853a..cc841b200a5b5 100644 --- a/x-pack/plugins/apm/public/components/routing/templates/mobile_service_template/index.tsx +++ b/x-pack/plugins/apm/public/components/routing/templates/mobile_service_template/index.tsx @@ -57,7 +57,7 @@ function TemplateWithContext({ const { path: { serviceName }, query, - query: { rangeFrom, rangeTo }, + query: { rangeFrom, rangeTo, environment }, } = useApmParams('/mobile-services/{serviceName}/*'); const { start, end } = useTimeRange({ rangeFrom, rangeTo }); @@ -116,6 +116,7 @@ function TemplateWithContext({ diff --git a/x-pack/plugins/apm/public/components/shared/service_icons/index.test.tsx b/x-pack/plugins/apm/public/components/shared/service_icons/index.test.tsx index 5ac5f6326c8c9..cec958da0eb70 100644 --- a/x-pack/plugins/apm/public/components/shared/service_icons/index.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/service_icons/index.test.tsx @@ -64,6 +64,7 @@ describe('ServiceIcons', () => { @@ -87,6 +88,7 @@ describe('ServiceIcons', () => { @@ -112,6 +114,7 @@ describe('ServiceIcons', () => { @@ -138,6 +141,7 @@ describe('ServiceIcons', () => { @@ -165,6 +169,7 @@ describe('ServiceIcons', () => { @@ -212,6 +217,7 @@ describe('ServiceIcons', () => { @@ -256,6 +262,7 @@ describe('ServiceIcons', () => { @@ -308,6 +315,7 @@ describe('ServiceIcons', () => { @@ -366,6 +374,7 @@ describe('ServiceIcons', () => { diff --git a/x-pack/plugins/apm/public/components/shared/service_icons/index.tsx b/x-pack/plugins/apm/public/components/shared/service_icons/index.tsx index f5a647d3ca488..a77c6b9c7c604 100644 --- a/x-pack/plugins/apm/public/components/shared/service_icons/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/service_icons/index.tsx @@ -25,6 +25,7 @@ import openTelemetryIcon from '../agent_icon/icons/opentelemetry.svg'; interface Props { serviceName: string; + environment: string; start: string; end: string; } @@ -92,7 +93,7 @@ export interface PopoverItem { component: ReactChild; } -export function ServiceIcons({ start, end, serviceName }: Props) { +export function ServiceIcons({ start, end, serviceName, environment }: Props) { const [selectedIconPopover, setSelectedIconPopover] = useState(); @@ -117,20 +118,20 @@ export function ServiceIcons({ start, end, serviceName }: Props) { const { data: details, status: detailsFetchStatus } = useFetcher( (callApmApi) => { - if (selectedIconPopover && serviceName && start && end) { + if (selectedIconPopover && serviceName && start && end && environment) { return callApmApi( 'GET /internal/apm/services/{serviceName}/metadata/details', { isCachable: true, params: { path: { serviceName }, - query: { start, end }, + query: { start, end, environment }, }, } ); } }, - [selectedIconPopover, serviceName, start, end] + [selectedIconPopover, serviceName, start, end, environment] ); const isLoading = !icons && iconsFetchStatus === FETCH_STATUS.LOADING; diff --git a/x-pack/plugins/apm/public/components/shared/service_icons/service_icons.stories.tsx b/x-pack/plugins/apm/public/components/shared/service_icons/service_icons.stories.tsx index d5ea675338856..6a0fc2753d03e 100644 --- a/x-pack/plugins/apm/public/components/shared/service_icons/service_icons.stories.tsx +++ b/x-pack/plugins/apm/public/components/shared/service_icons/service_icons.stories.tsx @@ -22,6 +22,7 @@ type ServiceIconsReturnType = interface Args { serviceName: string; + environment: string; start: string; end: string; icons: ServiceIconsReturnType; @@ -64,7 +65,12 @@ const stories: Meta = { }; export default stories; -export const Example: Story = ({ serviceName, start, end }) => { +export const Example: Story = ({ + serviceName, + environment, + start, + end, +}) => { return ( @@ -83,6 +89,7 @@ export const Example: Story = ({ serviceName, start, end }) => { @@ -98,6 +105,7 @@ export const Example: Story = ({ serviceName, start, end }) => { }; Example.args = { serviceName: 'opbeans-java', + environment: 'dev', start: '2021-09-10T13:59:00.000Z', end: '2021-09-10T14:14:04.789Z', icons: { diff --git a/x-pack/plugins/apm/server/routes/services/get_service_metadata_details.ts b/x-pack/plugins/apm/server/routes/services/get_service_metadata_details.ts index 2231caa7df71e..4dd26b007ce57 100644 --- a/x-pack/plugins/apm/server/routes/services/get_service_metadata_details.ts +++ b/x-pack/plugins/apm/server/routes/services/get_service_metadata_details.ts @@ -7,6 +7,7 @@ import { rangeQuery } from '@kbn/observability-plugin/server'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; +import { environmentQuery } from '../../../common/utils/environment_query'; import { AGENT, CONTAINER, @@ -86,17 +87,20 @@ export interface ServiceMetadataDetails { export async function getServiceMetadataDetails({ serviceName, + environment, apmEventClient, start, end, }: { serviceName: string; + environment: string; apmEventClient: APMEventClient; start: number; end: number; }): Promise { const filter = [ { term: { [SERVICE_NAME]: serviceName } }, + ...environmentQuery(environment), ...rangeQuery(start, end), ]; diff --git a/x-pack/plugins/apm/server/routes/services/route.ts b/x-pack/plugins/apm/server/routes/services/route.ts index 970a72d478f72..52c04271bf31a 100644 --- a/x-pack/plugins/apm/server/routes/services/route.ts +++ b/x-pack/plugins/apm/server/routes/services/route.ts @@ -234,17 +234,18 @@ const serviceMetadataDetailsRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/services/{serviceName}/metadata/details', params: t.type({ path: t.type({ serviceName: t.string }), - query: rangeRt, + query: t.intersection([rangeRt, environmentRt]), }), options: { tags: ['access:apm'] }, handler: async (resources): Promise => { const apmEventClient = await getApmEventClient(resources); const { params } = resources; const { serviceName } = params.path; - const { start, end } = params.query; + const { start, end, environment } = params.query; const serviceMetadataDetails = await getServiceMetadataDetails({ serviceName, + environment, apmEventClient, start, end, diff --git a/x-pack/test/apm_api_integration/tests/feature_controls.spec.ts b/x-pack/test/apm_api_integration/tests/feature_controls.spec.ts index d439663e8d0d8..fb908f7bbe9c8 100644 --- a/x-pack/test/apm_api_integration/tests/feature_controls.spec.ts +++ b/x-pack/test/apm_api_integration/tests/feature_controls.spec.ts @@ -184,7 +184,7 @@ export default function featureControlsTests({ getService }: FtrProviderContext) }, { req: { - url: `/internal/apm/services/foo/metadata/details?start=${start}&end=${end}`, + url: `/internal/apm/services/foo/metadata/details?start=${start}&end=${end}&environment=dev`, }, expectForbidden: expect403, expectResponse: expect200, diff --git a/x-pack/test/apm_api_integration/tests/services/service_details/service_details.spec.ts b/x-pack/test/apm_api_integration/tests/services/service_details/service_details.spec.ts index 1933d3552e611..f32c7bcb4083e 100644 --- a/x-pack/test/apm_api_integration/tests/services/service_details/service_details.spec.ts +++ b/x-pack/test/apm_api_integration/tests/services/service_details/service_details.spec.ts @@ -32,6 +32,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { query: { start: new Date(start).toISOString(), end: new Date(end).toISOString(), + environment: 'production', }, }, }); diff --git a/x-pack/test/apm_api_integration/tests/services/service_details/service_infra_metrics.spec.ts b/x-pack/test/apm_api_integration/tests/services/service_details/service_infra_metrics.spec.ts index 0a5d2f88453a6..194b266e927e8 100644 --- a/x-pack/test/apm_api_integration/tests/services/service_details/service_infra_metrics.spec.ts +++ b/x-pack/test/apm_api_integration/tests/services/service_details/service_infra_metrics.spec.ts @@ -6,6 +6,7 @@ */ import expect from '@kbn/expect'; import { APIReturnType } from '@kbn/apm-plugin/public/services/rest/create_call_apm_api'; +import { ENVIRONMENT_ALL } from '@kbn/apm-plugin/common/environment_filter_values'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; import archives_metadata from '../../../common/fixtures/es_archiver/archives_metadata'; @@ -94,6 +95,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { query: { start, end, + environment: ENVIRONMENT_ALL.value, }, }, }); @@ -124,6 +126,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { query: { start, end, + environment: ENVIRONMENT_ALL.value, }, }, }); @@ -151,6 +154,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { query: { start, end, + environment: ENVIRONMENT_ALL.value, }, }, }); From 591df706da23663c898247f23faa00c015c2d26c Mon Sep 17 00:00:00 2001 From: Jill Guyonnet Date: Tue, 26 Sep 2023 15:15:08 +0200 Subject: [PATCH 5/6] [Fleet] Fix bulk action dropdown (#166475) Closes https://github.com/elastic/kibana/issues/164083 Related to https://github.com/elastic/sdh-beats/issues/3759 Related to https://github.com/elastic/kibana/issues/157844 This PR addresses two current issues affecting agent selection in Fleet UI: 1. When there are inactive agents that are not listed on the current page and "Select everything on all pages" is clicked, the count of actionable agents is incorrect (cf. [this comment](https://github.com/elastic/kibana/issues/164083#issuecomment-1711780591) for details). This can have two consequences: 1. Incorrect and sometimes negative agent count in the "Actions" dropdown. 2. Disabled menu items in the "Actions" dropdown. 2. The "Select everything on all pages button is incorrectly displayed when there are agents on managed policies on the page and there is no pagination (cf. [this comment](https://github.com/elastic/kibana/issues/164083#issuecomment-1711781808)). --- .../agents_selection_status.test.tsx | 100 +++++++ .../components/agents_selection_status.tsx | 20 +- .../components/bulk_actions.test.tsx | 283 +++--------------- .../components/bulk_actions.tsx | 85 ++---- .../components/search_and_filter_bar.test.tsx | 12 +- .../components/search_and_filter_bar.tsx | 15 +- .../components/table_header.tsx | 3 + .../agents/agent_list_page/index.test.tsx | 1 + .../sections/agents/agent_list_page/index.tsx | 121 +++++--- 9 files changed, 286 insertions(+), 354 deletions(-) create mode 100644 x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.test.tsx diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.test.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.test.tsx new file mode 100644 index 0000000000000..07f42b65fdb0c --- /dev/null +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.test.tsx @@ -0,0 +1,100 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { createFleetTestRendererMock } from '../../../../../../mock'; + +import { AgentsSelectionStatus } from './agents_selection_status'; + +function render(props: any) { + const renderer = createFleetTestRendererMock(); + + return renderer.render(); +} + +const defaultProps = { + totalAgents: 30, + selectableAgents: 20, + managedAgentsOnCurrentPage: 0, + selectionMode: 'manual', + setSelectionMode: jest.fn(), + selectedAgents: [], + setSelectedAgents: jest.fn(), +}; + +function generateAgents(n: number) { + return [...Array(n).keys()].map((i) => ({ + id: `agent${i}`, + active: true, + })); +} + +describe('AgentsSelectionStatus', () => { + describe('when selection mode is manual', () => { + describe('when there are no selected agents', () => { + it('should not show any selection options', () => { + const res = render(defaultProps); + expect(res.queryByTestId('selectedAgentCountLabel')).toBeNull(); + expect(res.queryByTestId('clearAgentSelectionButton')).toBeNull(); + expect(res.queryByTestId('selectedEverythingOnAllPagesButton')).toBeNull(); + }); + }); + + describe('when there are selected agents', () => { + it('should show the number of selected agents and the Clear selection button', () => { + const res = render({ ...defaultProps, selectedAgents: generateAgents(2) }); + expect(res.queryByTestId('selectedAgentCountLabel')).not.toBeNull(); + expect(res.queryByTestId('clearAgentSelectionButton')).not.toBeNull(); + }); + + it('should not show the Select everything on all pages button if not all agents are selected', () => { + const res = render({ ...defaultProps, selectedAgents: generateAgents(2) }); + expect(res.queryByTestId('selectedEverythingOnAllPagesButton')).toBeNull(); + }); + + it('should not show the Select everything on all pages button if all agents are selected but there are no more selectable agents', () => { + const res = render({ + ...defaultProps, + totalAgents: 20, + selectableAgents: 19, + managedAgentsOnCurrentPage: 1, + selectedAgents: generateAgents(19), + }); + expect(res.queryByTestId('selectedEverythingOnAllPagesButton')).toBeNull(); + }); + + it('should show the Select everything on all pages button if all agents are selected and there are more selectable agents', () => { + const res = render({ ...defaultProps, selectedAgents: generateAgents(20) }); + expect(res.queryByTestId('selectedEverythingOnAllPagesButton')).not.toBeNull(); + }); + }); + }); + + describe('when selection mode is query', () => { + describe('when there are agents', () => { + it('should show the number of selected agents and the Clear selection button', () => { + const res = render({ + ...defaultProps, + selectionMode: 'query', + selectedAgents: generateAgents(2), + }); + expect(res.queryByTestId('selectedAgentCountLabel')).not.toBeNull(); + expect(res.queryByTestId('clearAgentSelectionButton')).not.toBeNull(); + }); + + it('should not show the Select everything on all pages button', () => { + const res = render({ + ...defaultProps, + selectionMode: 'query', + selectedAgents: generateAgents(20), + }); + expect(res.queryByTestId('selectedEverythingOnAllPagesButton')).toBeNull(); + }); + }); + }); +}); diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.tsx index 57728f275ccb5..0e1b6e43db46e 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agents_selection_status.tsx @@ -34,6 +34,7 @@ const Button = styled(EuiButtonEmpty)` export const AgentsSelectionStatus: React.FunctionComponent<{ totalAgents: number; selectableAgents: number; + managedAgentsOnCurrentPage: number; selectionMode: SelectionMode; setSelectionMode: (mode: SelectionMode) => void; selectedAgents: Agent[]; @@ -41,15 +42,19 @@ export const AgentsSelectionStatus: React.FunctionComponent<{ }> = ({ totalAgents, selectableAgents, + managedAgentsOnCurrentPage, selectionMode, setSelectionMode, selectedAgents, setSelectedAgents, }) => { + const showSelectionInfoAndOptions = + (selectionMode === 'manual' && selectedAgents.length > 0) || + (selectionMode === 'query' && totalAgents > 0); const showSelectEverything = selectionMode === 'manual' && selectedAgents.length === selectableAgents && - selectableAgents < totalAgents; + selectableAgents < totalAgents - managedAgentsOnCurrentPage; return ( <> @@ -74,14 +79,13 @@ export const AgentsSelectionStatus: React.FunctionComponent<{ )} - {(selectionMode === 'manual' && selectedAgents.length) || - (selectionMode === 'query' && totalAgents > 0) ? ( + {showSelectionInfoAndOptions ? ( <> - + -