Skip to content

Commit 7317730

Browse files
committed
feat(FR-1746): add BAILogger to replace with console (#4781)
resolves #4736 (FR-1746) This PR introduces a new structured logging system to replace direct console.log/error calls throughout the codebase. The new system provides: - A centralized logger with different log levels (LOG, DEBUG, INFO, WARN, ERROR) - Context-aware logging with metadata support - Plugin architecture for extending logger behavior (preprocessing, post-processing) - Conditional logging that's disabled in production by default - Method chaining for better developer experience The implementation includes: - New `useBAILogger` hook for React components - `ContextualLogger` class for adding context to log messages - Replacement of console.log/error calls with the new logger across multiple components - The `no-console` rule has been added to ESLint, and it is excluded for `*.test.*` and `*.stories.*` files. This change improves debugging capabilities while providing a foundation for future logging enhancements like remote logging, log filtering, and performance monitoring. **Checklist:** - [x] Documentation - [ ] Minium required manager version - [ ] Specific setting for review (eg., KB link, endpoint or how to setup) - [ ] Minimum requirements to check during review - [ ] Test case(s) to demonstrate the difference of before/after
1 parent de5e85c commit 7317730

44 files changed

Lines changed: 393 additions & 148 deletions

Some content is hidden

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

packages/backend.ai-ui/package.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,15 @@
7171
"rules": {
7272
"react-hooks/rules-of-hooks": "off"
7373
}
74+
},
75+
{
76+
"files": [
77+
"**/*.test.*",
78+
"**/*.stories.*"
79+
],
80+
"rules": {
81+
"no-console": "off"
82+
}
7483
}
7584
],
7685
"rules": {
@@ -94,6 +103,9 @@
94103
{
95104
"additionalHooks": "useRecoilCallback"
96105
}
106+
],
107+
"no-console": [
108+
"warn"
97109
]
98110
}
99111
},

packages/backend.ai-ui/src/components/provider/BAIClientProvider/hooks/useAnonymousBAIClient.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
import { useBAILogger } from '../../../../hooks';
12
import { BAIAnonymousClientContext } from '../context';
23
import { BAIClient } from '../types';
34
import { useContext } from 'react';
45

