Skip to content

Commit

Permalink
Remove deprecated PrefixedLogger interface (#4705)
Browse files Browse the repository at this point in the history
* Add some tests for `logger`

* Remove deprecated `PrefixedLogger` interface

`PrefixedLogger` has been deprecated for some time, so let's remove it now,
while we have a major version bump.

We can tidy up some of the other logic while we're here.

Unfortunately lots of the code still uses `logger.log` which isn't exposed by
the `Logger` interface, so we need to keep exposing that where it was before.
  • Loading branch information
richvdh authored Feb 11, 2025
1 parent c537a36 commit 33648a7
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 24 deletions.
51 changes: 51 additions & 0 deletions spec/unit/logger.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
Copyright 2025 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

/* eslint-disable no-console */

import loglevel from "loglevel";

import { logger } from "../../src/logger.ts";

afterEach(() => {
jest.restoreAllMocks();
});

describe("logger", () => {
it("should log to console by default", () => {
jest.spyOn(console, "debug").mockReturnValue(undefined);
logger.debug("test1");
logger.log("test2");

expect(console.debug).toHaveBeenCalledWith("test1");
expect(console.debug).toHaveBeenCalledWith("test2");
});

it("should allow creation of child loggers which add a prefix", () => {
jest.spyOn(loglevel, "getLogger");
jest.spyOn(console, "debug").mockReturnValue(undefined);

const childLogger = logger.getChild("[prefix1]");
expect(loglevel.getLogger).toHaveBeenCalledWith("matrix-[prefix1]");
childLogger.debug("test1");
expect(console.debug).toHaveBeenCalledWith("[prefix1]", "test1");

const grandchildLogger = childLogger.getChild("[prefix2]");
expect(loglevel.getLogger).toHaveBeenCalledWith("matrix-[prefix1][prefix2]");
grandchildLogger.debug("test2");
expect(console.debug).toHaveBeenCalledWith("[prefix1][prefix2]", "test2");
});
});
53 changes: 29 additions & 24 deletions src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ limitations under the License.

import loglevel from "loglevel";

/** Backwards-compatibility hack to expose `log` to applications that might still be relying on it. */
interface LoggerWithLogMethod extends Logger {
/**
* Output debug message to the logger.
*
* @param msg - Data to log.
*
* @deprecated prefer {@link Logger.debug}.
*/
log(...msg: any[]): void;
}

/** Logger interface used within the js-sdk codebase */
export interface Logger extends BaseLogger {
/**
Expand Down Expand Up @@ -104,44 +116,37 @@ loglevel.methodFactory = function (methodName, logLevel, loggerName) {

/**
* Implementation of {@link Logger} based on `loglevel`.
*
* @deprecated this shouldn't be public; prefer {@link Logger}.
*/
export interface PrefixedLogger extends loglevel.Logger, Logger {
/** @deprecated prefer {@link Logger.getChild} */
withPrefix: (prefix: string) => PrefixedLogger;

/** @deprecated internal property */
prefix: string;
interface PrefixedLogger extends loglevel.Logger, LoggerWithLogMethod {
prefix?: string;
}

/** Internal utility function to turn a `loglevel.Logger` into a `PrefixedLogger` */
function extendLogger(logger: loglevel.Logger): void {
const prefixedLogger = <PrefixedLogger>logger;
prefixedLogger.getChild = prefixedLogger.withPrefix = function (prefix: string): PrefixedLogger {
const existingPrefix = this.prefix || "";
return getPrefixedLogger(existingPrefix + prefix);
};
}
/**
* Internal utility function: gets a {@link Logger} based on `loglevel`.
*
* Child loggers produced by {@link Logger.getChild} add the name of the child logger as a prefix on each log line.
*
* @param prefix Prefix to add to each logged line. If undefined, no prefix will be added.
*/
function getPrefixedLogger(prefix?: string): LoggerWithLogMethod {
const loggerName = DEFAULT_NAMESPACE + (prefix === undefined ? "" : `-${prefix}`);
const prefixLogger = loglevel.getLogger(loggerName) as PrefixedLogger;

function getPrefixedLogger(prefix: string): PrefixedLogger {
const prefixLogger = loglevel.getLogger(`${DEFAULT_NAMESPACE}-${prefix}`) as PrefixedLogger;
if (prefixLogger.prefix !== prefix) {
// Only do this setup work the first time through, as loggers are saved by name.
extendLogger(prefixLogger);
if (prefixLogger.getChild === undefined) {
// This is a new loglevel Logger which has not been turned into a PrefixedLogger yet.
prefixLogger.prefix = prefix;
prefixLogger.getChild = (childPrefix): Logger => getPrefixedLogger((prefix ?? "") + childPrefix);
prefixLogger.setLevel(loglevel.levels.DEBUG, false);
}

return prefixLogger;
}

/**
* Drop-in replacement for `console` using {@link https://www.npmjs.com/package/loglevel|loglevel}.
* Can be tailored down to specific use cases if needed.
*/
export const logger = loglevel.getLogger(DEFAULT_NAMESPACE) as PrefixedLogger;
logger.setLevel(loglevel.levels.DEBUG, false);
extendLogger(logger);
export const logger = getPrefixedLogger();

/**
* A "span" for grouping related log lines together.
Expand Down

0 comments on commit 33648a7

Please sign in to comment.