From 56fa1f1e228cb5fa391345917c13bf6502d72440 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Tue, 16 Nov 2021 15:22:45 +0100 Subject: [PATCH] Enforce "LogRecord.meta: LogMeta" type (#118334) (#118686) * use LogRecord.meta: LogMeta * refactor other types * fix meta overrides tracing props test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- packages/kbn-logging/src/log_record.ts | 5 +- .../rewrite/policies/meta/meta_policy.test.ts | 58 +++++++++---------- .../logging/layouts/json_layout.test.ts | 15 +++-- .../logging/layouts/pattern_layout.test.ts | 2 + 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/packages/kbn-logging/src/log_record.ts b/packages/kbn-logging/src/log_record.ts index ee9ed0d69b7496..a212a50b8c98aa 100644 --- a/packages/kbn-logging/src/log_record.ts +++ b/packages/kbn-logging/src/log_record.ts @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import { LogLevel } from './log_level'; +import type { LogLevel } from './log_level'; +import type { LogMeta } from './log_meta'; /** * Essential parts of every log message. @@ -18,7 +19,7 @@ export interface LogRecord { context: string; message: string; error?: Error; - meta?: { [name: string]: any }; + meta?: LogMeta; pid: number; spanId?: string; traceId?: string; diff --git a/src/core/server/logging/appenders/rewrite/policies/meta/meta_policy.test.ts b/src/core/server/logging/appenders/rewrite/policies/meta/meta_policy.test.ts index f03c5bd9d1f8f2..47799ef5d43357 100644 --- a/src/core/server/logging/appenders/rewrite/policies/meta/meta_policy.test.ts +++ b/src/core/server/logging/appenders/rewrite/policies/meta/meta_policy.test.ts @@ -29,26 +29,27 @@ describe('MetaRewritePolicy', () => { // @ts-expect-error ECS custom meta const log = createLogRecord({ a: 'before' }); const policy = createPolicy('update', [{ path: 'a', value: 'after' }]); + // @ts-expect-error ECS custom meta expect(policy.rewrite(log).meta!.a).toBe('after'); }); it('updates nested properties in LogMeta', () => { - // @ts-expect-error ECS custom meta - const log = createLogRecord({ a: 'before a', b: { c: 'before b.c' }, d: [0, 1] }); + const log = createLogRecord({ + error: { message: 'before b.c' }, + tags: ['0', '1'], + }); const policy = createPolicy('update', [ - { path: 'a', value: 'after a' }, - { path: 'b.c', value: 'after b.c' }, - { path: 'd[1]', value: 2 }, + { path: 'error.message', value: 'after b.c' }, + { path: 'tags[1]', value: '2' }, ]); expect(policy.rewrite(log).meta).toMatchInlineSnapshot(` Object { - "a": "after a", - "b": Object { - "c": "after b.c", + "error": Object { + "message": "after b.c", }, - "d": Array [ - 0, - 2, + "tags": Array [ + "0", + "2", ], } `); @@ -80,14 +81,13 @@ describe('MetaRewritePolicy', () => { it(`does not add properties which don't exist yet`, () => { const policy = createPolicy('update', [ - { path: 'a.b', value: 'foo' }, - { path: 'a.c', value: 'bar' }, + { path: 'error.message', value: 'foo' }, + { path: 'error.id', value: 'bar' }, ]); - // @ts-expect-error ECS custom meta - const log = createLogRecord({ a: { b: 'existing meta' } }); + const log = createLogRecord({ error: { message: 'existing meta' } }); const { meta } = policy.rewrite(log); - expect(meta!.a.b).toBe('foo'); - expect(meta!.a.c).toBeUndefined(); + expect(meta?.error?.message).toBe('foo'); + expect(meta?.error?.id).toBeUndefined(); }); it('does not touch anything outside of LogMeta', () => { @@ -110,22 +110,19 @@ describe('MetaRewritePolicy', () => { describe('mode: remove', () => { it('removes existing properties in LogMeta', () => { - // @ts-expect-error ECS custom meta - const log = createLogRecord({ a: 'goodbye' }); - const policy = createPolicy('remove', [{ path: 'a' }]); - expect(policy.rewrite(log).meta!.a).toBeUndefined(); + const log = createLogRecord({ error: { message: 'before' } }); + const policy = createPolicy('remove', [{ path: 'error' }]); + expect(policy.rewrite(log).meta?.error).toBeUndefined(); }); it('removes nested properties in LogMeta', () => { - // @ts-expect-error ECS custom meta - const log = createLogRecord({ a: 'a', b: { c: 'b.c' }, d: [0, 1] }); - const policy = createPolicy('remove', [{ path: 'b.c' }, { path: 'd[1]' }]); + const log = createLogRecord({ error: { message: 'reason' }, tags: ['0', '1'] }); + const policy = createPolicy('remove', [{ path: 'error.message' }, { path: 'tags[1]' }]); expect(policy.rewrite(log).meta).toMatchInlineSnapshot(` Object { - "a": "a", - "b": Object {}, - "d": Array [ - 0, + "error": Object {}, + "tags": Array [ + "0", undefined, ], } @@ -133,12 +130,11 @@ describe('MetaRewritePolicy', () => { }); it('has no effect if property does not exist', () => { - // @ts-expect-error ECS custom meta - const log = createLogRecord({ a: 'a' }); + const log = createLogRecord({ error: {} }); const policy = createPolicy('remove', [{ path: 'b' }]); expect(policy.rewrite(log).meta).toMatchInlineSnapshot(` Object { - "a": "a", + "error": Object {}, } `); }); diff --git a/src/core/server/logging/layouts/json_layout.test.ts b/src/core/server/logging/layouts/json_layout.test.ts index 84546f777ed0b8..d3bf2eab473a4d 100644 --- a/src/core/server/logging/layouts/json_layout.test.ts +++ b/src/core/server/logging/layouts/json_layout.test.ts @@ -98,6 +98,7 @@ test('`format()` correctly formats record with meta-data', () => { timestamp, pid: 5355, meta: { + // @ts-expect-error ECS custom meta version: { from: 'v7', to: 'v8', @@ -140,6 +141,7 @@ test('`format()` correctly formats error record with meta-data', () => { timestamp, pid: 5355, meta: { + // @ts-expect-error ECS custom meta version: { from: 'v7', to: 'v8', @@ -182,6 +184,7 @@ test('format() meta can merge override logs', () => { pid: 3, meta: { log: { + // @ts-expect-error ECS custom meta kbn_custom_field: 'hello', }, }, @@ -213,6 +216,7 @@ test('format() meta can not override message', () => { context: 'bar', pid: 3, meta: { + // @ts-expect-error cannot override message message: 'baz', }, }) @@ -242,7 +246,8 @@ test('format() meta can not override ecs version', () => { context: 'bar', pid: 3, meta: { - message: 'baz', + // @ts-expect-error cannot override ecs version + ecs: 1, }, }) ) @@ -272,6 +277,7 @@ test('format() meta can not override logger or level', () => { pid: 3, meta: { log: { + // @ts-expect-error cannot override log.level level: 'IGNORE', logger: 'me', }, @@ -303,6 +309,7 @@ test('format() meta can not override timestamp', () => { context: 'bar', pid: 3, meta: { + // @ts-expect-error cannot override @timestamp '@timestamp': '2099-02-01T09:30:22.011-05:00', }, }) @@ -332,9 +339,9 @@ test('format() meta can not override tracing properties', () => { context: 'bar', pid: 3, meta: { - span: 'span_override', - trace: 'trace_override', - transaction: 'transaction_override', + span: { id: 'span_override' }, + trace: { id: 'trace_override' }, + transaction: { id: 'transaction_override' }, }, spanId: 'spanId-1', traceId: 'traceId-1', diff --git a/src/core/server/logging/layouts/pattern_layout.test.ts b/src/core/server/logging/layouts/pattern_layout.test.ts index bb66722d4c2d09..22495736227b3f 100644 --- a/src/core/server/logging/layouts/pattern_layout.test.ts +++ b/src/core/server/logging/layouts/pattern_layout.test.ts @@ -118,6 +118,7 @@ test('`format()` correctly formats record with meta data.', () => { timestamp, pid: 5355, meta: { + // @ts-expect-error not valid ECS field from: 'v7', to: 'v8', }, @@ -177,6 +178,7 @@ test('`format()` allows specifying pattern with meta.', () => { to: 'v8', }, }; + // @ts-expect-error not valid ECS field expect(layout.format(record)).toBe('context-{"from":"v7","to":"v8"}-message'); });