Skip to content

Commit 59d1e56

Browse files
maor-rozenfeldCopilotbigfluffycookiealexandrudanpop
authored
Fix log indexing (#1593)
Fixes OPS-2843. - Improve parsing of the error object, especially ApplicationError - Refactor the log-cleaner to multiple functions - Fix a NaN error for ResponseTime field --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Leyla <54935347+bigfluffycookie@users.noreply.github.com> Co-authored-by: Alexandru-Dan Pop <alexandrudanpop@gmail.com>
1 parent 242bdc8 commit 59d1e56

2 files changed

Lines changed: 126 additions & 51 deletions

File tree

‎packages/server/shared/src/lib/logger/log-cleaner.ts‎

Lines changed: 62 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1-
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-dynamic-delete */
1+
/* eslint-disable @typescript-eslint/no-explicit-any */
2+
3+
import { ApplicationError } from '@openops/shared';
4+
25
export const maxFieldLength = 2048;
36

4-
export const truncate = (value: string, maxLength: number = maxFieldLength) => {
5-
return value.length > maxLength
7+
export const truncate = (
8+
value: string | undefined,
9+
maxLength: number = maxFieldLength,
10+
) => {
11+
return value && value.length > maxLength
612
? `${value.substring(0, maxLength - 3)}...`
713
: value;
814
};
@@ -18,57 +24,76 @@ export const cleanLogEvent = (logEvent: any) => {
1824

1925
const eventData: any = {};
2026

27+
if (logEvent.event instanceof Error) {
28+
logEvent.event = { error: logEvent.event };
29+
}
30+
2131
for (const key in logEvent.event) {
2232
const value = logEvent.event[key];
2333
if (value === null || value === undefined) {
2434
continue;
2535
}
2636

27-
delete eventData[key];
28-
29-
if (key == 'res' && value && value.raw) {
30-
const rawResponse = value.raw;
31-
eventData.requestMethod = rawResponse.req.method;
32-
eventData.requestPath = truncate(rawResponse.req.url);
33-
eventData.statusCode = rawResponse.statusCode;
34-
const responseTime = parseFloat(logEvent.event.responseTime).toFixed();
35-
eventData.responseTime = responseTime;
36-
logEvent[
37-
'message'
38-
] = `Request completed [${eventData.requestMethod} ${eventData.requestPath} ${eventData.statusCode} ${responseTime}ms]`;
39-
logEvent['level'] = 'debug';
40-
continue;
41-
}
42-
43-
if (typeof value === 'object') {
37+
if (key === 'res' && value && value.raw) {
38+
extractRequestFields(value, eventData, logEvent);
39+
} else if (value instanceof Error) {
40+
extractErrorFields(key, value, eventData, logEvent);
41+
} else if (typeof value === 'number') {
42+
eventData[key] = Math.round(value * 100) / 100;
43+
} else if (typeof value === 'object') {
4444
try {
4545
eventData[key] = truncate(JSON.stringify(value));
4646
} catch (error) {
4747
eventData[key] = `Logger error - could not stringify object. ${error}`;
4848
}
49-
continue;
49+
} else {
50+
eventData[key] = truncate(value);
5051
}
52+
}
5153

52-
if (typeof value === 'number') {
53-
// Max 2 decimal points
54-
eventData[key] = Math.round(value * 100) / 100;
55-
continue;
56-
}
54+
logEvent.event = eventData;
55+
return logEvent;
56+
};
57+
58+
function extractRequestFields(value: any, eventData: any, logEvent: any) {
59+
const rawResponse = value.raw;
60+
eventData.requestMethod = rawResponse.req.method;
61+
eventData.requestPath = truncate(rawResponse.req.url);
62+
eventData.statusCode = rawResponse.statusCode;
63+
const responseTime = parseFloat(logEvent.event.responseTime);
5764

58-
eventData[key] = truncate(value);
65+
if (!isNaN(responseTime)) {
66+
eventData.responseTime = responseTime.toFixed();
5967
}
6068

61-
if (logEvent.event instanceof Error) {
62-
const { stack, message, name, ...context } = logEvent.event;
63-
eventData.stack = truncate(stack, 2000);
64-
eventData.name = name;
69+
logEvent['message'] = `Request completed [${eventData.requestMethod} ${
70+
eventData.requestPath
71+
} ${eventData.statusCode} ${eventData.responseTime ?? 0}ms]`;
72+
logEvent['level'] = 'debug';
73+
}
74+
75+
function extractErrorFields(
76+
key: string,
77+
value: Error | ApplicationError,
78+
eventData: any,
79+
logEvent: any,
80+
) {
81+
const errorKey = key === 'err' ? 'error' : key;
82+
const { stack, message, name, ...context } = value;
83+
eventData[errorKey + 'Stack'] = truncate(stack);
84+
if (message) {
85+
eventData[errorKey + 'Message'] = truncate(message);
6586
if (!logEvent.message) {
6687
logEvent.message = truncate(message);
67-
} else {
68-
eventData.message = truncate(message);
6988
}
70-
Object.assign(eventData, context);
7189
}
72-
logEvent.event = eventData;
73-
return logEvent;
74-
};
90+
eventData[errorKey + 'Name'] = truncate(name);
91+
if (value instanceof ApplicationError) {
92+
eventData[errorKey + 'Code'] = truncate(value.error.code);
93+
eventData[errorKey + 'Params'] = truncate(
94+
JSON.stringify(value.error.params),
95+
);
96+
} else if (context && Object.keys(context).length) {
97+
eventData[errorKey + 'Context'] = truncate(JSON.stringify(context));
98+
}
99+
}

‎packages/server/shared/test/log-cleaner.test.ts‎

Lines changed: 64 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { ApplicationError, ErrorCode } from '@openops/shared';
12
import { cleanLogEvent, maxFieldLength } from '../src/lib/logger/log-cleaner';
23

34
// It's over 9000
@@ -150,9 +151,10 @@ describe('log-cleaner', () => {
150151

151152
expect(result).toEqual({
152153
event: {
153-
stack: expect.stringMatching(/^Error: test error\s+at Object/),
154-
name: 'Error',
155-
testContext: 'testContext',
154+
errorStack: expect.stringMatching(/^Error: test error\s+at Object/),
155+
errorName: 'Error',
156+
errorContext: '{"testContext":"testContext"}',
157+
errorMessage: 'test error',
156158
},
157159
message: 'test error',
158160
});
@@ -169,9 +171,9 @@ describe('log-cleaner', () => {
169171
expect(result).toEqual({
170172
message: 'existing message',
171173
event: {
172-
stack: expect.stringMatching(/^Error: test error\s+at Object/),
173-
name: 'Error',
174-
message: 'test error',
174+
errorStack: expect.stringMatching(/^Error: test error\s+at Object/),
175+
errorName: 'Error',
176+
errorMessage: 'test error',
175177
},
176178
});
177179
});
@@ -187,9 +189,9 @@ describe('log-cleaner', () => {
187189

188190
const result = cleanLogEvent(logEvent);
189191

190-
expect(result.event.stack).toHaveLength(2000);
191-
expect(result.event.stack).toMatch(/\.\.\.$/);
192-
expect(result.event.name).toBe('Error');
192+
expect(result.event.errorStack).toHaveLength(2048);
193+
expect(result.event.errorStack).toMatch(/\.\.\.$/);
194+
expect(result.event.errorName).toBe('Error');
193195
expect(result.message).toBe('test error');
194196
});
195197

@@ -209,10 +211,11 @@ describe('log-cleaner', () => {
209211

210212
expect(result).toEqual({
211213
event: {
212-
stack: expect.stringMatching(
214+
errorStack: expect.stringMatching(
213215
/^CustomError: custom error message\s+at Object/,
214216
),
215-
name: 'CustomError',
217+
errorName: 'CustomError',
218+
errorMessage: 'custom error message',
216219
},
217220
message: 'custom error message',
218221
});
@@ -228,10 +231,57 @@ describe('log-cleaner', () => {
228231

229232
expect(result).toEqual({
230233
event: {
231-
stack: expect.stringMatching(/^Error(?::.*)?\n\s+at /),
232-
name: 'Error',
234+
errorStack: expect.stringMatching(/^Error(?::.*)?\n\s+at /),
235+
errorName: 'Error',
233236
},
234-
message: '',
237+
});
238+
});
239+
240+
it('should flatten error context', () => {
241+
const logEvent = {
242+
event: new ApplicationError({
243+
code: ErrorCode.ENTITY_NOT_FOUND,
244+
params: {
245+
message: 'No Flow',
246+
entityType: 'Flow',
247+
entityId: '123',
248+
},
249+
}),
250+
};
251+
const result = cleanLogEvent(logEvent);
252+
253+
expect(result).toEqual({
254+
event: {
255+
errorStack: expect.stringMatching(/^Error(?::.*)?\n\s+at /),
256+
errorName: 'Error',
257+
errorMessage: 'ENTITY_NOT_FOUND',
258+
errorCode: 'ENTITY_NOT_FOUND',
259+
errorParams:
260+
'{"message":"No Flow","entityType":"Flow","entityId":"123"}',
261+
},
262+
message: 'ENTITY_NOT_FOUND',
263+
});
264+
});
265+
266+
it('should flatten error in correct fields by prefix', () => {
267+
const logEvent = {
268+
event: {
269+
networkError: new Error("Can't connect"),
270+
},
271+
level: 'info',
272+
message: 'Completed with an error',
273+
};
274+
275+
const result = cleanLogEvent(logEvent);
276+
277+
expect(result).toEqual({
278+
event: {
279+
networkErrorStack: expect.stringMatching(/^Error(?::.*)?\n\s+at /),
280+
networkErrorName: 'Error',
281+
networkErrorMessage: "Can't connect",
282+
},
283+
level: 'info',
284+
message: 'Completed with an error',
235285
});
236286
});
237287
});

0 commit comments

Comments
 (0)