From 2d2d8cf603b18afc32e16c0b805fc96f4f96e19c Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 22 Jul 2024 13:49:12 +0200 Subject: [PATCH 1/3] [ES|QL] Max/Min support IP fields (#188808) ## Summary Follow up of https://github.com/elastic/elasticsearch/pull/110921 ES|QL max and min aggregations support now IP fields. ### Checklist - [ ] [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 --- .../autocomplete.command.stats.test.ts | 4 +- .../src/definitions/aggs.ts | 8 ++ .../esql_validation_meta_tests.json | 104 ++++++++++++++++++ .../src/validation/validation.test.ts | 52 +++++++++ 4 files changed, 166 insertions(+), 2 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts index 2d81fe794193b..a37756ecd0866 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts @@ -116,8 +116,8 @@ describe('autocomplete.suggest', () => { test('when typing inside function left paren', async () => { const { assertSuggestions } = await setup(); const expected = [ - ...getFieldNamesByType(['number', 'date', 'boolean']), - ...getFunctionSignaturesByReturnType('stats', ['number', 'date', 'boolean'], { + ...getFieldNamesByType(['number', 'date', 'boolean', 'ip']), + ...getFunctionSignaturesByReturnType('stats', ['number', 'date', 'boolean', 'ip'], { evalMath: true, }), ]; diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts index 69f8f76807daf..a27b8a68a9be0 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts @@ -112,6 +112,10 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [ params: [{ name: 'column', type: 'boolean', noNestingFunctions: true }], returnType: 'boolean', }, + { + params: [{ name: 'column', type: 'ip', noNestingFunctions: true }], + returnType: 'ip', + }, ], examples: [`from index | stats result = max(field)`, `from index | stats max(field)`], }, @@ -135,6 +139,10 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [ params: [{ name: 'column', type: 'boolean', noNestingFunctions: true }], returnType: 'boolean', }, + { + params: [{ name: 'column', type: 'ip', noNestingFunctions: true }], + returnType: 'ip', + }, ], examples: [`from index | stats result = min(field)`, `from index | stats min(field)`], }, diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json index a2cc5bf55ff35..65dc30e79064b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json +++ b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json @@ -24201,6 +24201,58 @@ ], "warning": [] }, + { + "query": "from a_index | stats var = max(ipField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | stats max(ipField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where max(ipField)", + "error": [ + "WHERE does not support function max" + ], + "warning": [] + }, + { + "query": "from a_index | where max(ipField) > 0", + "error": [ + "WHERE does not support function max" + ], + "warning": [] + }, + { + "query": "from a_index | eval var = max(ipField)", + "error": [ + "EVAL does not support function max" + ], + "warning": [] + }, + { + "query": "from a_index | eval var = max(ipField) > 0", + "error": [ + "EVAL does not support function max" + ], + "warning": [] + }, + { + "query": "from a_index | eval max(ipField)", + "error": [ + "EVAL does not support function max" + ], + "warning": [] + }, + { + "query": "from a_index | eval max(ipField) > 0", + "error": [ + "EVAL does not support function max" + ], + "warning": [] + }, { "query": "from a_index | stats var = min(numberField)", "error": [], @@ -24526,6 +24578,58 @@ ], "warning": [] }, + { + "query": "from a_index | stats var = min(ipField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | stats min(ipField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where min(ipField)", + "error": [ + "WHERE does not support function min" + ], + "warning": [] + }, + { + "query": "from a_index | where min(ipField) > 0", + "error": [ + "WHERE does not support function min" + ], + "warning": [] + }, + { + "query": "from a_index | eval var = min(ipField)", + "error": [ + "EVAL does not support function min" + ], + "warning": [] + }, + { + "query": "from a_index | eval var = min(ipField) > 0", + "error": [ + "EVAL does not support function min" + ], + "warning": [] + }, + { + "query": "from a_index | eval min(ipField)", + "error": [ + "EVAL does not support function min" + ], + "warning": [] + }, + { + "query": "from a_index | eval min(ipField) > 0", + "error": [ + "EVAL does not support function min" + ], + "warning": [] + }, { "query": "from a_index | stats var = count(stringField)", "error": [], diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts index afbd3db5458bb..e9a707fd85e98 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts @@ -9202,6 +9202,32 @@ describe('validation logic', () => { testErrorsAndWarnings('from a_index | eval max(booleanField) > 0', [ 'EVAL does not support function max', ]); + testErrorsAndWarnings('from a_index | stats var = max(ipField)', []); + testErrorsAndWarnings('from a_index | stats max(ipField)', []); + + testErrorsAndWarnings('from a_index | where max(ipField)', [ + 'WHERE does not support function max', + ]); + + testErrorsAndWarnings('from a_index | where max(ipField) > 0', [ + 'WHERE does not support function max', + ]); + + testErrorsAndWarnings('from a_index | eval var = max(ipField)', [ + 'EVAL does not support function max', + ]); + + testErrorsAndWarnings('from a_index | eval var = max(ipField) > 0', [ + 'EVAL does not support function max', + ]); + + testErrorsAndWarnings('from a_index | eval max(ipField)', [ + 'EVAL does not support function max', + ]); + + testErrorsAndWarnings('from a_index | eval max(ipField) > 0', [ + 'EVAL does not support function max', + ]); }); describe('min', () => { @@ -9374,6 +9400,32 @@ describe('validation logic', () => { testErrorsAndWarnings('from a_index | eval min(booleanField) > 0', [ 'EVAL does not support function min', ]); + testErrorsAndWarnings('from a_index | stats var = min(ipField)', []); + testErrorsAndWarnings('from a_index | stats min(ipField)', []); + + testErrorsAndWarnings('from a_index | where min(ipField)', [ + 'WHERE does not support function min', + ]); + + testErrorsAndWarnings('from a_index | where min(ipField) > 0', [ + 'WHERE does not support function min', + ]); + + testErrorsAndWarnings('from a_index | eval var = min(ipField)', [ + 'EVAL does not support function min', + ]); + + testErrorsAndWarnings('from a_index | eval var = min(ipField) > 0', [ + 'EVAL does not support function min', + ]); + + testErrorsAndWarnings('from a_index | eval min(ipField)', [ + 'EVAL does not support function min', + ]); + + testErrorsAndWarnings('from a_index | eval min(ipField) > 0', [ + 'EVAL does not support function min', + ]); }); describe('count', () => { From e5638db74e1c2ceb43fa7124298286936abdbc5a Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Mon, 22 Jul 2024 14:02:12 +0200 Subject: [PATCH 2/3] [core rendering] get rid of `getInjectedVar` (#188755) ## Summary Fix https://github.com/elastic/kibana/issues/54376 Fix https://github.com/elastic/kibana/issues/127733 - get rid of the `InjectedMetadata.vars` and `getInjectedVar` deprecated "API" - Add `apmConfig` as an explicit `InjectedMetadata` property instead of passing it via `vars` - Inject the apm config from the `rendering` service instead of `httpResource`, as it's just how it should be and how all other things get injected. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../src/http_resources_service.test.mocks.ts | 13 ---- .../src/http_resources_service.test.ts | 14 ---- .../src/http_resources_service.ts | 10 --- .../tsconfig.json | 1 - .../src/injected_metadata_service.test.ts | 76 ------------------- .../src/injected_metadata_service.ts | 9 --- .../src/types.ts | 4 - .../src/injected_metadata_service.mock.ts | 2 - .../src/types.ts | 2 +- .../rendering_service.test.ts.snap | 64 ++++++++++++---- .../src/get_apm_config.test.mocks.ts | 0 .../src/get_apm_config.test.ts | 0 .../src/get_apm_config.ts | 0 .../src/rendering_service.test.mocks.ts | 7 ++ .../src/rendering_service.test.ts | 21 +++++ .../src/rendering_service.tsx | 6 +- .../src/types.ts | 7 -- .../tsconfig.json | 1 + .../src/kbn_bootstrap.ts | 8 +- .../core-root-browser-internal/tsconfig.json | 1 + 20 files changed, 89 insertions(+), 157 deletions(-) delete mode 100644 packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.mocks.ts rename packages/core/{http/core-http-resources-server-internal => rendering/core-rendering-server-internal}/src/get_apm_config.test.mocks.ts (100%) rename packages/core/{http/core-http-resources-server-internal => rendering/core-rendering-server-internal}/src/get_apm_config.test.ts (100%) rename packages/core/{http/core-http-resources-server-internal => rendering/core-rendering-server-internal}/src/get_apm_config.ts (100%) diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.mocks.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.mocks.ts deleted file mode 100644 index 0d0c637a768df..0000000000000 --- a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.mocks.ts +++ /dev/null @@ -1,13 +0,0 @@ -/* - * 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 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -export const getApmConfigMock = jest.fn(); - -jest.doMock('./get_apm_config', () => ({ - getApmConfig: getApmConfigMock, -})); diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts index 481b0b694747a..acf8950d0d147 100644 --- a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts +++ b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts @@ -6,10 +6,7 @@ * Side Public License, v 1. */ -import { getApmConfigMock } from './http_resources_service.test.mocks'; - import type { RouteConfig } from '@kbn/core-http-server'; - import { mockCoreContext } from '@kbn/core-base-server-mocks'; import { httpServiceMock, httpServerMock } from '@kbn/core-http-server-mocks'; import { renderingServiceMock } from '@kbn/core-rendering-server-mocks'; @@ -29,11 +26,6 @@ describe('HttpResources service', () => { let router: ReturnType; const kibanaRequest = httpServerMock.createKibanaRequest(); const context = createCoreRequestHandlerContextMock(); - const apmConfig = { mockApmConfig: true }; - - beforeEach(() => { - getApmConfigMock.mockReturnValue(apmConfig); - }); describe('#createRegistrar', () => { beforeEach(() => { @@ -93,9 +85,6 @@ describe('HttpResources service', () => { }, { isAnonymousPage: false, - vars: { - apmConfig, - }, } ); }); @@ -118,9 +107,6 @@ describe('HttpResources service', () => { }, { isAnonymousPage: true, - vars: { - apmConfig, - }, } ); }); diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts index a896d6b98542f..bb9320bc5cb48 100644 --- a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts +++ b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts @@ -33,8 +33,6 @@ import type { import type { InternalHttpResourcesSetup } from './types'; -import { getApmConfig } from './get_apm_config'; - /** * @internal */ @@ -112,13 +110,9 @@ export class HttpResourcesService implements CoreService { }).toThrowError(); }); }); - -describe('setup.getInjectedVar()', () => { - it('returns values from injectedMetadata.vars', () => { - const setup = new InjectedMetadataService({ - injectedMetadata: { - vars: { - foo: { - bar: '1', - }, - 'baz:box': { - foo: 2, - }, - }, - }, - } as any).setup(); - - expect(setup.getInjectedVar('foo')).toEqual({ - bar: '1', - }); - expect(setup.getInjectedVar('foo.bar')).toBe('1'); - expect(setup.getInjectedVar('baz:box')).toEqual({ - foo: 2, - }); - expect(setup.getInjectedVar('')).toBe(undefined); - }); - - it('returns read-only values', () => { - const setup = new InjectedMetadataService({ - injectedMetadata: { - vars: { - foo: { - bar: 1, - }, - }, - }, - } as any).setup(); - - const foo: any = setup.getInjectedVar('foo'); - expect(() => { - foo.bar = 2; - }).toThrowErrorMatchingInlineSnapshot( - `"Cannot assign to read only property 'bar' of object '#'"` - ); - expect(() => { - foo.newProp = 2; - }).toThrowErrorMatchingInlineSnapshot( - `"Cannot add property newProp, object is not extensible"` - ); - }); -}); - -describe('setup.getInjectedVars()', () => { - it('returns all injected vars, readonly', () => { - const setup = new InjectedMetadataService({ - injectedMetadata: { - vars: { - foo: { - bar: 1, - }, - }, - }, - } as any).setup(); - - const vars: any = setup.getInjectedVars(); - expect(() => { - vars.foo = 2; - }).toThrowErrorMatchingInlineSnapshot( - `"Cannot assign to read only property 'foo' of object '#'"` - ); - expect(() => { - vars.newProp = 2; - }).toThrowErrorMatchingInlineSnapshot( - `"Cannot add property newProp, object is not extensible"` - ); - }); -}); diff --git a/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/injected_metadata_service.ts b/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/injected_metadata_service.ts index a50de7ca3f5e7..104500ef19215 100644 --- a/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/injected_metadata_service.ts +++ b/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/injected_metadata_service.ts @@ -6,7 +6,6 @@ * Side Public License, v 1. */ -import { get } from 'lodash'; import { deepFreeze } from '@kbn/std'; import type { InjectedMetadata } from '@kbn/core-injected-metadata-common-internal'; import type { @@ -76,14 +75,6 @@ export class InjectedMetadataService { return this.state.legacyMetadata; }, - getInjectedVar: (name: string, defaultValue?: any): unknown => { - return get(this.state.vars, name, defaultValue); - }, - - getInjectedVars: () => { - return this.state.vars; - }, - getKibanaBuildNumber: () => { return this.state.buildNumber; }, diff --git a/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/types.ts b/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/types.ts index bf77a2531660a..12bee868702b6 100644 --- a/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/types.ts +++ b/packages/core/injected-metadata/core-injected-metadata-browser-internal/src/types.ts @@ -56,10 +56,6 @@ export interface InternalInjectedMetadataSetup { user?: Record | undefined; }; }; - getInjectedVar: (name: string, defaultValue?: any) => unknown; - getInjectedVars: () => { - [key: string]: unknown; - }; getCustomBranding: () => CustomBranding; } diff --git a/packages/core/injected-metadata/core-injected-metadata-browser-mocks/src/injected_metadata_service.mock.ts b/packages/core/injected-metadata/core-injected-metadata-browser-mocks/src/injected_metadata_service.mock.ts index 69148ca90e31b..e2dad19650a2c 100644 --- a/packages/core/injected-metadata/core-injected-metadata-browser-mocks/src/injected_metadata_service.mock.ts +++ b/packages/core/injected-metadata/core-injected-metadata-browser-mocks/src/injected_metadata_service.mock.ts @@ -27,8 +27,6 @@ const createSetupContractMock = () => { getLegacyMetadata: jest.fn(), getTheme: jest.fn(), getPlugins: jest.fn(), - getInjectedVar: jest.fn(), - getInjectedVars: jest.fn(), getKibanaBuildNumber: jest.fn(), getCustomBranding: jest.fn(), }; diff --git a/packages/core/injected-metadata/core-injected-metadata-common-internal/src/types.ts b/packages/core/injected-metadata/core-injected-metadata-common-internal/src/types.ts index 6d4e3df08a5ef..c2f1e85e1e60d 100644 --- a/packages/core/injected-metadata/core-injected-metadata-common-internal/src/types.ts +++ b/packages/core/injected-metadata/core-injected-metadata-common-internal/src/types.ts @@ -71,7 +71,7 @@ export interface InjectedMetadata { warnLegacyBrowsers: boolean; }; externalUrl: { policy: InjectedMetadataExternalUrlPolicy[] }; - vars: Record; + apmConfig: Record | null; uiPlugins: InjectedMetadataPlugin[]; legacyMetadata: { uiSettings: { diff --git a/packages/core/rendering/core-rendering-server-internal/src/__snapshots__/rendering_service.test.ts.snap b/packages/core/rendering/core-rendering-server-internal/src/__snapshots__/rendering_service.test.ts.snap index 69f534cf837b4..e92e760b400e5 100644 --- a/packages/core/rendering/core-rendering-server-internal/src/__snapshots__/rendering_service.test.ts.snap +++ b/packages/core/rendering/core-rendering-server-internal/src/__snapshots__/rendering_service.test.ts.snap @@ -3,6 +3,9 @@ exports[`RenderingService preboot() render() renders "core" CDN url injected 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "http://foo.bar:1773", "basePath": "/mock-server-basepath", "branch": Any, @@ -75,7 +78,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -83,6 +85,9 @@ Object { exports[`RenderingService preboot() render() renders "core" page 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -151,7 +156,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -159,6 +163,9 @@ Object { exports[`RenderingService preboot() render() renders "core" page driven by settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -231,7 +238,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -239,6 +245,9 @@ Object { exports[`RenderingService preboot() render() renders "core" page for blank basepath 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "", "branch": Any, @@ -307,7 +316,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -315,6 +323,9 @@ Object { exports[`RenderingService preboot() render() renders "core" page for unauthenticated requests 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -383,7 +394,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -391,6 +401,9 @@ Object { exports[`RenderingService preboot() render() renders "core" page with global settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -463,7 +476,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -471,6 +483,9 @@ Object { exports[`RenderingService preboot() render() renders "core" with excluded global user settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -539,7 +554,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -547,6 +561,9 @@ Object { exports[`RenderingService preboot() render() renders "core" with excluded user settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -615,7 +632,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -623,6 +639,9 @@ Object { exports[`RenderingService setup() render() renders "core" CDN url injected 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -700,7 +719,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -708,6 +726,9 @@ Object { exports[`RenderingService setup() render() renders "core" page 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -776,7 +797,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -784,6 +804,9 @@ Object { exports[`RenderingService setup() render() renders "core" page driven by settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -861,7 +884,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -869,6 +891,9 @@ Object { exports[`RenderingService setup() render() renders "core" page for blank basepath 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "", "branch": Any, @@ -942,7 +967,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -950,6 +974,9 @@ Object { exports[`RenderingService setup() render() renders "core" page for unauthenticated requests 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -1018,7 +1045,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -1026,6 +1052,9 @@ Object { exports[`RenderingService setup() render() renders "core" page with global settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -1103,7 +1132,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -1111,6 +1139,9 @@ Object { exports[`RenderingService setup() render() renders "core" with excluded global user settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -1184,7 +1215,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; @@ -1192,6 +1222,9 @@ Object { exports[`RenderingService setup() render() renders "core" with excluded user settings 1`] = ` Object { "anonymousStatusPage": false, + "apmConfig": Object { + "stubApmConfig": true, + }, "assetsHrefBase": "/mock-server-basepath", "basePath": "/mock-server-basepath", "branch": Any, @@ -1265,7 +1298,6 @@ Object { "version": "v8", }, "uiPlugins": Array [], - "vars": Object {}, "version": Any, } `; diff --git a/packages/core/http/core-http-resources-server-internal/src/get_apm_config.test.mocks.ts b/packages/core/rendering/core-rendering-server-internal/src/get_apm_config.test.mocks.ts similarity index 100% rename from packages/core/http/core-http-resources-server-internal/src/get_apm_config.test.mocks.ts rename to packages/core/rendering/core-rendering-server-internal/src/get_apm_config.test.mocks.ts diff --git a/packages/core/http/core-http-resources-server-internal/src/get_apm_config.test.ts b/packages/core/rendering/core-rendering-server-internal/src/get_apm_config.test.ts similarity index 100% rename from packages/core/http/core-http-resources-server-internal/src/get_apm_config.test.ts rename to packages/core/rendering/core-rendering-server-internal/src/get_apm_config.test.ts diff --git a/packages/core/http/core-http-resources-server-internal/src/get_apm_config.ts b/packages/core/rendering/core-rendering-server-internal/src/get_apm_config.ts similarity index 100% rename from packages/core/http/core-http-resources-server-internal/src/get_apm_config.ts rename to packages/core/rendering/core-rendering-server-internal/src/get_apm_config.ts diff --git a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.mocks.ts b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.mocks.ts index e7d95312ed56a..96807a64b9560 100644 --- a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.mocks.ts +++ b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.mocks.ts @@ -28,3 +28,10 @@ jest.doMock('./render_utils', () => ({ getScriptPaths: getScriptPathsMock, getBrowserLoggingConfig: getBrowserLoggingConfigMock, })); + +export const getApmConfigMock = jest.fn(); +jest.doMock('./get_apm_config', () => { + return { + getApmConfig: getApmConfigMock, + }; +}); diff --git a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.ts b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.ts index b07b8a1cd6fa1..9adf0a0ea3d69 100644 --- a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.ts +++ b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.ts @@ -14,6 +14,7 @@ import { getThemeStylesheetPathsMock, getScriptPathsMock, getBrowserLoggingConfigMock, + getApmConfigMock, } from './rendering_service.test.mocks'; import { load } from 'cheerio'; @@ -259,6 +260,25 @@ function renderTestCases( expect(data.logging).toEqual(loggingConfig); }); + it('renders "core" with APM config injected', async () => { + const someApmConfig = { someConfig: 9000 }; + getApmConfigMock.mockReturnValue(someApmConfig); + + const request = createKibanaRequest(); + + const [render] = await getRender(); + const content = await render(createKibanaRequest(), uiSettings, { + isAnonymousPage: false, + }); + + expect(getApmConfigMock).toHaveBeenCalledTimes(1); + expect(getApmConfigMock).toHaveBeenCalledWith(request.url.pathname); + + const dom = load(content); + const data = JSON.parse(dom('kbn-injected-metadata').attr('data') ?? '""'); + expect(data.apmConfig).toEqual(someApmConfig); + }); + it('use the correct translation url when CDN is enabled', async () => { const userSettings = { 'theme:darkMode': { userValue: true } }; uiSettings.client.getUserProvided.mockResolvedValue(userSettings); @@ -511,6 +531,7 @@ describe('RenderingService', () => { getThemeStylesheetPathsMock.mockReturnValue(['/style-1.css', '/style-2.css']); getScriptPathsMock.mockReturnValue(['/script-1.js']); getBrowserLoggingConfigMock.mockReset().mockReturnValue({}); + getApmConfigMock.mockReset().mockReturnValue({ stubApmConfig: true }); }); describe('preboot()', () => { diff --git a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx index 7a7e6e56fb49f..4b7e75ea9fb84 100644 --- a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx +++ b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx @@ -42,6 +42,7 @@ import { getBrowserLoggingConfig, } from './render_utils'; import { filterUiPlugins } from './filter_ui_plugins'; +import { getApmConfig } from './get_apm_config'; import type { InternalRenderingRequestHandlerContext } from './internal_types'; type RenderOptions = @@ -121,7 +122,7 @@ export class RenderingService { client: IUiSettingsClient; globalClient: IUiSettingsClient; }, - { isAnonymousPage = false, vars, includeExposedConfigKeys }: IRenderOptions = {} + { isAnonymousPage = false, includeExposedConfigKeys }: IRenderOptions = {} ) { const { elasticsearch, http, uiPlugins, status, customBranding, userSettings, i18n } = renderOptions; @@ -221,6 +222,7 @@ export class RenderingService { translationsUrl = `${serverBasePath}/translations/${translationHash}/${locale}.json`; } + const apmConfig = getApmConfig(request.url.pathname); const filteredPlugins = filterUiPlugins({ uiPlugins, isAnonymousPage }); const bootstrapScript = isAnonymousPage ? 'bootstrap-anonymous.js' : 'bootstrap.js'; const metadata: RenderingMetadata = { @@ -249,6 +251,7 @@ export class RenderingService { logging: loggingConfig, env, clusterInfo, + apmConfig, anonymousStatusPage: status?.isStatusPageAnonymous() ?? false, i18n: { translationsUrl, @@ -268,7 +271,6 @@ export class RenderingService { }, csp: { warnLegacyBrowsers: http.csp.warnLegacyBrowsers }, externalUrl: http.externalUrl, - vars: vars ?? {}, uiPlugins: await Promise.all( filteredPlugins.map(async ([id, plugin]) => { const { browserConfig, exposedConfigKeys } = await getUiConfig(uiPlugins, id); diff --git a/packages/core/rendering/core-rendering-server-internal/src/types.ts b/packages/core/rendering/core-rendering-server-internal/src/types.ts index ed6a45d071423..98887a9f9da29 100644 --- a/packages/core/rendering/core-rendering-server-internal/src/types.ts +++ b/packages/core/rendering/core-rendering-server-internal/src/types.ts @@ -64,13 +64,6 @@ export interface IRenderOptions { */ isAnonymousPage?: boolean; - /** - * Inject custom vars into the page metadata. - * @deprecated for legacy use only. Can be removed when https://github.com/elastic/kibana/issues/127733 is done. - * @internal - */ - vars?: Record; - /** * @internal * This is only used for integration tests that allow us to verify which config keys are exposed to the browser. diff --git a/packages/core/rendering/core-rendering-server-internal/tsconfig.json b/packages/core/rendering/core-rendering-server-internal/tsconfig.json index e306dca24059c..2689069f79d79 100644 --- a/packages/core/rendering/core-rendering-server-internal/tsconfig.json +++ b/packages/core/rendering/core-rendering-server-internal/tsconfig.json @@ -44,6 +44,7 @@ "@kbn/core-i18n-server", "@kbn/core-i18n-server-internal", "@kbn/core-i18n-server-mocks", + "@kbn/apm-config-loader", ], "exclude": [ "target/**/*", diff --git a/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts b/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts index c1ca8cb752d2d..f7a0685967ba8 100644 --- a/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts +++ b/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts @@ -7,6 +7,7 @@ */ import { i18n } from '@kbn/i18n'; +import type { InjectedMetadata } from '@kbn/core-injected-metadata-common-internal'; import { KBN_LOAD_MARKS } from './events'; import { CoreSystem } from './core_system'; import { ApmSystem } from './apm_system'; @@ -19,12 +20,15 @@ export async function __kbnBootstrap__() { detail: LOAD_BOOTSTRAP_START, }); - const injectedMetadata = JSON.parse( + const injectedMetadata: InjectedMetadata = JSON.parse( document.querySelector('kbn-injected-metadata')!.getAttribute('data')! ); let i18nError: Error | undefined; - const apmSystem = new ApmSystem(injectedMetadata.vars.apmConfig, injectedMetadata.basePath); + const apmSystem = new ApmSystem( + injectedMetadata.apmConfig ?? undefined, + injectedMetadata.basePath + ); await Promise.all([ // eslint-disable-next-line no-console diff --git a/packages/core/root/core-root-browser-internal/tsconfig.json b/packages/core/root/core-root-browser-internal/tsconfig.json index 152c7d3683e38..e576ecf8cf920 100644 --- a/packages/core/root/core-root-browser-internal/tsconfig.json +++ b/packages/core/root/core-root-browser-internal/tsconfig.json @@ -66,6 +66,7 @@ "@kbn/core-security-browser-internal", "@kbn/core-user-profile-browser-mocks", "@kbn/core-user-profile-browser-internal", + "@kbn/core-injected-metadata-common-internal", ], "exclude": [ "target/**/*", From 0db883a9d5adfbf27b753a8bfe768b9665769b06 Mon Sep 17 00:00:00 2001 From: jennypavlova Date: Mon, 22 Jul 2024 14:09:57 +0200 Subject: [PATCH 3/3] [Infra] Disable Top 10 functions full screen table in a flyout (#188743) ## Summary This PR disables functions full-screen option in the asset details flyout. It adds control of the `showFullScreenSelector` to the `EmbeddableFunctionsGrid` so we can set it based on the render mode in the asset details - in apm it will be set to true as before. ## Testing - Go to the asset details page - Check the Universal Profiling tab > Top 10 Functions tab - The full-screen option should be visible: ![image](https://github.com/user-attachments/assets/aace7ca0-ed4f-404d-8cbf-91c29088c133) - Same in the tab inside APM service view: ![image](https://github.com/user-attachments/assets/f885630a-4f43-4c23-9ff4-05c03f7ede72) - Go to Hosts view and open the flyout for a host - The full-screen option should not be visible: ![image](https://github.com/user-attachments/assets/e724c569-df7a-4e4a-b6c3-9dcfa7b60ad6) --------- Co-authored-by: Elastic Machine --- .../components/asset_details/tabs/profiling/functions.tsx | 3 +++ .../profiling/embeddables/embeddable_functions.tsx | 1 + .../profiling/public/components/topn_functions/index.tsx | 4 +++- .../public/embeddables/functions/embeddable_functions.tsx | 8 +++++++- .../embeddables/functions/embeddable_functions_grid.tsx | 4 +++- 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/profiling/functions.tsx b/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/profiling/functions.tsx index 811172fca2695..5dee9438b280c 100644 --- a/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/profiling/functions.tsx +++ b/x-pack/plugins/observability_solution/infra/public/components/asset_details/tabs/profiling/functions.tsx @@ -31,11 +31,13 @@ export function Functions({ kuery }: Props) { const { dateRange, getDateRangeInTimestamp } = useDatePickerContext(); const { from, to } = getDateRangeInTimestamp(); const { request$ } = useRequestObservable(); + const { renderMode } = useAssetDetailsRenderPropsContext(); const profilingLinkLocator = services.observabilityShared.locators.profiling.topNFunctionsLocator; const profilingLinkLabel = i18n.translate('xpack.infra.flamegraph.profilingAppTopFunctionsLink', { defaultMessage: 'Go to Universal Profiling Functions', }); + const showFullScreenSelector = renderMode.mode === 'page'; const params = useMemo( () => ({ @@ -86,6 +88,7 @@ export function Functions({ kuery }: Props) { rangeFrom={from} rangeTo={to} height="60vh" + showFullScreenSelector={showFullScreenSelector} /> ); diff --git a/x-pack/plugins/observability_solution/observability_shared/public/components/profiling/embeddables/embeddable_functions.tsx b/x-pack/plugins/observability_solution/observability_shared/public/components/profiling/embeddables/embeddable_functions.tsx index c4fad57b890ce..aa5d3c335ec05 100644 --- a/x-pack/plugins/observability_solution/observability_shared/public/components/profiling/embeddables/embeddable_functions.tsx +++ b/x-pack/plugins/observability_solution/observability_shared/public/components/profiling/embeddables/embeddable_functions.tsx @@ -17,6 +17,7 @@ interface Props { rangeFrom: number; rangeTo: number; height?: string; + showFullScreenSelector?: boolean; } export function EmbeddableFunctions(props: Props) { diff --git a/x-pack/plugins/observability_solution/profiling/public/components/topn_functions/index.tsx b/x-pack/plugins/observability_solution/profiling/public/components/topn_functions/index.tsx index 4f56865d54a84..cad716654a843 100644 --- a/x-pack/plugins/observability_solution/profiling/public/components/topn_functions/index.tsx +++ b/x-pack/plugins/observability_solution/profiling/public/components/topn_functions/index.tsx @@ -32,6 +32,7 @@ interface Props { comparisonTopNFunctions?: TopNFunctions; totalSeconds: number; isDifferentialView: boolean; + showFullScreenSelector?: boolean; baselineScaleFactor?: number; comparisonScaleFactor?: number; onFrameClick?: (functionName: string) => void; @@ -50,6 +51,7 @@ export const TopNFunctionsGrid = ({ topNFunctions, comparisonTopNFunctions, totalSeconds, + showFullScreenSelector = true, isDifferentialView, baselineScaleFactor, comparisonScaleFactor, @@ -316,7 +318,7 @@ export const TopNFunctionsGrid = ({ showColumnSelector: false, showKeyboardShortcuts: !isDifferentialView, showDisplaySelector: !isDifferentialView, - showFullScreenSelector: !isDifferentialView, + showFullScreenSelector: showFullScreenSelector && !isDifferentialView, showSortSelector: false, }} virtualizationOptions={{ diff --git a/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions.tsx b/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions.tsx index e281f12224c8f..851eeeb8fe103 100644 --- a/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions.tsx +++ b/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions.tsx @@ -23,6 +23,7 @@ export interface FunctionsProps { isLoading: boolean; rangeFrom: number; rangeTo: number; + showFullScreenSelector?: boolean; } export function EmbeddableFunctions({ @@ -30,6 +31,7 @@ export function EmbeddableFunctions({ isLoading, rangeFrom, rangeTo, + showFullScreenSelector, ...deps }: EmbeddableFunctionsProps) { const totalSeconds = useMemo(() => (rangeTo - rangeFrom) / 1000, [rangeFrom, rangeTo]); @@ -37,7 +39,11 @@ export function EmbeddableFunctions({
- +
diff --git a/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions_grid.tsx b/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions_grid.tsx index 5579217a59d22..8b4dfd5d62c27 100644 --- a/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions_grid.tsx +++ b/x-pack/plugins/observability_solution/profiling/public/embeddables/functions/embeddable_functions_grid.tsx @@ -13,9 +13,10 @@ import { TopNFunctionsGrid } from '../../components/topn_functions'; interface Props { data?: TopNFunctions; totalSeconds: number; + showFullScreenSelector?: boolean; } -export function EmbeddableFunctionsGrid({ data, totalSeconds }: Props) { +export function EmbeddableFunctionsGrid({ data, totalSeconds, showFullScreenSelector }: Props) { const [sortField, setSortField] = useState(TopNFunctionSortField.Rank); const [sortDirection, setSortDirection] = useState<'asc' | 'desc'>('asc'); const [pageIndex, setPageIndex] = useState(0); @@ -34,6 +35,7 @@ export function EmbeddableFunctionsGrid({ data, totalSeconds }: Props) { setSortDirection(sorting.direction); }} isEmbedded + showFullScreenSelector={showFullScreenSelector} /> ); }