Skip to content

Commit 2e7e920

Browse files
ArtemHoruzhenkoArtemHoruzhenko
authored
Feature/ri 6205 enhance logs (#4521)
* RI-6205 enhance logs to be more useful * RI-6205 address PR comments
1 parent b3b1657 commit 2e7e920

File tree

7 files changed

+245
-91
lines changed

7 files changed

+245
-91
lines changed

‎redisinsight/api/config/default.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ export default {
217217
: true,
218218
pipelineSummaryLimit:
219219
parseInt(process.env.RI_LOGGER_PIPELINE_SUMMARY_LIMIT, 10) || 5,
220+
logDepthLevel: parseInt(process.env.RI_LOGGER_DEPTH_LEVEL, 10) || 5,
220221
},
221222
plugins: {
222223
stateMaxSize:

‎redisinsight/api/config/logger.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {
66
} from 'nest-winston';
77
import { join } from 'path';
88
import config from 'src/utils/config';
9-
import { prettyFormat, sensitiveDataFormatter } from 'src/utils/logsFormatter';
9+
import {
10+
prepareLogsData,
11+
prettyFileFormat,
12+
} from 'src/utils/logsFormatter';
1013

1114
const PATH_CONFIG = config.get('dir_path');
1215
const LOGGER_CONFIG = config.get('logger');
@@ -17,7 +20,7 @@ if (LOGGER_CONFIG.stdout) {
1720
transportsConfig.push(
1821
new transports.Console({
1922
format: format.combine(
20-
sensitiveDataFormatter({
23+
prepareLogsData({
2124
omitSensitiveData: LOGGER_CONFIG.omitSensitiveData,
2225
}),
2326
format.timestamp(),
@@ -42,10 +45,10 @@ if (LOGGER_CONFIG.files) {
4245
filename: 'redisinsight-errors-%DATE%.log',
4346
level: 'error',
4447
format: format.combine(
45-
sensitiveDataFormatter({
48+
prepareLogsData({
4649
omitSensitiveData: LOGGER_CONFIG.omitSensitiveData,
4750
}),
48-
prettyFormat,
51+
prettyFileFormat,
4952
),
5053
}),
5154
);
@@ -57,10 +60,10 @@ if (LOGGER_CONFIG.files) {
5760
maxFiles: '7d',
5861
filename: 'redisinsight-%DATE%.log',
5962
format: format.combine(
60-
sensitiveDataFormatter({
63+
prepareLogsData({
6164
omitSensitiveData: LOGGER_CONFIG.omitSensitiveData,
6265
}),
63-
prettyFormat,
66+
prettyFileFormat,
6467
),
6568
}),
6669
);

‎redisinsight/api/src/common/logger/app-logger.spec.ts‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,7 @@ describe('AppLogger', () => {
7171
message: 'Test message',
7272
context: null,
7373
data: [error2],
74-
error: {
75-
message: error1.message,
76-
response: undefined,
77-
stack: error1.stack,
78-
},
74+
error: error1,
7975
});
8076
},
8177
);
@@ -95,14 +91,7 @@ describe('AppLogger', () => {
9591
message: 'Test message',
9692
context: null,
9793
data: undefined,
98-
error: {
99-
message: error1.message,
100-
response: {
101-
status: 500,
102-
data: 'Internal server error',
103-
},
104-
stack: error1.stack,
105-
},
94+
error: error1,
10695
});
10796
},
10897
);
@@ -181,11 +170,7 @@ describe('AppLogger', () => {
181170
},
182171
sessionMetadata: clientMetadata.sessionMetadata,
183172
data: [{ foo: 'bar' }],
184-
error: {
185-
message: error.message,
186-
stack: error.stack,
187-
response: undefined,
188-
},
173+
error,
189174
});
190175
expect(optionalParams).toEqual(optionalParamsOriginal);
191176
},

‎redisinsight/api/src/common/logger/app-logger.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,14 @@ export class AppLogger implements LoggerService {
3838
* Note: args array might be mutated
3939
* @param args
4040
*/
41-
static getError(args: ErrorOrMeta[] = []): void | {} {
41+
static getError(args: ErrorOrMeta[] = []): Error {
4242
let error = null;
4343
const index = args.findIndex((arg) => arg instanceof Error);
4444
if (index > -1) {
4545
[error] = args.splice(index, 1);
4646
}
4747

48-
if (error) {
49-
return {
50-
message: error.message,
51-
stack: error.stack,
52-
response: error.response,
53-
};
54-
}
55-
56-
return undefined;
48+
return error || undefined;
5749
}
5850

5951
/**

‎redisinsight/api/src/modules/cloud/auth/cloud-auth.service.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,13 @@ export class CloudAuthService {
122122

123123
return CloudAuthStrategy.generateAuthUrl(authRequest).toString();
124124
} catch (e) {
125+
this.logger.error('Unable to generate authorization url', e);
126+
125127
if (e instanceof CloudOauthSsoUnsupportedEmailException) {
126128
throw e;
127129
}
128130

129-
throw new CloudOauthMisconfigurationException();
131+
throw new CloudOauthMisconfigurationException(undefined, { cause: e });
130132
}
131133
}
132134

‎redisinsight/api/src/utils/logsFormatter.spec.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import { BadRequestException, NotFoundException } from '@nestjs/common';
2+
import { CloudOauthMisconfigurationException } from 'src/modules/cloud/auth/exceptions';
3+
import { AxiosError, AxiosHeaders } from 'axios';
4+
import { mockSessionMetadata } from 'src/__mocks__';
5+
import { getOriginalErrorCause, sanitizeError, sanitizeErrors } from './logsFormatter';
6+
7+
const simpleError = new Error('Original error');
8+
simpleError['some'] = 'field';
9+
const errorWithCause = new NotFoundException('Not found', { cause: simpleError });
10+
const errorWithCauseDepth2 = new BadRequestException('Bad req', { cause: errorWithCause });
11+
const errorWithCauseDepth3 = new CloudOauthMisconfigurationException('Misconfigured', { cause: errorWithCauseDepth2 });
12+
const axiosError = new AxiosError(
13+
'Request failed with status code 404',
14+
'NOT_FOUND',
15+
{
16+
method: 'get',
17+
url: '/test-endpoint',
18+
headers: AxiosHeaders.prototype,
19+
data: null,
20+
},
21+
null,
22+
{
23+
status: 404,
24+
statusText: 'Not Found',
25+
headers: {},
26+
config: {
27+
headers: AxiosHeaders.prototype,
28+
},
29+
data: {
30+
message: 'Resource not found',
31+
},
32+
},
33+
);
34+
35+
const mockLogData: any = {
36+
sessionMetadata: mockSessionMetadata,
37+
error: errorWithCauseDepth3,
38+
data: [
39+
errorWithCauseDepth2,
40+
{
41+
any: [
42+
'other',
43+
{
44+
possible: 'data',
45+
with: [
46+
'nested',
47+
'structure',
48+
errorWithCause,
49+
{
50+
error: simpleError,
51+
},
52+
],
53+
},
54+
],
55+
},
56+
],
57+
};
58+
mockLogData.data.push({ circular: mockLogData.data });
59+
60+
describe('logsFormatter', () => {
61+
describe('getOriginalErrorCause', () => {
62+
it('should return last cause in the chain', () => {
63+
expect(getOriginalErrorCause(errorWithCauseDepth3)).toEqual(simpleError);
64+
});
65+
66+
it('should return undefined if input is not an Error instance', () => {
67+
expect(getOriginalErrorCause({ cause: simpleError })).toEqual(undefined);
68+
});
69+
70+
it('should not fail if input is not specified', () => {
71+
expect(getOriginalErrorCause(undefined)).toEqual(undefined);
72+
});
73+
});
74+
75+
describe('sanitizeError', () => {
76+
it('should sanitize simple error and return only message', () => {
77+
expect(sanitizeError(simpleError, { omitSensitiveData: true })).toEqual({
78+
type: 'Error',
79+
message: simpleError.message,
80+
});
81+
});
82+
83+
it('should sanitize simple error and return message (with stack)', () => {
84+
expect(sanitizeError(simpleError)).toEqual({
85+
type: 'Error',
86+
message: simpleError.message,
87+
stack: simpleError.stack,
88+
});
89+
});
90+
91+
it('should return sanitized object with a single original cause for nested errors', () => {
92+
expect(sanitizeError(errorWithCauseDepth3, { omitSensitiveData: true })).toEqual({
93+
type: 'CloudOauthMisconfigurationException',
94+
message: errorWithCauseDepth3.message,
95+
cause: {
96+
type: 'Error',
97+
message: simpleError.message,
98+
},
99+
});
100+
});
101+
102+
it('should return sanitized object with a single original cause for nested errors (with stack)', () => {
103+
expect(sanitizeError(errorWithCauseDepth3)).toEqual({
104+
type: 'CloudOauthMisconfigurationException',
105+
message: errorWithCauseDepth3.message,
106+
stack: errorWithCauseDepth3.stack,
107+
cause: {
108+
type: 'Error',
109+
message: simpleError.message,
110+
stack: simpleError.stack,
111+
},
112+
});
113+
});
114+
115+
it('should sanitize axios error and return only message when sensitive data is omitted', () => {
116+
expect(sanitizeError(axiosError, { omitSensitiveData: true })).toEqual({
117+
type: 'AxiosError',
118+
message: axiosError.message,
119+
});
120+
});
121+
});
122+
123+
describe('sanitizeErrors', () => {
124+
it('should sanitize all errors and replace circular dependencies', () => {
125+
expect(sanitizeErrors(mockLogData, { omitSensitiveData: true })).toEqual({
126+
sessionMetadata: mockSessionMetadata,
127+
error: {
128+
type: 'CloudOauthMisconfigurationException',
129+
message: 'Misconfigured',
130+
cause: {
131+
message: 'Original error',
132+
type: 'Error',
133+
},
134+
},
135+
data: [
136+
{
137+
type: 'BadRequestException',
138+
message: 'Bad req',
139+
cause: {
140+
type: 'Error',
141+
message: 'Original error',
142+
},
143+
},
144+
{
145+
any: [
146+
'other',
147+
{
148+
possible: 'data',
149+
with: [
150+
'nested',
151+
'structure',
152+
{
153+
cause: {
154+
message: 'Original error',
155+
type: 'Error',
156+
},
157+
message: 'Not found',
158+
type: 'NotFoundException',
159+
},
160+
{
161+
error: {
162+
message: 'Original error',
163+
type: 'Error',
164+
},
165+
},
166+
],
167+
},
168+
],
169+
},
170+
{
171+
circular: '[Circular]',
172+
},
173+
],
174+
});
175+
});
176+
});
177+
});

0 commit comments

Comments
 (0)