56
const useBAIAnonymousClient = (apiEndpoint: string): BAIClient => {
7+
const { logger } = useBAILogger();
8+
69
try {
710
const baiAnonymousClient = useContext(BAIAnonymousClientContext);
811
if (!baiAnonymousClient) {
@@ -12,7 +15,7 @@ const useBAIAnonymousClient = (apiEndpoint: string): BAIClient => {
1215
}
1316
return baiAnonymousClient(apiEndpoint);
1417
} catch (error) {
15-
console.error('Error using BAI Anonymous Client:', error);
18+
logger.error('Error using BAI Anonymous Client:', error);
1619
throw error;
1720
}
1821
};

packages/backend.ai-ui/src/hooks/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,9 @@ export type { ErrorResponse } from './useErrorMessageResolver';
3939
export type { ESMClientErrorResponse } from './useErrorMessageResolver';
4040
export { default as useGetAvailableFolderName } from './useGetAvailableFolderName';
4141
export { useInterval, useIntervalValue } from './useIntervalValue';
42+
export {
43+
default as useBAILogger,
44+
ContextualLogger,
45+
LogLevel,
46+
} from './useBAILogger';
47+
export type { LoggerPlugin, LogContext, BAILogger } from './useBAILogger';
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
export enum LogLevel {
2+
LOG = 0,
3+
DEBUG = 1,
4+
INFO = 2,
5+
WARN = 3,
6+
ERROR = 4,
7+
}
8+
9+
export interface LogContext {
10+
level: LogLevel;
11+
args: any[];
12+
timestamp: Date;
13+
metadata?: Record<string, any>;
14+
}
15+
16+
/**
17+
* Plugin interface for extending logger behavior
18+
*/
19+
export interface LoggerPlugin {
20+
/**
21+
* Hook called before logging. Can modify the log context or abort logging.
22+
* @param context - The current log context
23+
* @returns Modified context to continue logging, or null to abort
24+
*/
25+
beforeLog?(context: LogContext): LogContext | null;
26+
27+
/**
28+
* Hook called after logging is completed
29+
* @param context - The log context that was used
30+
*/
31+
afterLog?(context: LogContext): void;
32+
33+
/**
34+
* Hook called when an error occurs during the logging process
35+
* @param error - The error that occurred
36+
* @param context - The log context when the error happened
37+
*/
38+
onError?(error: Error, context: LogContext): void;
39+
}
40+
41+
/**
42+
* BAI Logger class for structured logging with plugin support
43+
*/
44+
class Logger {
45+
private plugins: LoggerPlugin[] = [];
46+
private metadata: Record<string, any> = {};
47+
private enabled: boolean = process.env.NODE_ENV !== 'production';
48+
private static instance: Logger;
49+
50+
private constructor() {
51+
// Singleton pattern: private constructor
52+
}
53+
54+
use(plugin: LoggerPlugin): this {
55+
this.plugins.push(plugin);
56+
return this;
57+
}
58+
59+
setMetadata(key: string, value: any): this {
60+
this.metadata[key] = value;
61+
return this;
62+
}
63+
64+
clearMetadata(): this {
65+
this.metadata = {};
66+
return this;
67+
}
68+
69+
setEnabled(enabled: boolean): this {
70+
this.enabled = enabled;
71+
return this;
72+
}
73+
74+
private logging(level: LogLevel, ...args: any[]) {
75+
if (!this.enabled) return;
76+
77+
let context: LogContext = {
78+
level,
79+
args,
80+
timestamp: new Date(),
81+
metadata: { ...this.metadata },
82+
};
83+
84+
try {
85+
// Before hooks
86+
for (const plugin of this.plugins) {
87+
const result = plugin.beforeLog?.(context);
88+
if (result === null) return;
89+
if (result !== undefined) {
90+
context = result;
91+
}
92+
}
93+
94+
// Actual log output
95+
const consoleMethods = [
96+
// eslint-disable-next-line no-console
97+
console.log,
98+
// eslint-disable-next-line no-console
99+
console.debug,
100+
// eslint-disable-next-line no-console
101+
console.info,
102+
// eslint-disable-next-line no-console
103+
console.warn,
104+
// eslint-disable-next-line no-console
105+
console.error,
106+
];
107+
consoleMethods[level](...context.args);
108+
109+
// After hooks
110+
for (const plugin of this.plugins) {
111+
plugin.afterLog?.(context);
112+
}
113+
} catch (error) {
114+
// Plugin error handling
115+
for (const plugin of this.plugins) {
116+
plugin.onError?.(error as Error, context);
117+
}
118+
}
119+
}
120+
121+
log(...args: any[]) {
122+
this.logging(LogLevel.LOG, ...args);
123+
}
124+
125+
debug(...args: any[]) {
126+
this.logging(LogLevel.DEBUG, ...args);
127+
}
128+
129+
info(...args: any[]) {
130+
this.logging(LogLevel.INFO, ...args);
131+
}
132+
133+
warn(...args: any[]) {
134+
this.logging(LogLevel.WARN, ...args);
135+
}
136+
137+
error(...args: any[]) {
138+
this.logging(LogLevel.ERROR, ...args);
139+
}
140+
141+
// Helper for method chaining
142+
withContext(key: string, value: any) {
143+
return new ContextualLogger(this, { [key]: value });
144+
}
145+
146+
static getInstance(): Logger {
147+
if (!Logger.instance) {
148+
Logger.instance = new Logger();
149+
}
150+
return Logger.instance;
151+
}
152+
}
153+
154+
// Logger with context support
155+
export class ContextualLogger {
156+
constructor(
157+
private baseLogger: Logger,
158+
private context: Record<string, any>,
159+
) {}
160+
161+
private withContext(fn: () => void) {
162+
this.baseLogger.setMetadata('context', this.context);
163+
try {
164+
fn();
165+
} finally {
166+
this.baseLogger.clearMetadata();
167+
}
168+
}
169+
170+
log(...args: any[]) {
171+
this.withContext(() => this.baseLogger.log(...args));
172+
}
173+
174+
debug(...args: any[]) {
175+
this.withContext(() => this.baseLogger.debug(...args));
176+
}
177+
178+
info(...args: any[]) {
179+
this.withContext(() => this.baseLogger.info(...args));
180+
}
181+
182+
warn(...args: any[]) {
183+
this.withContext(() => this.baseLogger.warn(...args));
184+
}
185+
186+
error(...args: any[]) {
187+
this.withContext(() => this.baseLogger.error(...args));
188+
}
189+
}
190+
191+
// Export Logger instance type (not the class itself)
192+
export type BAILogger = Logger;
193+
194+
const useBAILogger = () => {
195+
return { logger: Logger.getInstance() };
196+
};
197+
198+
export default useBAILogger;

react/package.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,17 @@
102102
"relay",
103103
"import"
104104
],
105+
"overrides": [
106+
{
107+
"files": [
108+
"**/*.test.*",
109+
"**/*.stories.*"
110+
],
111+
"rules": {
112+
"no-console": "off"
113+
}
114+
}
115+
],
105116
"rules": {
106117
"new-cap": "off",
107118
"prefer-promise-reject-errors": "off",
@@ -129,6 +140,9 @@
129140
"@lobehub/fluent-emoji"
130141
]
131142
}
143+
],
144+
"no-console": [
145+
"warn"
132146
]
133147
}
134148
},

