From b3b16dd1159362afec3de60210e7680802535930 Mon Sep 17 00:00:00 2001 From: kyracho Date: Mon, 14 Oct 2024 00:14:53 -0700 Subject: [PATCH 1/7] basic enhancements for import logging --- .../src/import/saved_objects_importer.ts | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts index f990eb13c435b..a191f33a7a9c1 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts @@ -62,7 +62,16 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { compatibilityMode, managed, }: SavedObjectsImportOptions): Promise { - this.#log.debug('Starting the import process'); + // Get the allowed types for import from the typeRegistry + const allowedTypes = this.#typeRegistry + .getImportableAndExportableTypes() + .map((type) => type.name); + // Log the allowed types, similar to how the exporter logs types + this.#log.debug(`Initiating import process.`); + this.#log.debug(`Allowed types: [${allowedTypes.join(', ')}]`); + this.#log.info(`Import size limit: ${this.#importSizeLimit} saved objects.`); + const overwriteStatus = overwrite ? 'enabled' : 'disabled'; + this.#log.info(`Automatic overwrite is ${overwriteStatus} for the current import operation.`); return importSavedObjectsFromStream({ readStream, createNewCopies, @@ -75,7 +84,19 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { typeRegistry: this.#typeRegistry, importHooks: this.#importHooks, managed, - }); + }) + .then((response: SavedObjectsImportResponse) => { + this.#log.info(`🌸 Successfully imported ${response.successCount} saved objects.`); + return response; + }) + .catch((error) => { + this.#log.error('Failed to import saved objects'); + const errors = Array.isArray(error) ? error : [error]; + errors.forEach((err) => { + this.#log.error(`Import error: ${err.message}`); + }); + throw error; + }); } public resolveImportErrors({ From dee4e6aef51edb2c7f1a3dc46aea7aae909d6691 Mon Sep 17 00:00:00 2001 From: kyracho Date: Fri, 25 Oct 2024 19:52:16 -0700 Subject: [PATCH 2/7] move logging onto importSavedObjectsFromStream --- .../src/import/import_saved_objects.ts | 11 ++++++++- .../src/import/saved_objects_importer.ts | 23 +------------------ 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts index 4182daa610b37..6576caf7be1fc 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts @@ -17,6 +17,7 @@ import type { ISavedObjectTypeRegistry, SavedObjectsImportHook, } from '@kbn/core-saved-objects-server'; +import type { Logger } from '@kbn/logging'; import { checkReferenceOrigins, validateReferences, @@ -59,6 +60,7 @@ export interface ImportSavedObjectsOptions { * If provided, Kibana will apply the given option to the `managed` property. */ managed?: boolean; + log: Logger; } /** @@ -79,7 +81,9 @@ export async function importSavedObjectsFromStream({ refresh, compatibilityMode, managed, + log, }: ImportSavedObjectsOptions): Promise { + log.debug(`Importing with overwrite ${overwrite ? 'enabled' : 'disabled'} and size limit ${objectLimit}`); let errorAccumulator: SavedObjectsImportFailure[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); @@ -90,6 +94,7 @@ export async function importSavedObjectsFromStream({ supportedTypes, managed, }); + log.debug(`Importing types: ${[...new Set(collectSavedObjectsResult.collectedObjects.map((obj) => obj.type))].join(', ')}`); errorAccumulator = [...errorAccumulator, ...collectSavedObjectsResult.errors]; // Map of all IDs for objects that we are attempting to import, and any references that are not included in the read stream; // each value is empty by default @@ -197,7 +202,11 @@ export async function importSavedObjectsFromStream({ objects: createSavedObjectsResult.createdObjects, importHooks, }); - + if (errorAccumulator.length > 0) { + log.error(`Failed to import saved objects. ${errorAccumulator.length} errors: ${JSON.stringify(errorAccumulator)}`); + } else { + log.info(`Successfully imported ${createSavedObjectsResult.createdObjects.length} saved objects.`); + } return { successCount: createSavedObjectsResult.createdObjects.length, success: errorAccumulator.length === 0, diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts index a191f33a7a9c1..0c3dd717e4db4 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts @@ -62,16 +62,6 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { compatibilityMode, managed, }: SavedObjectsImportOptions): Promise { - // Get the allowed types for import from the typeRegistry - const allowedTypes = this.#typeRegistry - .getImportableAndExportableTypes() - .map((type) => type.name); - // Log the allowed types, similar to how the exporter logs types - this.#log.debug(`Initiating import process.`); - this.#log.debug(`Allowed types: [${allowedTypes.join(', ')}]`); - this.#log.info(`Import size limit: ${this.#importSizeLimit} saved objects.`); - const overwriteStatus = overwrite ? 'enabled' : 'disabled'; - this.#log.info(`Automatic overwrite is ${overwriteStatus} for the current import operation.`); return importSavedObjectsFromStream({ readStream, createNewCopies, @@ -84,19 +74,8 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { typeRegistry: this.#typeRegistry, importHooks: this.#importHooks, managed, + log: this.#log, }) - .then((response: SavedObjectsImportResponse) => { - this.#log.info(`🌸 Successfully imported ${response.successCount} saved objects.`); - return response; - }) - .catch((error) => { - this.#log.error('Failed to import saved objects'); - const errors = Array.isArray(error) ? error : [error]; - errors.forEach((err) => { - this.#log.error(`Import error: ${err.message}`); - }); - throw error; - }); } public resolveImportErrors({ From 4d93dcef93dd223a9692e84657547443c378bd69 Mon Sep 17 00:00:00 2001 From: kyracho Date: Fri, 25 Oct 2024 19:52:45 -0700 Subject: [PATCH 3/7] adjust unit test for changes --- .../src/import/import_saved_objects.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts index d8c2b0b25874f..d0c0a7c19963c 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts @@ -72,7 +72,17 @@ describe('#importSavedObjectsFromStream', () => { let savedObjectsClient: jest.Mocked; let typeRegistry: jest.Mocked; const namespace = 'some-namespace'; - + const mockLog = { + trace: jest.fn(), + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + fatal: jest.fn(), + log: jest.fn(), + error: jest.fn(), + get: jest.fn(), + isLevelEnabled: jest.fn(), + }; const setupOptions = ({ createNewCopies = false, getTypeImpl = (type: string) => @@ -102,6 +112,7 @@ describe('#importSavedObjectsFromStream', () => { createNewCopies, importHooks, managed, + log: mockLog, }; }; const createObject = ({ From 05d6df7bf29acfb25e6e1017508129ce339fbec2 Mon Sep 17 00:00:00 2001 From: Kyra Cho Date: Sun, 27 Oct 2024 09:37:45 +0000 Subject: [PATCH 4/7] fix formatting issues for eslintprettier/prettier --- .../src/import/import_saved_objects.ts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts index 6576caf7be1fc..7d6bf9668286a 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.ts @@ -83,7 +83,9 @@ export async function importSavedObjectsFromStream({ managed, log, }: ImportSavedObjectsOptions): Promise { - log.debug(`Importing with overwrite ${overwrite ? 'enabled' : 'disabled'} and size limit ${objectLimit}`); + log.debug( + `Importing with overwrite ${overwrite ? 'enabled' : 'disabled'} and size limit ${objectLimit}` + ); let errorAccumulator: SavedObjectsImportFailure[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); @@ -94,7 +96,11 @@ export async function importSavedObjectsFromStream({ supportedTypes, managed, }); - log.debug(`Importing types: ${[...new Set(collectSavedObjectsResult.collectedObjects.map((obj) => obj.type))].join(', ')}`); + log.debug( + `Importing types: ${[ + ...new Set(collectSavedObjectsResult.collectedObjects.map((obj) => obj.type)), + ].join(', ')}` + ); errorAccumulator = [...errorAccumulator, ...collectSavedObjectsResult.errors]; // Map of all IDs for objects that we are attempting to import, and any references that are not included in the read stream; // each value is empty by default @@ -203,9 +209,15 @@ export async function importSavedObjectsFromStream({ importHooks, }); if (errorAccumulator.length > 0) { - log.error(`Failed to import saved objects. ${errorAccumulator.length} errors: ${JSON.stringify(errorAccumulator)}`); + log.error( + `Failed to import saved objects. ${errorAccumulator.length} errors: ${JSON.stringify( + errorAccumulator + )}` + ); } else { - log.info(`Successfully imported ${createSavedObjectsResult.createdObjects.length} saved objects.`); + log.info( + `Successfully imported ${createSavedObjectsResult.createdObjects.length} saved objects.` + ); } return { successCount: createSavedObjectsResult.createdObjects.length, From c44e2ce9a1fbf68ebaea611bfa3b215e754a2557 Mon Sep 17 00:00:00 2001 From: Kyra Cho Date: Mon, 28 Oct 2024 00:03:16 +0000 Subject: [PATCH 5/7] fix unit test --- .../src/import/import_saved_objects.test.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts index d0c0a7c19963c..06c8e351bc445 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/import_saved_objects.test.ts @@ -19,6 +19,7 @@ import { } from './import_saved_objects.test.mock'; import { Readable } from 'stream'; +import { loggerMock, type MockedLogger } from '@kbn/logging-mocks'; import { v4 as uuidv4 } from 'uuid'; import type { SavedObjectsImportFailure, @@ -40,8 +41,10 @@ import { import type { ImportStateMap } from './lib'; describe('#importSavedObjectsFromStream', () => { + let logger: MockedLogger; beforeEach(() => { jest.clearAllMocks(); + logger = loggerMock.create(); // mock empty output of each of these mocked modules so the import doesn't throw an error mockCollectSavedObjects.mockResolvedValue({ errors: [], @@ -72,17 +75,6 @@ describe('#importSavedObjectsFromStream', () => { let savedObjectsClient: jest.Mocked; let typeRegistry: jest.Mocked; const namespace = 'some-namespace'; - const mockLog = { - trace: jest.fn(), - debug: jest.fn(), - info: jest.fn(), - warn: jest.fn(), - fatal: jest.fn(), - log: jest.fn(), - error: jest.fn(), - get: jest.fn(), - isLevelEnabled: jest.fn(), - }; const setupOptions = ({ createNewCopies = false, getTypeImpl = (type: string) => @@ -112,7 +104,7 @@ describe('#importSavedObjectsFromStream', () => { createNewCopies, importHooks, managed, - log: mockLog, + log: logger, }; }; const createObject = ({ From 0a2abc94e6308be53157ccf7fc2828a0541059aa Mon Sep 17 00:00:00 2001 From: Kyra Cho Date: Mon, 28 Oct 2024 00:29:23 +0000 Subject: [PATCH 6/7] fix integration test to use existing mock logger instead of custom --- .../saved_objects/routes/import.test.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/core/server/integration_tests/saved_objects/routes/import.test.ts b/src/core/server/integration_tests/saved_objects/routes/import.test.ts index 917f7f1642e8c..bf1fae4967e95 100644 --- a/src/core/server/integration_tests/saved_objects/routes/import.test.ts +++ b/src/core/server/integration_tests/saved_objects/routes/import.test.ts @@ -13,7 +13,6 @@ import supertest from 'supertest'; import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server'; import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks'; import type { ICoreUsageStatsClient } from '@kbn/core-usage-data-base-server-internal'; -import type { Logger, LogLevelId } from '@kbn/logging'; import { coreUsageStatsClientMock, coreUsageDataServiceMock, @@ -28,6 +27,7 @@ import { type InternalSavedObjectsRequestHandlerContext, } from '@kbn/core-saved-objects-server-internal'; import { setupServer, createExportableType } from '@kbn/core-test-helpers-test-utils'; +import { loggerMock, type MockedLogger } from '@kbn/logging-mocks'; type SetupServerReturn = Awaited>; @@ -41,6 +41,7 @@ describe(`POST ${URL}`, () => { let httpSetup: SetupServerReturn['httpSetup']; let handlerContext: SetupServerReturn['handlerContext']; let savedObjectsClient: ReturnType; + let mockLogger: MockedLogger; const emptyResponse = { saved_objects: [], total: 0, per_page: 0, page: 0 }; const mockIndexPattern = { @@ -57,20 +58,10 @@ describe(`POST ${URL}`, () => { references: [], managed: false, }; - const mockLogger: jest.Mocked = { - debug: jest.fn(), - info: jest.fn(), - error: jest.fn(), - warn: jest.fn(), - trace: jest.fn(), - fatal: jest.fn(), - log: jest.fn(), - isLevelEnabled: jest.fn((level: LogLevelId) => true), - get: jest.fn(() => mockLogger), - }; beforeEach(async () => { ({ server, httpSetup, handlerContext } = await setupServer()); + mockLogger = loggerMock.create(); handlerContext.savedObjects.typeRegistry.getImportableAndExportableTypes.mockReturnValue( allowedTypes.map(createExportableType) ); From ad45ea34583b6a5eb5456390cfc54cdd27d2e22d Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 28 Oct 2024 21:40:45 +0000 Subject: [PATCH 7/7] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../src/import/saved_objects_importer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts index 0c3dd717e4db4..e8c13180bbdaf 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/import/saved_objects_importer.ts @@ -75,7 +75,7 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { importHooks: this.#importHooks, managed, log: this.#log, - }) + }); } public resolveImportErrors({