Skip to content

Commit

Permalink
basic enhancements for import logging (elastic#196056)
Browse files Browse the repository at this point in the history
## Summary
Hello, this is a follow up PR to elastic#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 <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Alejandro Fernández Haro <[email protected]>
(cherry picked from commit 45543a1)
  • Loading branch information
kyracho committed Nov 1, 2024
1 parent 014478f commit 9a1e3fa
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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: [],
Expand Down Expand Up @@ -72,7 +75,6 @@ describe('#importSavedObjectsFromStream', () => {
let savedObjectsClient: jest.Mocked<SavedObjectsClientContract>;
let typeRegistry: jest.Mocked<ISavedObjectTypeRegistry>;
const namespace = 'some-namespace';

const setupOptions = ({
createNewCopies = false,
getTypeImpl = (type: string) =>
Expand Down Expand Up @@ -102,6 +104,7 @@ describe('#importSavedObjectsFromStream', () => {
createNewCopies,
importHooks,
managed,
log: logger,
};
};
const createObject = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
ISavedObjectTypeRegistry,
SavedObjectsImportHook,
} from '@kbn/core-saved-objects-server';
import type { Logger } from '@kbn/logging';
import {
checkReferenceOrigins,
validateReferences,
Expand Down Expand Up @@ -59,6 +60,7 @@ export interface ImportSavedObjectsOptions {
* If provided, Kibana will apply the given option to the `managed` property.
*/
managed?: boolean;
log: Logger;
}

/**
Expand All @@ -79,7 +81,11 @@ export async function importSavedObjectsFromStream({
refresh,
compatibilityMode,
managed,
log,
}: ImportSavedObjectsOptions): Promise<SavedObjectsImportResponse> {
log.debug(
`Importing with overwrite ${overwrite ? 'enabled' : 'disabled'} and size limit ${objectLimit}`
);
let errorAccumulator: SavedObjectsImportFailure[] = [];
const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name);

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export class SavedObjectsImporter implements ISavedObjectsImporter {
compatibilityMode,
managed,
}: SavedObjectsImportOptions): Promise<SavedObjectsImportResponse> {
this.#log.debug('Starting the import process');
return importSavedObjectsFromStream({
readStream,
createNewCopies,
Expand All @@ -75,6 +74,7 @@ export class SavedObjectsImporter implements ISavedObjectsImporter {
typeRegistry: this.#typeRegistry,
importHooks: this.#importHooks,
managed,
log: this.#log,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<ReturnType<typeof setupServer>>;

Expand All @@ -41,6 +41,7 @@ describe(`POST ${URL}`, () => {
let httpSetup: SetupServerReturn['httpSetup'];
let handlerContext: SetupServerReturn['handlerContext'];
let savedObjectsClient: ReturnType<typeof savedObjectsClientMock.create>;
let mockLogger: MockedLogger;

const emptyResponse = { saved_objects: [], total: 0, per_page: 0, page: 0 };
const mockIndexPattern = {
Expand All @@ -57,20 +58,10 @@ describe(`POST ${URL}`, () => {
references: [],
managed: false,
};
const mockLogger: jest.Mocked<Logger> = {
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)
);
Expand Down

0 comments on commit 9a1e3fa

Please sign in to comment.