Skip to content

Commit 72670eb

Browse files
authored
[core.logging] Add response logs to the KP logging system. (#87939)
1 parent 1818dd7 commit 72670eb

File tree

21 files changed

+1393
-67
lines changed

21 files changed

+1393
-67
lines changed

docs/migration/migrate_8_0.asciidoc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,16 @@ for example, `logstash-*`.
5858
==== Responses are never logged by default
5959
*Details:* Previously responses would be logged if either `logging.json` was true, `logging.dest` was specified, or a `TTY` was detected.
6060

61-
*Impact:* To restore the previous behavior, in kibana.yml set `logging.events.response=*`.
61+
*Impact:* To restore the previous behavior, in kibana.yml enable `debug` logs for the `http.server.response` context under `logging.loggers`:
62+
[source,yaml]
63+
-------------------
64+
logging:
65+
loggers:
66+
- context: http.server.response
67+
appenders: [console]
68+
level: debug
69+
-------------------
70+
See https://github.com/elastic/kibana/pull/87939 for more details.
6271

6372
[float]
6473
==== `xpack.security.authProviders` is no longer valid

packages/kbn-legacy-logging/src/get_logging_config.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ export function getLoggingConfiguration(config: LegacyLoggingConfig, opsInterval
2727
});
2828
} else if (config.verbose) {
2929
_.defaults(events, {
30+
error: '*',
3031
log: '*',
31-
// To avoid duplicate logs, we explicitly disable this in verbose
32-
// mode as it is already provided by the new logging config under
33-
// the `metrics.ops` context.
32+
// To avoid duplicate logs, we explicitly disable these in verbose
33+
// mode as they are already provided by the new logging config under
34+
// the `http.server.response` and `metrics.ops` contexts.
3435
ops: '!',
35-
request: '*',
36-
response: '*',
37-
error: '*',
36+
request: '!',
37+
response: '!',
3838
});
3939
} else {
4040
_.defaults(events, {
@@ -75,6 +75,7 @@ export function getLoggingConfiguration(config: LegacyLoggingConfig, opsInterval
7575
},
7676
includes: {
7777
request: ['headers', 'payload'],
78+
response: ['headers', 'payload'],
7879
},
7980
reporters: {
8081
logReporter: [loggerStream],

packages/kbn-legacy-logging/src/log_events.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Side Public License, v 1.
77
*/
88

9+
import type { ResponseObject } from '@hapi/hapi';
910
import { EventData, isEventData } from './metadata';
1011

1112
export interface BaseEvent {
@@ -21,7 +22,8 @@ export interface ResponseEvent extends BaseEvent {
2122
statusCode: number;
2223
path: string;
2324
headers: Record<string, string | string[]>;
24-
responsePayload: string;
25+
responseHeaders: Record<string, string | string[]>;
26+
responsePayload: ResponseObject['source'];
2527
responseTime: string;
2628
query: Record<string, any>;
2729
}

packages/kbn-legacy-logging/src/log_format.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@ import _ from 'lodash';
1212
import queryString from 'query-string';
1313
import numeral from '@elastic/numeral';
1414
import chalk from 'chalk';
15-
// @ts-expect-error missing type def
16-
import stringify from 'json-stringify-safe';
1715
import { inspect } from 'util';
1816

19-
import { applyFiltersToKeys } from './utils';
17+
import { applyFiltersToKeys, getResponsePayloadBytes } from './utils';
2018
import { getLogEventData } from './metadata';
2119
import { LegacyLoggingConfig } from './schema';
2220
import {
2321
AnyEvent,
22+
ResponseEvent,
2423
isResponseEvent,
2524
isOpsEvent,
2625
isErrorEvent,
@@ -70,6 +69,23 @@ export abstract class BaseLogFormat extends Stream.Transform {
7069
next();
7170
}
7271

72+
getContentLength({ responsePayload, responseHeaders }: ResponseEvent): number | undefined {
73+
try {
74+
return getResponsePayloadBytes(responsePayload, responseHeaders);
75+
} catch (e) {
76+
// We intentionally swallow any errors as this information is
77+
// only a nicety for logging purposes, and should not cause the
78+
// server to crash if it cannot be determined.
79+
this.push(
80+
this.format({
81+
type: 'log',
82+
tags: ['warning', 'logging'],
83+
message: `Failed to calculate response payload bytes. [${e}]`,
84+
}) + '\n'
85+
);
86+
}
87+
}
88+
7389
extractAndFormatTimestamp(data: Record<string, any>, format?: string) {
7490
const { timezone } = this.config;
7591
const date = moment(data['@timestamp']);
@@ -100,15 +116,10 @@ export abstract class BaseLogFormat extends Stream.Transform {
100116
referer: source.referer,
101117
};
102118

103-
const contentLength =
104-
event.responsePayload === 'object'
105-
? stringify(event.responsePayload).length
106-
: String(event.responsePayload).length;
107-
108119
data.res = {
109120
statusCode: event.statusCode,
110121
responseTime: event.responseTime,
111-
contentLength,
122+
contentLength: this.getContentLength(event),
112123
};
113124

114125
const query = queryString.stringify(event.query, { sort: false });
@@ -122,7 +133,9 @@ export abstract class BaseLogFormat extends Stream.Transform {
122133
data.message += levelColor(data.res.statusCode);
123134
data.message += ' ';
124135
data.message += chalk.gray(data.res.responseTime + 'ms');
125-
data.message += chalk.gray(' - ' + numeral(contentLength).format('0.0b'));
136+
if (data.res.contentLength) {
137+
data.message += chalk.gray(' - ' + numeral(data.res.contentLength).format('0.0b'));
138+
}
126139
} else if (isOpsEvent(event)) {
127140
_.defaults(data, _.pick(event, ['pid', 'os', 'proc', 'load']));
128141
data.message = chalk.gray('memory: ');

packages/kbn-legacy-logging/src/log_format_json.test.ts

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,30 +39,45 @@ describe('KbnLoggerJsonFormat', () => {
3939
expect(message).toBe('undefined');
4040
});
4141

42-
it('response', async () => {
43-
const event = {
44-
...makeEvent('response'),
45-
statusCode: 200,
46-
contentLength: 800,
47-
responseTime: 12000,
48-
method: 'GET',
49-
path: '/path/to/resource',
50-
responsePayload: '1234567879890',
51-
source: {
52-
remoteAddress: '127.0.0.1',
53-
userAgent: 'Test Thing',
54-
referer: 'elastic.co',
55-
},
56-
};
57-
const result = await createPromiseFromStreams<string>([createListStream([event]), format]);
58-
const { type, method, statusCode, message, req } = JSON.parse(result);
59-
60-
expect(type).toBe('response');
61-
expect(method).toBe('GET');
62-
expect(statusCode).toBe(200);
63-
expect(message).toBe('GET /path/to/resource 200 12000ms - 13.0B');
64-
expect(req.remoteAddress).toBe('127.0.0.1');
65-
expect(req.userAgent).toBe('Test Thing');
42+
describe('response', () => {
43+
it('handles a response object', async () => {
44+
const event = {
45+
...makeEvent('response'),
46+
statusCode: 200,
47+
contentLength: 800,
48+
responseTime: 12000,
49+
method: 'GET',
50+
path: '/path/to/resource',
51+
responsePayload: '1234567879890',
52+
source: {
53+
remoteAddress: '127.0.0.1',
54+
userAgent: 'Test Thing',
55+
referer: 'elastic.co',
56+
},
57+
};
58+
const result = await createPromiseFromStreams<string>([createListStream([event]), format]);
59+
const { type, method, statusCode, message, req } = JSON.parse(result);
60+
61+
expect(type).toBe('response');
62+
expect(method).toBe('GET');
63+
expect(statusCode).toBe(200);
64+
expect(message).toBe('GET /path/to/resource 200 12000ms - 13.0B');
65+
expect(req.remoteAddress).toBe('127.0.0.1');
66+
expect(req.userAgent).toBe('Test Thing');
67+
});
68+
69+
it('leaves payload size empty if not available', async () => {
70+
const event = {
71+
...makeEvent('response'),
72+
statusCode: 200,
73+
responseTime: 12000,
74+
method: 'GET',
75+
path: '/path/to/resource',
76+
responsePayload: null,
77+
};
78+
const result = await createPromiseFromStreams<string>([createListStream([event]), format]);
79+
expect(JSON.parse(result).message).toBe('GET /path/to/resource 200 12000ms');
80+
});
6681
});
6782

6883
it('ops', async () => {
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import mockFs from 'mock-fs';
10+
import { createReadStream } from 'fs';
11+
12+
import { getResponsePayloadBytes } from './get_payload_size';
13+
14+
describe('getPayloadSize', () => {
15+
describe('handles Buffers', () => {
16+
test('with ascii characters', () => {
17+
const payload = 'heya';
18+
const result = getResponsePayloadBytes(Buffer.from(payload));
19+
expect(result).toBe(4);
20+
});
21+
22+
test('with special characters', () => {
23+
const payload = '¡hola!';
24+
const result = getResponsePayloadBytes(Buffer.from(payload));
25+
expect(result).toBe(7);
26+
});
27+
});
28+
29+
describe('handles fs streams', () => {
30+
afterEach(() => mockFs.restore());
31+
32+
test('with ascii characters', async () => {
33+
mockFs({ 'test.txt': 'heya' });
34+
const readStream = createReadStream('test.txt');
35+
36+
let data = '';
37+
for await (const chunk of readStream) {
38+
data += chunk;
39+
}
40+
41+
const result = getResponsePayloadBytes(readStream);
42+
expect(result).toBe(Buffer.byteLength(data));
43+
});
44+
45+
test('with special characters', async () => {
46+
mockFs({ 'test.txt': '¡hola!' });
47+
const readStream = createReadStream('test.txt');
48+
49+
let data = '';
50+
for await (const chunk of readStream) {
51+
data += chunk;
52+
}
53+
54+
const result = getResponsePayloadBytes(readStream);
55+
expect(result).toBe(Buffer.byteLength(data));
56+
});
57+
});
58+
59+
describe('handles plain responses', () => {
60+
test('when source is text', () => {
61+
const result = getResponsePayloadBytes('heya');
62+
expect(result).toBe(4);
63+
});
64+
65+
test('when source contains special characters', () => {
66+
const result = getResponsePayloadBytes('¡hola!');
67+
expect(result).toBe(7);
68+
});
69+
70+
test('when source is object', () => {
71+
const payload = { message: 'heya' };
72+
const result = getResponsePayloadBytes(payload);
73+
expect(result).toBe(JSON.stringify(payload).length);
74+
});
75+
});
76+
77+
describe('handles content-length header', () => {
78+
test('always provides content-length header if available', () => {
79+
const headers = { 'content-length': '123' };
80+
const result = getResponsePayloadBytes('heya', headers);
81+
expect(result).toBe(123);
82+
});
83+
84+
test('uses first value when hapi header is an array', () => {
85+
const headers = { 'content-length': ['123', '456'] };
86+
const result = getResponsePayloadBytes(null, headers);
87+
expect(result).toBe(123);
88+
});
89+
90+
test('returns undefined if length is NaN', () => {
91+
const headers = { 'content-length': 'oops' };
92+
const result = getResponsePayloadBytes(null, headers);
93+
expect(result).toBeUndefined();
94+
});
95+
});
96+
97+
test('defaults to undefined', () => {
98+
const result = getResponsePayloadBytes(null);
99+
expect(result).toBeUndefined();
100+
});
101+
});
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
import type { ReadStream } from 'fs';
10+
import type { ResponseObject } from '@hapi/hapi';
11+
12+
const isBuffer = (obj: unknown): obj is Buffer => Buffer.isBuffer(obj);
13+
const isObject = (obj: unknown): obj is Record<string, unknown> =>
14+
typeof obj === 'object' && obj !== null;
15+
const isFsReadStream = (obj: unknown): obj is ReadStream =>
16+
typeof obj === 'object' && obj !== null && 'bytesRead' in obj;
17+
const isString = (obj: unknown): obj is string => typeof obj === 'string';
18+
19+
/**
20+
* Attempts to determine the size (in bytes) of a hapi/good
21+
* responsePayload based on the payload type. Falls back to
22+
* `undefined` if the size cannot be determined.
23+
*
24+
* This is similar to the implementation in `core/server/http/logging`,
25+
* however it uses more duck typing as we do not have access to the
26+
* entire hapi request object like we do in the HttpServer.
27+
*
28+
* @param headers responseHeaders from hapi/good event
29+
* @param payload responsePayload from hapi/good event
30+
*
31+
* @internal
32+
*/
33+
export function getResponsePayloadBytes(
34+
payload: ResponseObject['source'],
35+
headers: Record<string, any> = {}
36+
): number | undefined {
37+
const contentLength = headers['content-length'];
38+
if (contentLength) {
39+
const val = parseInt(
40+
// hapi response headers can be `string | string[]`, so we need to handle both cases
41+
Array.isArray(contentLength) ? String(contentLength) : contentLength,
42+
10
43+
);
44+
return !isNaN(val) ? val : undefined;
45+
}
46+
47+
if (isBuffer(payload)) {
48+
return payload.byteLength;
49+
}
50+
51+
if (isFsReadStream(payload)) {
52+
return payload.bytesRead;
53+
}
54+
55+
if (isString(payload)) {
56+
return Buffer.byteLength(payload);
57+
}
58+
59+
if (isObject(payload)) {
60+
return Buffer.byteLength(JSON.stringify(payload));
61+
}
62+
63+
return undefined;
64+
}

packages/kbn-legacy-logging/src/utils/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@
77
*/
88

99
export { applyFiltersToKeys } from './apply_filters_to_keys';
10+
export { getResponsePayloadBytes } from './get_payload_size';

0 commit comments

Comments
 (0)