Skip to content

Commit

Permalink
browser-side logging: allow per-logger level configuration (#184568)
Browse files Browse the repository at this point in the history
## Summary

Part of #144276
Somewhat related to #184520

Allow to configure level of individual browser-side loggers

**Example:**
```yaml
logging.browser:
    loggers:
       - name: analytics 
         level: debug
```
  • Loading branch information
pgayvallet authored Jun 3, 2024
1 parent dea26c6 commit 76837c0
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,30 @@
* Side Public License, v 1.
*/

import type { BrowserLoggingConfig } from '@kbn/core-logging-common-internal';
import type { LogLevelId, Logger } from '@kbn/logging';
import { unsafeConsole } from '@kbn/security-hardening';
import type { BrowserLoggingConfig } from '@kbn/core-logging-common-internal';
import { BrowserLoggingSystem } from './logging_system';
import type { BaseLogger } from './logger';

describe('BrowserLoggingSystem', () => {
const timestamp = new Date(Date.UTC(2012, 1, 1, 14, 33, 22, 11));

let mockConsoleLog: jest.SpyInstance;
let loggingSystem: BrowserLoggingSystem;

const createLoggingConfig = (parts: Partial<BrowserLoggingConfig> = {}): BrowserLoggingConfig => {
return {
root: {
level: 'warn',
},
loggers: [],
...parts,
};
};

beforeEach(() => {
mockConsoleLog = jest.spyOn(unsafeConsole, 'log').mockReturnValue(undefined);
jest.spyOn<any, any>(global, 'Date').mockImplementation(() => timestamp);
loggingSystem = new BrowserLoggingSystem(createLoggingConfig());
});

afterEach(() => {
Expand All @@ -37,20 +38,23 @@ describe('BrowserLoggingSystem', () => {

describe('#get', () => {
it('returns the same logger for same context', () => {
const loggingSystem = new BrowserLoggingSystem(createLoggingConfig());
const loggerA = loggingSystem.get('same.logger');
const loggerB = loggingSystem.get('same.logger');
expect(loggerA).toBe(loggerB);
});

it('returns different loggers for different contexts', () => {
const loggingSystem = new BrowserLoggingSystem(createLoggingConfig());
const loggerA = loggingSystem.get('some.logger');
const loggerB = loggingSystem.get('another.logger');
expect(loggerA).not.toBe(loggerB);
});
});

describe('logger configuration', () => {
describe('root logger configuration', () => {
it('properly configure the logger to use the correct context and pattern', () => {
const loggingSystem = new BrowserLoggingSystem(createLoggingConfig());
const logger = loggingSystem.get('foo.bar');
logger.warn('some message');

Expand All @@ -62,6 +66,7 @@ describe('BrowserLoggingSystem', () => {
});

it('properly configure the logger to use the correct level', () => {
const loggingSystem = new BrowserLoggingSystem(createLoggingConfig());
const logger = loggingSystem.get('foo.bar');
logger.trace('some trace message');
logger.debug('some debug message');
Expand All @@ -86,9 +91,12 @@ describe('BrowserLoggingSystem', () => {
});

it('allows to override the root logger level', () => {
loggingSystem = new BrowserLoggingSystem(createLoggingConfig({ root: { level: 'debug' } }));
const loggingSystem = new BrowserLoggingSystem(
createLoggingConfig({ root: { level: 'debug' } })
);

const logger = loggingSystem.get('foo.bar');

logger.trace('some trace message');
logger.debug('some debug message');
logger.info('some info message');
Expand Down Expand Up @@ -117,4 +125,71 @@ describe('BrowserLoggingSystem', () => {
`);
});
});

describe('loggers configuration', () => {
it('uses the logger config if specified', () => {
const loggingSystem = new BrowserLoggingSystem(
createLoggingConfig({
root: { level: 'debug' },
loggers: [{ name: 'foo.bar', level: 'warn' }],
})
);

const logger = loggingSystem.get('foo.bar') as BaseLogger;

expect(getLoggerLevel(logger)).toBe('warn');
});

it('uses the parent config if present and logger config is not', () => {
const loggingSystem = new BrowserLoggingSystem(
createLoggingConfig({
root: { level: 'debug' },
loggers: [{ name: 'foo', level: 'warn' }],
})
);

const logger = loggingSystem.get('foo.bar') as BaseLogger;

expect(getLoggerLevel(logger)).toBe('warn');
});

it('uses the closest parent config', () => {
const loggingSystem = new BrowserLoggingSystem(
createLoggingConfig({
root: { level: 'debug' },
loggers: [
{ name: 'foo', level: 'warn' },
{ name: 'foo.bar', level: 'error' },
],
})
);

const logger = loggingSystem.get('foo.bar.hello') as BaseLogger;

expect(getLoggerLevel(logger)).toBe('error');
});

it('uses the root logger config by default', () => {
const loggingSystem = new BrowserLoggingSystem(
createLoggingConfig({
root: { level: 'debug' },
loggers: [],
})
);

const logger = loggingSystem.get('foo.bar.hello') as BaseLogger;

expect(getLoggerLevel(logger)).toBe('debug');
});
});
});

const getLoggerLevel = (logger: Logger): LogLevelId => {
const levels: LogLevelId[] = ['all', 'trace', 'debug', 'info', 'warn', 'error', 'fatal', 'all'];
for (const level of levels) {
if (logger.isLevelEnabled(level)) {
return level;
}
}
return 'off';
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
*/

import { LogLevel, Logger, LoggerFactory, DisposableAppender } from '@kbn/logging';
import { getLoggerContext, BrowserLoggingConfig } from '@kbn/core-logging-common-internal';
import {
ROOT_CONTEXT_NAME,
getLoggerContext,
getParentLoggerContext,
BrowserLoggingConfig,
BrowserLoggerConfig,
} from '@kbn/core-logging-common-internal';
import type { LoggerConfigType } from './types';
import { BaseLogger } from './logger';
import { PatternLayout } from './layouts';
Expand All @@ -22,15 +28,20 @@ export interface IBrowserLoggingSystem extends LoggerFactory {
asLoggerFactory(): LoggerFactory;
}

interface ComputedLoggerConfig {
loggers: Map<string, BrowserLoggerConfig>;
}

/**
* @internal
*/
export class BrowserLoggingSystem implements IBrowserLoggingSystem {
private readonly computedConfig: ComputedLoggerConfig;
private readonly loggers: Map<string, Logger> = new Map();
private readonly appenders: Map<string, DisposableAppender> = new Map();

constructor(private readonly loggingConfig: BrowserLoggingConfig) {
this.setupSystem(loggingConfig);
constructor(loggingConfig: BrowserLoggingConfig) {
this.computedConfig = this.setupSystem(loggingConfig);
}

public get(...contextParts: string[]): Logger {
Expand All @@ -49,16 +60,32 @@ export class BrowserLoggingSystem implements IBrowserLoggingSystem {
}

private getLoggerConfigByContext(context: string): LoggerConfigType {
return {
level: this.loggingConfig.root.level,
appenders: [CONSOLE_APPENDER_ID],
name: context,
};
const loggerConfig = this.computedConfig.loggers.get(context);
if (loggerConfig !== undefined) {
return {
name: loggerConfig.name,
level: loggerConfig.level,
appenders: [CONSOLE_APPENDER_ID],
};
}

// If we don't have configuration for the specified context, we move up to the parent context, up to `root`
return this.getLoggerConfigByContext(getParentLoggerContext(context));
}

private setupSystem(loggingConfig: BrowserLoggingConfig) {
const consoleAppender = new ConsoleAppender(new PatternLayout());
this.appenders.set(CONSOLE_APPENDER_ID, consoleAppender);

const loggerConfigs = loggingConfig.loggers.reduce((loggers, logger) => {
loggers.set(logger.name, logger);
return loggers;
}, new Map<string, BrowserLoggerConfig>());
loggerConfigs.set(ROOT_CONTEXT_NAME, { name: ROOT_CONTEXT_NAME, ...loggingConfig.root });

return {
loggers: loggerConfigs,
};
}

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/core/logging/core-logging-common-internal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ export {
ROOT_CONTEXT_NAME,
DEFAULT_APPENDER_NAME,
} from './src';
export type { BrowserLoggingConfig, BrowserRootLoggerConfig } from './src/browser_config';
export type {
BrowserLoggingConfig,
BrowserRootLoggerConfig,
BrowserLoggerConfig,
} from './src/browser_config';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ import type { LogLevelId } from '@kbn/logging';
*/
export interface BrowserLoggingConfig {
root: BrowserRootLoggerConfig;
loggers: BrowserLoggerConfig[];
}

export interface BrowserLoggerConfig {
name: string;
level: LogLevelId;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ test('`schema` creates correct schema with defaults.', () => {
Object {
"appenders": Map {},
"browser": Object {
"loggers": Array [],
"root": Object {
"level": "info",
},
Expand Down Expand Up @@ -165,6 +166,36 @@ test('correctly fills in custom `loggers` config.', () => {
});
});

test('correctly fills in custom browser-side `loggers` config.', () => {
const configValue = config.schema.validate({
browser: {
loggers: [
{
name: 'plugins',
level: 'warn',
},
{
name: 'http',
level: 'error',
},
],
},
});

expect(configValue.browser.loggers).toMatchInlineSnapshot(`
Array [
Object {
"level": "warn",
"name": "plugins",
},
Object {
"level": "error",
"name": "http",
},
]
`);
});

test('fails if loggers use unknown appenders.', () => {
const validateConfig = config.schema.validate({
loggers: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ const levelSchema = schema.oneOf(
}
);

// until we have feature parity between browser and server logging, we need to define distinct logger schemas
const browserLoggerSchema = schema.object({
name: schema.string(),
level: levelSchema,
});

const browserConfig = schema.object({
root: schema.object({
level: levelSchema,
}),
loggers: schema.arrayOf(browserLoggerSchema, {
defaultValue: [],
}),
});

/**
* Config schema for validating the `loggers` key in {@link LoggerContextConfigType} or {@link LoggingConfigType}.
*
Expand All @@ -47,12 +62,6 @@ export const loggerSchema = schema.object({
level: levelSchema,
});

const browserConfig = schema.object({
root: schema.object({
level: levelSchema,
}),
});

export const config = {
path: 'logging',
schema: schema.object({
Expand Down

0 comments on commit 76837c0

Please sign in to comment.