react/src/components/AutoScalingRuleEditorModal.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
Space,
2020
Typography,
2121
} from 'antd';
22-
import { BAIFlex, BAIModal, BAIModalProps } from 'backend.ai-ui';
22+
import { BAIFlex, BAIModal, BAIModalProps, useBAILogger } from 'backend.ai-ui';
2323
import _ from 'lodash';
2424
import React, { useRef, useState } from 'react';
2525
import { useTranslation } from 'react-i18next';
@@ -66,6 +66,7 @@ const AutoScalingRuleEditorModal: React.FC<AutoScalingRuleEditorModalProps> = ({
6666
}) => {
6767
const { t } = useTranslation();
6868
const { message } = App.useApp();
69+
const { logger } = useBAILogger();
6970

7071
const [nameOptions, setNameOptions] = useState<Array<string>>(
7172
METRIC_NAMES_MAP.KERNEL || [],
@@ -222,7 +223,7 @@ const AutoScalingRuleEditorModal: React.FC<AutoScalingRuleEditorModalProps> = ({
222223
}
223224
})
224225
.catch((err) => {
225-
console.log(err);
226+
logger.error(err);
226227
});
227228
};
228229

react/src/components/Chat/ChatCard.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
} from 'ai';
2626
import { Alert, App, Card, CardProps, theme } from 'antd';
2727
import { createStyles } from 'antd-style';
28+
import { BAILogger, useBAILogger } from 'backend.ai-ui';
2829
import classNames from 'classnames';
2930
import _ from 'lodash';
3031
import React, {
@@ -163,13 +164,17 @@ const ChatHeader = PureChatHeader;
163164

164165
const ChatInput = React.memo(PureChatInput);
165166

166-
function createBaseURL(basePath?: string, endpointUrl?: string | null) {
167+
function createBaseURL(
168+
logger: BAILogger,
169+
basePath?: string,
170+
endpointUrl?: string | null,
171+
) {
167172
try {
168173
return endpointUrl
169174
? new URL(basePath ?? '', endpointUrl).toString()
170175
: undefined;
171176
} catch {
172-
console.error('Invalid base URL:', basePath, 'endpointUrl', endpointUrl);
177+
logger.error('Invalid base URL:', basePath, 'endpointUrl', endpointUrl);
173178
}
174179
}
175180

@@ -189,6 +194,7 @@ const PureChatCard: React.FC<ChatCardProps> = ({
189194
}) => {
190195
'use memo';
191196
const { t } = useTranslation();
197+
const { logger } = useBAILogger();
192198
const { message: appMessage } = App.useApp();
193199
const endpointResult = useLazyLoadQuery<ChatCardQuery>(
194200
graphql`
@@ -222,7 +228,7 @@ const PureChatCard: React.FC<ChatCardProps> = ({
222228
const [fetchKey, updateFetchKey] = useUpdatableState('first');
223229
const [startTime, setStartTime] = useState<number | null>(null);
224230

225-
const baseURL = createBaseURL(chat.provider.basePath, endpoint?.url);
231+
const baseURL = createBaseURL(logger, chat.provider.basePath, endpoint?.url);
226232
const { models, modelId, modelsError } = useModels(
227233
chat.provider,
228234
fetchKey,
@@ -287,7 +293,7 @@ const PureChatCard: React.FC<ChatCardProps> = ({
287293
},
288294
});
289295
} catch (error) {
290-
console.error('Client-side streaming error:', error);
296+
logger.error('Client-side streaming error:', error);
291297
// Fallback to regular fetch
292298
return fetch(input, init);
293299
}

0 commit comments

Comments
 (0)