From 9a1e3fa015f377060d0a5d83e53b16e4847ed750 Mon Sep 17 00:00:00 2001 From: Kyra Cho Date: Thu, 31 Oct 2024 17:50:21 -0700 Subject: [PATCH] basic enhancements for import logging (#196056) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Hello, this is a follow up PR to #192234 . The previous PR added simplistic logging to the saved objects importer. The goal now is to enhance the logs with information on the saved objects being imported, how they are imported, and by displaying any errors. #### `import_saved_objects.ts`: - Logs specific types being imported - Logs size limit and overwrite status - Logs Success/Fail messages #### Changes to `saved_objects_importer.ts`: - Passes the logger to `importSavedObjectsFromStream` - Removes "starting import" #### Changes to `import_saved_objects.test.ts`: - Updates it for the new logger parameter #### Changes to `import.test.ts`: - Uses the mock logger provided by core, instead of using a custom one ### Checklist - [x] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Elastic Machine Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alejandro Fernández Haro (cherry picked from commit 45543a12a6ce7c32d0a14b45a24eccceca23d9d0) --- .../src/import/import_saved_objects.test.ts | 5 +++- .../src/import/import_saved_objects.ts | 23 ++++++++++++++++++- .../src/import/saved_objects_importer.ts | 2 +- .../saved_objects/routes/import.test.ts | 15 +++--------- 4 files changed, 30 insertions(+), 15 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 d8c2b0b25874f..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,7 +75,6 @@ describe('#importSavedObjectsFromStream', () => { let savedObjectsClient: jest.Mocked; let typeRegistry: jest.Mocked; const namespace = 'some-namespace'; - const setupOptions = ({ createNewCopies = false, getTypeImpl = (type: string) => @@ -102,6 +104,7 @@ describe('#importSavedObjectsFromStream', () => { createNewCopies, importHooks, managed, + log: logger, }; }; const createObject = ({ 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..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 @@ -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,11 @@ 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 +96,11 @@ 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 +208,17 @@ 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 f990eb13c435b..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 @@ -62,7 +62,6 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { compatibilityMode, managed, }: SavedObjectsImportOptions): Promise { - this.#log.debug('Starting the import process'); return importSavedObjectsFromStream({ readStream, createNewCopies, @@ -75,6 +74,7 @@ export class SavedObjectsImporter implements ISavedObjectsImporter { typeRegistry: this.#typeRegistry, importHooks: this.#importHooks, managed, + log: this.#log, }); } 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) );