Skip to content

Commit 06aac53

Browse files
seth-riedelseth-riedel
authored
Merge pull request #4187 from RedisInsight/be/feature/CR-277-v2
CR-277: Change log to debug calls, log additional metadata
2 parents 632ac08 + 117e6bb commit 06aac53

File tree

95 files changed

+841
-463
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

95 files changed

+841
-463
lines changed

‎redisinsight/api/config/logger.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ if (LOGGER_CONFIG.stdout) {
1919
format: format.combine(
2020
sensitiveDataFormatter({ omitSensitiveData: LOGGER_CONFIG.omitSensitiveData }),
2121
format.timestamp(),
22-
nestWinstonModuleUtilities.format.nestLike(),
22+
nestWinstonModuleUtilities.format.nestLike('Redis Insight', {
23+
colors: true,
24+
prettyPrint: true,
25+
processId: true,
26+
appName: true,
27+
}),
2328
),
2429
}),
2530
);

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

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import { plainToClass } from 'class-transformer';
2+
import { cloneDeep } from 'lodash';
3+
import { WinstonModule } from 'nest-winston';
4+
import { AppLogger } from 'src/common/logger/app-logger';
5+
import {
6+
ClientContext,
7+
ClientMetadata,
8+
SessionMetadata,
9+
} from 'src/common/models';
10+
11+
const loggerConfig = {};
12+
const mockWinstonLogger = {
13+
log: jest.fn(),
14+
verbose: jest.fn(),
15+
debug: jest.fn(),
16+
warn: jest.fn(),
17+
error: jest.fn(),
18+
fatal: jest.fn(),
19+
};
20+
const logLevels = Object.keys(mockWinstonLogger);
21+
22+
jest.spyOn(WinstonModule, 'createLogger').mockReturnValue(mockWinstonLogger);
23+
24+
const getSessionMetadata = () => plainToClass(SessionMetadata, {
25+
userId: '123',
26+
sessionId: 'test-session-id',
27+
});
28+
29+
const getClientMetadata = () => plainToClass(ClientMetadata, {
30+
sessionMetadata: getSessionMetadata(),
31+
databaseId: 'db-123',
32+
context: ClientContext.Browser,
33+
uniqueId: 'unique-id',
34+
db: 1,
35+
});
36+
37+
describe('AppLogger', () => {
38+
let logger: AppLogger;
39+
40+
beforeEach(() => {
41+
logger = new AppLogger(loggerConfig);
42+
jest.clearAllMocks();
43+
});
44+
45+
test.each(logLevels)(
46+
'should get context from last optional param if it is a string - logger.%s',
47+
(level) => {
48+
logger[level]('Test message', 'optional arg', 'Test context');
49+
50+
expect(mockWinstonLogger[level]).toHaveBeenCalledTimes(1);
51+
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
52+
message: 'Test message',
53+
context: 'Test context',
54+
data: ['optional arg'],
55+
error: undefined,
56+
});
57+
},
58+
);
59+
60+
test.each(logLevels)(
61+
'should find and separate the first error object if exists - logger.%s',
62+
(level) => {
63+
const error1 = new Error('Test error 1');
64+
const error2 = new Error('Test error 2');
65+
logger[level]('Test message', error1, error2);
66+
67+
expect(mockWinstonLogger[level]).toHaveBeenCalledTimes(1);
68+
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
69+
message: 'Test message',
70+
context: null,
71+
data: [error2],
72+
error: {
73+
message: error1.message,
74+
response: undefined,
75+
stack: error1.stack,
76+
},
77+
});
78+
},
79+
);
80+
81+
test.each(logLevels)(
82+
'should get error response and include it if exists - logger.%s',
83+
(level) => {
84+
const error1 = new Error('Test error 1');
85+
(error1 as Error & { response: unknown }).response = {
86+
status: 500,
87+
data: 'Internal server error',
88+
};
89+
logger[level]('Test message', error1);
90+
91+
expect(mockWinstonLogger[level]).toHaveBeenCalledTimes(1);
92+
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
93+
message: 'Test message',
94+
context: null,
95+
data: undefined,
96+
error: {
97+
message: error1.message,
98+
response: {
99+
status: 500,
100+
data: 'Internal server error',
101+
},
102+
stack: error1.stack,
103+
},
104+
});
105+
},
106+
);
107+
108+
test.each(logLevels)(
109+
'should parse client metadata from optional params - logger.%s',
110+
(level) => {
111+
const clientMetadata = getClientMetadata();
112+
logger[level](
113+
'Test message',
114+
clientMetadata,
115+
{ foo: 'bar' },
116+
'Test context',
117+
);
118+
119+
expect(mockWinstonLogger[level]).toHaveBeenCalledTimes(1);
120+
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
121+
message: 'Test message',
122+
context: 'Test context',
123+
clientMetadata: {
124+
...clientMetadata,
125+
sessionMetadata: undefined,
126+
},
127+
sessionMetadata: clientMetadata.sessionMetadata,
128+
data: [{ foo: 'bar' }],
129+
error: undefined,
130+
});
131+
},
132+
);
133+
134+
test.each(logLevels)(
135+
'should parse session metadata from optional params - logger.%s',
136+
(level) => {
137+
const sessionMetadata = getSessionMetadata();
138+
logger[level](
139+
'Test message',
140+
sessionMetadata,
141+
{ foo: 'bar' },
142+
'Test context',
143+
);
144+
145+
expect(mockWinstonLogger[level]).toHaveBeenCalledTimes(1);
146+
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
147+
message: 'Test message',
148+
context: 'Test context',
149+
sessionMetadata,
150+
data: [{ foo: 'bar' }],
151+
error: undefined,
152+
});
153+
},
154+
);
155+
156+
test.each(logLevels)(
157+
'should not mutate original arguments - logger.%s',
158+
(level) => {
159+
const clientMetadata = getClientMetadata();
160+
161+
const error = new Error('test 123');
162+
const optionalParams = [
163+
clientMetadata,
164+
error,
165+
{ foo: 'bar' },
166+
'Test context',
167+
];
168+
const optionalParamsOriginal = cloneDeep(optionalParams);
169+
170+
logger[level]('Test message', ...optionalParams);
171+
172+
expect(mockWinstonLogger[level]).toHaveBeenCalledTimes(1);
173+
expect(mockWinstonLogger[level]).toHaveBeenCalledWith({
174+
message: 'Test message',
175+
context: 'Test context',
176+
clientMetadata: {
177+
...clientMetadata,
178+
sessionMetadata: undefined,
179+
},
180+
sessionMetadata: clientMetadata.sessionMetadata,
181+
data: [{ foo: 'bar' }],
182+
error: {
183+
message: error.message,
184+
stack: error.stack,
185+
response: undefined,
186+
},
187+
});
188+
expect(optionalParams).toEqual(optionalParamsOriginal);
189+
},
190+
);
191+
});

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

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
import { LoggerService, Injectable } from '@nestjs/common';
2+
import { WinstonModule, WinstonModuleOptions } from 'nest-winston';
3+
import { cloneDeep, isString } from 'lodash';
4+
import { ClientMetadata, SessionMetadata } from 'src/common/models';
5+
6+
type LogMeta = object;
7+
8+
type ErrorOrMeta = Error | LogMeta | string | ClientMetadata | SessionMetadata;
9+
10+
@Injectable()
11+
export class AppLogger implements LoggerService {
12+
private readonly logger: ReturnType<typeof WinstonModule.createLogger>;
13+
14+
constructor(loggerConfig: WinstonModuleOptions) {
15+
this.logger = WinstonModule.createLogger(loggerConfig);
16+
}
17+
18+
/**
19+
* Get context from optional arguments
20+
* If the last argument is a string - it will be handled like a context
21+
* since nest passes the logger context as the last argument
22+
* Note: args array might be mutated
23+
* @param args
24+
*/
25+
static getContext(args: ErrorOrMeta[] = []) {
26+
const lastArg = args?.[args.length - 1];
27+
28+
if (isString(lastArg)) {
29+
return args.pop() as string;
30+
}
31+
32+
return null;
33+
}
34+
35+
/**
36+
* Get an error from the optional arguments
37+
* Will find first entry which is error type
38+
* Note: args array might be mutated
39+
* @param args
40+
*/
41+
static getError(args: ErrorOrMeta[] = []): void | {} {
42+
let error = null;
43+
const index = args.findIndex((arg) => arg instanceof Error);
44+
if (index > -1) {
45+
[error] = args.splice(index, 1);
46+
}
47+
48+
if (error) {
49+
return {
50+
message: error.message,
51+
stack: error.stack,
52+
response: error.response,
53+
};
54+
}
55+
56+
return undefined;
57+
}
58+
59+
/**
60+
* Get clientMetadata and/or sessionMetadata object(s) from args
61+
* Will find first entry of ClientMetadata and get SessionMetadata, from it and return both
62+
* otherwise will find SessionMetadata and return only it
63+
* otherwise will return empty object
64+
* Note: args array might be mutated
65+
* @param args
66+
*/
67+
static getUserMetadata(args: ErrorOrMeta[] = []): {
68+
clientMetadata?: Partial<ClientMetadata>;
69+
sessionMetadata?: SessionMetadata;
70+
} {
71+
// check for client metadata in args
72+
const clientMetadataIndex = args.findIndex(
73+
(arg) => arg instanceof ClientMetadata,
74+
);
75+
if (clientMetadataIndex > -1) {
76+
const [clientMetadata] = args.splice(
77+
clientMetadataIndex,
78+
1,
79+
) as ClientMetadata[];
80+
return {
81+
clientMetadata: {
82+
...clientMetadata,
83+
sessionMetadata: undefined,
84+
},
85+
sessionMetadata: clientMetadata.sessionMetadata,
86+
};
87+
}
88+
89+
// check for session metadata in args
90+
const sessionMetadataIndex = args.findIndex(
91+
(arg) => arg instanceof SessionMetadata,
92+
);
93+
if (sessionMetadataIndex > -1) {
94+
const [sessionMetadata] = args.splice(
95+
sessionMetadataIndex,
96+
1,
97+
) as SessionMetadata[];
98+
return {
99+
sessionMetadata,
100+
};
101+
}
102+
103+
// by default will return empty object
104+
return {};
105+
}
106+
107+
private parseLoggerArgs(message: string, optionalParams: ErrorOrMeta[] = []) {
108+
const optionalParamsCopy = cloneDeep(optionalParams);
109+
const context = AppLogger.getContext(optionalParamsCopy);
110+
const error = AppLogger.getError(optionalParamsCopy);
111+
const userMetadata = AppLogger.getUserMetadata(optionalParamsCopy);
112+
113+
return {
114+
message,
115+
context,
116+
error,
117+
...userMetadata,
118+
data: optionalParamsCopy?.length ? optionalParamsCopy : undefined,
119+
};
120+
}
121+
122+
/**
123+
* Write a 'log' level log.
124+
*/
125+
log(message: string, ...optionalParams: ErrorOrMeta[]) {
126+
this.logger.log(this.parseLoggerArgs(message, optionalParams));
127+
}
128+
129+
/**
130+
* Write a 'fatal' level log.
131+
*/
132+
fatal(message: string, ...optionalParams: ErrorOrMeta[]) {
133+
this.logger.fatal(this.parseLoggerArgs(message, optionalParams));
134+
}
135+
136+
/**
137+
* Write an 'error' level log.
138+
*/
139+
error(message: string, ...optionalParams: ErrorOrMeta[]) {
140+
this.logger.error(this.parseLoggerArgs(message, optionalParams));
141+
}
142+
143+
/**
144+
* Write a 'warn' level log.
145+
*/
146+
warn(message: string, ...optionalParams: ErrorOrMeta[]) {
147+
this.logger.warn(this.parseLoggerArgs(message, optionalParams));
148+
}
149+
150+
/**
151+
* Write a 'debug' level log.
152+
*/
153+
debug?(message: string, ...optionalParams: ErrorOrMeta[]) {
154+
this.logger.debug(this.parseLoggerArgs(message, optionalParams));
155+
}
156+
157+
/**
158+
* Write a 'verbose' level log.
159+
*/
160+
verbose?(message: string, ...optionalParams: ErrorOrMeta[]) {
161+
this.logger.verbose(this.parseLoggerArgs(message, optionalParams));
162+
}
163+
}

‎redisinsight/api/src/exceptions/global-exception.filter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ArgumentsHost, Logger } from '@nestjs/common';
33
import { Request, Response } from 'express';
44

55
export class GlobalExceptionFilter extends BaseExceptionFilter {
6-
private staticServerLogger = new Logger('StaticServerLogger');
6+
private staticServerLogger = new Logger('GlobalExceptionFilter');
77

88
catch(exception: Error, host: ArgumentsHost) {
99
const ctx = host.switchToHttp();

0 commit comments

Comments
 (0)