From ff90bd4d8602d91e3d53f2764c68a9133ef666fa Mon Sep 17 00:00:00 2001 From: Liza Katz Date: Mon, 7 Mar 2022 19:33:30 +0200 Subject: [PATCH 1/6] Use execution context for Fullstory (#126780) * fix a couple bugs in context management add execution context to fullstory * Update execution_context_service.ts * stop and app name tests * Use execution context in fullstory * Fix user hash Report org id to FS * Use setUserVars for esorgid * pass orgid into identify * fix Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- x-pack/plugins/cloud/public/fullstory.ts | 2 + .../plugins/cloud/public/plugin.test.mocks.ts | 1 + x-pack/plugins/cloud/public/plugin.test.ts | 91 +++++++++++++++---- x-pack/plugins/cloud/public/plugin.tsx | 64 +++++++++---- 4 files changed, 121 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/cloud/public/fullstory.ts b/x-pack/plugins/cloud/public/fullstory.ts index 4f76abf540cae3..602b1c4cc63d37 100644 --- a/x-pack/plugins/cloud/public/fullstory.ts +++ b/x-pack/plugins/cloud/public/fullstory.ts @@ -15,9 +15,11 @@ export interface FullStoryDeps { } export type FullstoryUserVars = Record; +export type FullstoryVars = Record; export interface FullStoryApi { identify(userId: string, userVars?: FullstoryUserVars): void; + setVars(pageName: string, vars?: FullstoryVars): void; setUserVars(userVars?: FullstoryUserVars): void; event(eventName: string, eventProperties: Record): void; } diff --git a/x-pack/plugins/cloud/public/plugin.test.mocks.ts b/x-pack/plugins/cloud/public/plugin.test.mocks.ts index b79fb1bc651307..1c185d0194912a 100644 --- a/x-pack/plugins/cloud/public/plugin.test.mocks.ts +++ b/x-pack/plugins/cloud/public/plugin.test.mocks.ts @@ -11,6 +11,7 @@ import type { FullStoryDeps, FullStoryApi, FullStoryService } from './fullstory' export const fullStoryApiMock: jest.Mocked = { event: jest.fn(), setUserVars: jest.fn(), + setVars: jest.fn(), identify: jest.fn(), }; export const initializeFullStoryMock = jest.fn(() => ({ diff --git a/x-pack/plugins/cloud/public/plugin.test.ts b/x-pack/plugins/cloud/public/plugin.test.ts index 1eef581610f004..edbf724e25390e 100644 --- a/x-pack/plugins/cloud/public/plugin.test.ts +++ b/x-pack/plugins/cloud/public/plugin.test.ts @@ -12,6 +12,7 @@ import { securityMock } from '../../security/public/mocks'; import { fullStoryApiMock, initializeFullStoryMock } from './plugin.test.mocks'; import { CloudPlugin, CloudConfigType, loadFullStoryUserId } from './plugin'; import { Observable, Subject } from 'rxjs'; +import { KibanaExecutionContext } from 'kibana/public'; describe('Cloud Plugin', () => { describe('#setup', () => { @@ -24,12 +25,12 @@ describe('Cloud Plugin', () => { config = {}, securityEnabled = true, currentUserProps = {}, - currentAppId$ = undefined, + currentContext$ = undefined, }: { config?: Partial; securityEnabled?: boolean; currentUserProps?: Record; - currentAppId$?: Observable; + currentContext$?: Observable; }) => { const initContext = coreMock.createPluginInitializerContext({ id: 'cloudId', @@ -51,8 +52,8 @@ describe('Cloud Plugin', () => { const coreSetup = coreMock.createSetup(); const coreStart = coreMock.createStart(); - if (currentAppId$) { - coreStart.application.currentAppId$ = currentAppId$; + if (currentContext$) { + coreStart.executionContext.context$ = currentContext$; } coreSetup.getStartServices.mockResolvedValue([coreStart, {}, undefined]); @@ -94,44 +95,98 @@ describe('Cloud Plugin', () => { }); expect(fullStoryApiMock.identify).toHaveBeenCalledWith( - '03ac674216f3e15c761ee1a5e255f067953623c8b388b4459e13f978d7c846f4', + '5ef112cfdae3dea57097bc276e275b2816e73ef2a398dc0ffaf5b6b4e3af2041', { version_str: 'version', version_major_int: -1, version_minor_int: -1, version_patch_int: -1, + org_id_str: 'cloudId', } ); }); - it('calls FS.setUserVars everytime an app changes', async () => { - const currentAppId$ = new Subject(); + it('user hash includes org id', async () => { + await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg1' }, + currentUserProps: { + username: '1234', + }, + }); + + const hashId1 = fullStoryApiMock.identify.mock.calls[0][0]; + + await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg2' }, + currentUserProps: { + username: '1234', + }, + }); + + const hashId2 = fullStoryApiMock.identify.mock.calls[1][0]; + + expect(hashId1).not.toEqual(hashId2); + }); + + it('calls FS.setVars everytime an app changes', async () => { + const currentContext$ = new Subject(); const { plugin } = await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' } }, currentUserProps: { username: '1234', }, - currentAppId$, + currentContext$, }); - expect(fullStoryApiMock.setUserVars).not.toHaveBeenCalled(); - currentAppId$.next('App1'); - expect(fullStoryApiMock.setUserVars).toHaveBeenCalledWith({ + // takes the app name + expect(fullStoryApiMock.setVars).not.toHaveBeenCalled(); + currentContext$.next({ + name: 'App1', + description: '123', + }); + + expect(fullStoryApiMock.setVars).toHaveBeenCalledWith('page', { + pageName: 'App1', app_id_str: 'App1', }); - currentAppId$.next(); - expect(fullStoryApiMock.setUserVars).toHaveBeenCalledWith({ - app_id_str: 'unknown', + + // context clear + currentContext$.next({}); + expect(fullStoryApiMock.setVars).toHaveBeenCalledWith('page', { + pageName: 'App1', + app_id_str: 'App1', }); - currentAppId$.next('App2'); - expect(fullStoryApiMock.setUserVars).toHaveBeenCalledWith({ + // different app + currentContext$.next({ + name: 'App2', + page: 'page2', + id: '123', + }); + expect(fullStoryApiMock.setVars).toHaveBeenCalledWith('page', { + pageName: 'App2:page2', app_id_str: 'App2', + page_str: 'page2', + ent_id_str: '123', + }); + + // Back to first app + currentContext$.next({ + name: 'App1', + page: 'page3', + id: '123', + }); + + expect(fullStoryApiMock.setVars).toHaveBeenCalledWith('page', { + pageName: 'App1:page3', + app_id_str: 'App1', + page_str: 'page3', + ent_id_str: '123', }); - expect(currentAppId$.observers.length).toBe(1); + expect(currentContext$.observers.length).toBe(1); plugin.stop(); - expect(currentAppId$.observers.length).toBe(0); + expect(currentContext$.observers.length).toBe(0); }); it('does not call FS.identify when security is not available', async () => { diff --git a/x-pack/plugins/cloud/public/plugin.tsx b/x-pack/plugins/cloud/public/plugin.tsx index 991a7c1f8b565c..89f24971de25c1 100644 --- a/x-pack/plugins/cloud/public/plugin.tsx +++ b/x-pack/plugins/cloud/public/plugin.tsx @@ -13,11 +13,12 @@ import { PluginInitializerContext, HttpStart, IBasePath, - ApplicationStart, + ExecutionContextStart, } from 'src/core/public'; import { i18n } from '@kbn/i18n'; import useObservable from 'react-use/lib/useObservable'; import { BehaviorSubject, Subscription } from 'rxjs'; +import { compact, isUndefined, omitBy } from 'lodash'; import type { AuthenticatedUser, SecurityPluginSetup, @@ -83,8 +84,9 @@ export interface CloudSetup { } interface SetupFullstoryDeps extends CloudSetupDependencies { - application?: Promise; + executionContextPromise?: Promise; basePath: IBasePath; + esOrgId?: string; } interface SetupChatDeps extends Pick { @@ -103,11 +105,16 @@ export class CloudPlugin implements Plugin { } public setup(core: CoreSetup, { home, security }: CloudSetupDependencies) { - const application = core.getStartServices().then(([coreStart]) => { - return coreStart.application; + const executionContextPromise = core.getStartServices().then(([coreStart]) => { + return coreStart.executionContext; }); - this.setupFullstory({ basePath: core.http.basePath, security, application }).catch((e) => + this.setupFullstory({ + basePath: core.http.basePath, + security, + executionContextPromise, + esOrgId: this.config.id, + }).catch((e) => // eslint-disable-next-line no-console console.debug(`Error setting up FullStory: ${e.toString()}`) ); @@ -223,9 +230,14 @@ export class CloudPlugin implements Plugin { return user?.roles.includes('superuser') ?? true; } - private async setupFullstory({ basePath, security, application }: SetupFullstoryDeps) { - const { enabled, org_id: orgId } = this.config.full_story; - if (!enabled || !orgId) { + private async setupFullstory({ + basePath, + security, + executionContextPromise, + esOrgId, + }: SetupFullstoryDeps) { + const { enabled, org_id: fsOrgId } = this.config.full_story; + if (!enabled || !fsOrgId) { return; // do not load any fullstory code in the browser if not enabled } @@ -243,7 +255,7 @@ export class CloudPlugin implements Plugin { const { fullStory, sha256 } = initializeFullStory({ basePath, - orgId, + orgId: fsOrgId, packageInfo: this.initializerContext.env.packageInfo, }); @@ -252,16 +264,29 @@ export class CloudPlugin implements Plugin { // This needs to be called syncronously to be sure that we populate the user ID soon enough to make sessions merging // across domains work if (userId) { - // Do the hashing here to keep it at clear as possible in our source code that we do not send literal user IDs - const hashedId = sha256(userId.toString()); - application - ?.then(async () => { - const appStart = await application; - this.appSubscription = appStart.currentAppId$.subscribe((appId) => { - // Update the current application every time it changes - fullStory.setUserVars({ - app_id_str: appId ?? 'unknown', - }); + // Join the cloud org id and the user to create a truly unique user id. + // The hashing here is to keep it at clear as possible in our source code that we do not send literal user IDs + const hashedId = sha256(esOrgId ? `${esOrgId}:${userId}` : `${userId}`); + + executionContextPromise + ?.then(async (executionContext) => { + this.appSubscription = executionContext.context$.subscribe((context) => { + const { name, page, id } = context; + // Update the current context every time it changes + fullStory.setVars( + 'page', + omitBy( + { + // Read about the special pageName property + // https://help.fullstory.com/hc/en-us/articles/1500004101581-FS-setVars-API-Sending-custom-page-data-to-FullStory + pageName: `${compact([name, page]).join(':')}`, + app_id_str: name ?? 'unknown', + page_str: page, + ent_id_str: id, + }, + isUndefined + ) + ); }); }) .catch((e) => { @@ -282,6 +307,7 @@ export class CloudPlugin implements Plugin { version_major_int: parsedVer[0] ?? -1, version_minor_int: parsedVer[1] ?? -1, version_patch_int: parsedVer[2] ?? -1, + org_id_str: esOrgId, }); } } catch (e) { From 935fcc2d3a8ab00bcb1bbc774dcbb79af641bc82 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Mon, 7 Mar 2022 17:50:58 +0000 Subject: [PATCH 2/6] skip flaky suite (#126658) --- test/functional_ccs/apps/discover/_data_view_ccs.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional_ccs/apps/discover/_data_view_ccs.ts b/test/functional_ccs/apps/discover/_data_view_ccs.ts index 91d9cb2faf6816..5a04e0e2a7532b 100644 --- a/test/functional_ccs/apps/discover/_data_view_ccs.ts +++ b/test/functional_ccs/apps/discover/_data_view_ccs.ts @@ -27,7 +27,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.click('saveIndexPatternButton'); }; - describe('discover integration with data view editor', function describeIndexTests() { + // FLAKY: https://github.com/elastic/kibana/issues/126658 + describe.skip('discover integration with data view editor', function describeIndexTests() { before(async function () { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); From b2e419d7077c0761c554dbe083ccb7fe7ce9dfcf Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Mon, 7 Mar 2022 11:16:27 -0700 Subject: [PATCH 3/6] [Reporting] Improve error logging for rescheduled jobs (#126737) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../server/lib/tasks/monitor_reports.ts | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/reporting/server/lib/tasks/monitor_reports.ts b/x-pack/plugins/reporting/server/lib/tasks/monitor_reports.ts index 1d406d7a5cc623..56e12d7d5512bd 100644 --- a/x-pack/plugins/reporting/server/lib/tasks/monitor_reports.ts +++ b/x-pack/plugins/reporting/server/lib/tasks/monitor_reports.ts @@ -92,31 +92,42 @@ export class MonitorReportsTask implements ReportingTask { return; } - const { - _id: jobId, - _source: { process_expiration: processExpiration, status }, - } = recoveredJob; + const report = new SavedReport({ ...recoveredJob, ...recoveredJob._source }); + const { _id: jobId, process_expiration: processExpiration, status } = report; + const eventLog = this.reporting.getEventLogger(report); if (![statuses.JOB_STATUS_PENDING, statuses.JOB_STATUS_PROCESSING].includes(status)) { - throw new Error(`Invalid job status in the monitoring search result: ${status}`); // only pending or processing jobs possibility need rescheduling + const invalidStatusError = new Error( + `Invalid job status in the monitoring search result: ${status}` + ); // only pending or processing jobs possibility need rescheduling + this.logger.error(invalidStatusError); + eventLog.logError(invalidStatusError); + + // fatal: can not reschedule the job + throw invalidStatusError; } if (status === statuses.JOB_STATUS_PENDING) { - this.logger.info( + const migratingJobError = new Error( `${jobId} was scheduled in a previous version and left in [${status}] status. Rescheduling...` ); + this.logger.error(migratingJobError); + eventLog.logError(migratingJobError); } if (status === statuses.JOB_STATUS_PROCESSING) { const expirationTime = moment(processExpiration); const overdueValue = moment().valueOf() - expirationTime.valueOf(); - this.logger.info( + const overdueExpirationError = new Error( `${jobId} status is [${status}] and the expiration time was [${overdueValue}ms] ago. Rescheduling...` ); + this.logger.error(overdueExpirationError); + eventLog.logError(overdueExpirationError); } + eventLog.logRetry(); + // clear process expiration and set status to pending - const report = new SavedReport({ ...recoveredJob, ...recoveredJob._source }); await reportingStore.prepareReportForRetry(report); // if there is a version conflict response, this just throws and logs an error // clear process expiration and reschedule @@ -154,8 +165,6 @@ export class MonitorReportsTask implements ReportingTask { const newTask = await this.reporting.scheduleTask(task); - this.reporting.getEventLogger({ _id: task.id, ...task }, newTask).logRetry(); - return newTask; } From 629e0f3995ed74702ec91ad4d605e3c14ed6ea27 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Mon, 7 Mar 2022 18:24:55 +0000 Subject: [PATCH 4/6] skip flaky suite (#126027) --- test/functional/apps/management/_scripted_fields_filter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/management/_scripted_fields_filter.ts b/test/functional/apps/management/_scripted_fields_filter.ts index 82d15908197506..4f6d1a41d05237 100644 --- a/test/functional/apps/management/_scripted_fields_filter.ts +++ b/test/functional/apps/management/_scripted_fields_filter.ts @@ -17,7 +17,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const PageObjects = getPageObjects(['settings']); - describe('filter scripted fields', function describeIndexTests() { + // FLAKY: https://github.com/elastic/kibana/issues/126027 + describe.skip('filter scripted fields', function describeIndexTests() { before(async function () { // delete .kibana index and then wait for Kibana to re-create it await browser.setWindowSize(1200, 800); From 064078235ec9a307b557691e016bf455e04e6a27 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 7 Mar 2022 18:33:00 +0000 Subject: [PATCH 5/6] [ML] Adding data recognizer module config cache (#126338) * [ML] Adding data recognizer module config cache * adding comment about cache Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../models/data_recognizer/data_recognizer.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/server/models/data_recognizer/data_recognizer.ts b/x-pack/plugins/ml/server/models/data_recognizer/data_recognizer.ts index 3df5016f560c07..4a621bc5f608b5 100644 --- a/x-pack/plugins/ml/server/models/data_recognizer/data_recognizer.ts +++ b/x-pack/plugins/ml/server/models/data_recognizer/data_recognizer.ts @@ -124,6 +124,17 @@ export class DataRecognizer { private _resultsService: ReturnType; private _calculateModelMemoryLimit: ReturnType; + /** + * A temporary cache of configs loaded from disk and from save object service. + * The configs from disk will not change while kibana is running. + * The configs from saved objects could potentially change while an instance of + * DataRecognizer exists, if a fleet package containing modules is installed. + * However the chance of this happening is very low and so the benefit of using + * this cache outweighs the risk of the cache being out of date during the short + * existence of a DataRecognizer instance. + */ + private _configCache: Config[] | null = null; + /** * List of the module jobs that require model memory estimation */ @@ -181,6 +192,10 @@ export class DataRecognizer { } private async _loadConfigs(): Promise { + if (this._configCache !== null) { + return this._configCache; + } + const configs: Config[] = []; const dirs = await this._listDirs(this._modulesDir); await Promise.all( @@ -211,7 +226,9 @@ export class DataRecognizer { isSavedObject: true, })); - return [...configs, ...savedObjectConfigs]; + this._configCache = [...configs, ...savedObjectConfigs]; + + return this._configCache; } private async _loadSavedObjectModules() { From de3ae9bfec3a5b72acb808d3308e2d715ba89115 Mon Sep 17 00:00:00 2001 From: "Mark J. Hoy" Date: Mon, 7 Mar 2022 14:33:10 -0500 Subject: [PATCH 6/6] Remove License Requirement for Enterprise Search App Search Meta Engines (#127046) * Remove license for enterprise search meta engines * missed a flag for the unit test --- .../app_search/utils/role/get_role_abilities.test.ts | 6 +++--- .../app_search/utils/role/get_role_abilities.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.test.ts index 60d0dcc0c5911e..0d5e4e9824f170 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.test.ts @@ -23,6 +23,7 @@ describe('getRoleAbilities', () => { // Has access canViewAccountCredentials: true, canManageEngines: true, + canManageMetaEngines: true, // Does not have access canViewMetaEngines: false, canViewEngineAnalytics: false, @@ -35,7 +36,6 @@ describe('getRoleAbilities', () => { canViewMetaEngineSourceEngines: false, canViewSettings: false, canViewRoleMappings: false, - canManageMetaEngines: false, canManageLogSettings: false, canManageSettings: false, canManageEngineCrawler: false, @@ -81,10 +81,10 @@ describe('getRoleAbilities', () => { expect(myRole.canManageMetaEngines).toEqual(true); }); - it('returns false when the user can manage any engines but the account does not have a platinum license', () => { + it('returns true when the user can manage any engines but the account does not have a platinum license', () => { const myRole = getRoleAbilities({ ...mockRole, ...canManageEngines }, false); - expect(myRole.canManageMetaEngines).toEqual(false); + expect(myRole.canManageMetaEngines).toEqual(true); }); it('returns false when has a platinum license but the user cannot manage any engines', () => { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.ts index ef3e22d851f387..15cba16ab04345 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/get_role_abilities.ts @@ -49,7 +49,7 @@ export const getRoleAbilities = (role: Account['role'], hasPlatinumLicense = fal canViewSettings: myRole.can('view', 'account_settings'), canViewRoleMappings: myRole.can('view', 'role_mappings'), canManageEngines: myRole.can('manage', 'account_engines'), - canManageMetaEngines: hasPlatinumLicense && myRole.can('manage', 'account_engines'), + canManageMetaEngines: myRole.can('manage', 'account_engines'), canManageLogSettings: myRole.can('manage', 'account_log_settings'), canManageSettings: myRole.can('manage', 'account_settings'), canManageEngineCrawler: myRole.can('manage', 'engine_crawler'),