From 3d6ac9d46a64d7f14acd75523706aa2bd33c2aef Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Wed, 15 Nov 2023 19:27:52 +0100 Subject: [PATCH] [json layout] use json representation of `meta` when available (#171310) ## Summary Use the json representation (`toJSON`) of the log's `meta` when merging the message and the meta, if possible. --- .../src/layouts/json_layout.test.ts | 87 +++++++++++++++++ .../src/layouts/json_layout.ts | 8 +- .../elasticsearch/error_logging.test.ts | 94 +++++++++++++++++++ 3 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/core/server/integration_tests/elasticsearch/error_logging.test.ts diff --git a/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.test.ts b/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.test.ts index dfa2144c28015..60b415b1b2a2b 100644 --- a/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.test.ts +++ b/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.test.ts @@ -365,3 +365,90 @@ test('format() meta can not override tracing properties', () => { transaction: { id: 'transaction_override' }, }); }); + +test('format() meta.toJSON() is used if own property', () => { + const layout = new JsonLayout(); + expect( + JSON.parse( + layout.format({ + message: 'foo', + timestamp, + level: LogLevel.Debug, + context: 'bar', + pid: 3, + meta: { + server: { + address: 'localhost', + }, + service: { + version: '1', + }, + // @ts-expect-error cannot override @timestamp + toJSON() { + return { + server: { + address: 'localhost', + }, + }; + }, + }, + }) + ) + ).toStrictEqual({ + ecs: { version: expect.any(String) }, + '@timestamp': '2012-02-01T09:30:22.011-05:00', + message: 'foo', + log: { + level: 'DEBUG', + logger: 'bar', + }, + process: { + pid: 3, + }, + server: { + address: 'localhost', + }, + }); +}); + +test('format() meta.toJSON() is used if present on prototype', () => { + class SomeClass { + foo: string = 'bar'; + hello: string = 'dolly'; + + toJSON() { + return { + foo: this.foo, + }; + } + } + + const someInstance = new SomeClass(); + + const layout = new JsonLayout(); + expect( + JSON.parse( + layout.format({ + message: 'foo', + timestamp, + level: LogLevel.Debug, + context: 'bar', + pid: 3, + // @ts-expect-error meta is not of the correct type + meta: someInstance, + }) + ) + ).toStrictEqual({ + ecs: { version: expect.any(String) }, + '@timestamp': '2012-02-01T09:30:22.011-05:00', + message: 'foo', + log: { + level: 'DEBUG', + logger: 'bar', + }, + process: { + pid: 3, + }, + foo: 'bar', + }); +}); diff --git a/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.ts b/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.ts index cd41b7675464e..43b906fa8407c 100644 --- a/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.ts +++ b/packages/core/logging/core-logging-server-internal/src/layouts/json_layout.ts @@ -58,7 +58,13 @@ export class JsonLayout implements Layout { trace: traceId ? { id: traceId } : undefined, transaction: transactionId ? { id: transactionId } : undefined, }; - const output = record.meta ? merge({ ...record.meta }, log) : log; + + let output = log; + if (record.meta) { + // @ts-expect-error toJSON not defined on `LogMeta`, but some structured meta can have it defined + const serializedMeta = record.meta.toJSON ? record.meta.toJSON() : { ...record.meta }; + output = merge(serializedMeta, log); + } return JSON.stringify(output); } diff --git a/src/core/server/integration_tests/elasticsearch/error_logging.test.ts b/src/core/server/integration_tests/elasticsearch/error_logging.test.ts new file mode 100644 index 0000000000000..8cd4f5ae41134 --- /dev/null +++ b/src/core/server/integration_tests/elasticsearch/error_logging.test.ts @@ -0,0 +1,94 @@ +/* + * 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. + */ + +import { + createTestServers, + type TestElasticsearchUtils, + type TestKibanaUtils, +} from '@kbn/core-test-helpers-kbn-server'; + +describe('Error logging', () => { + describe('ES client errors', () => { + let mockConsoleLog: jest.SpyInstance; + let esServer: TestElasticsearchUtils; + let kibanaServer: TestKibanaUtils; + + beforeAll(async () => { + mockConsoleLog = jest.spyOn(global.console, 'log'); + + const { startES, startKibana } = createTestServers({ + adjustTimeout: jest.setTimeout, + settings: { + kbn: { + logging: { + appenders: { + 'console-json': { + type: 'console', + layout: { + type: 'json', + }, + }, + }, + loggers: [{ name: 'console-json', appenders: ['console-json'], level: 'debug' }], + }, + }, + }, + }); + + esServer = await startES(); + kibanaServer = await startKibana(); + }); + + beforeEach(() => { + mockConsoleLog.mockClear(); + }); + + afterAll(async () => { + mockConsoleLog.mockRestore(); + await kibanaServer.stop(); + await esServer.stop(); + }); + + it('logs errors following the expected pattern for the json layout', async () => { + const esClient = kibanaServer.coreStart.elasticsearch.client.asInternalUser; + const logger = kibanaServer.root.logger.get('console-json'); + + try { + await esClient.search({ + index: '.kibana', + // @ts-expect-error yes this is invalid + query: { someInvalidQuery: { foo: 'bar' } }, + }); + expect('should have thrown').toEqual('but it did not'); + } catch (e) { + logger.info('logging elasticsearch error', e); + + const calls = mockConsoleLog.mock.calls; + const ourCall = calls + .map((call) => call[0]) + .find((call) => call.includes('logging elasticsearch error')); + + expect(JSON.parse(ourCall)).toEqual({ + '@timestamp': expect.any(String), + ecs: { + version: expect.any(String), + }, + log: { + level: 'INFO', + logger: 'console-json', + }, + message: 'logging elasticsearch error', + name: 'ResponseError', + process: { + pid: expect.any(Number), + }, + }); + } + }); + }); +});