Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions yarn-project/foundation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
},
"devDependencies": {
"@jest/globals": "^29.5.0",
"@libp2p/interface": "1.3.1",
"@types/bn.js": "^5.1.3",
"@types/debug": "^4.1.7",
"@types/detect-node": "^2.0.0",
Expand Down
1 change: 1 addition & 0 deletions yarn-project/foundation/src/log/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from './debug.js';
export * from './pino-logger.js';
export * from './log_history.js';
export * from './log_fn.js';
export * from './libp2p_logger.js';
48 changes: 48 additions & 0 deletions yarn-project/foundation/src/log/libp2p_logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { type ComponentLogger, type Logger } from '@libp2p/interface';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add libp2p/interface to foundation's dev dependencies


import { getLogLevelFromFilters } from './log-filters.js';
import { logFilters, logger } from './pino-logger.js';

/**
* Creates a libp2p compatible logger that wraps our pino logger.
* This adapter implements the ComponentLogger interface required by libp2p.
*/
export function createLibp2pComponentLogger(namespace: string): ComponentLogger {
return {
forComponent: (component: string) => createLibp2pLogger(`${namespace}:${component}`),
};
}

function createLibp2pLogger(component: string): Logger {
// Create a direct pino logger instance for libp2p that supports string interpolation
const log = logger.child({ module: component }, { level: getLogLevelFromFilters(logFilters, component) });

// Default log level is trace as this is super super noisy
const logFn = (message: string, ...args: unknown[]) => {
log.trace(message, ...args);
};

return Object.assign(logFn, {
enabled: log.isLevelEnabled('debug'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I had already commented about whether this should be hardcoded, and you had replied, and I can't remember the answer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much that it is extremely noisy, and we only want to be able to see it whenever we are running with LOG_LEVEL=debug and below

error(message: string, ...args: unknown[]) {
// We write error outputs as debug as they are often expected, e.g. connection errors can happen in happy paths
log.debug(message, ...args);
},

debug(message: string, ...args: unknown[]) {
log.debug(message, ...args);
},

info(message: string, ...args: unknown[]) {
log.info(message, ...args);
},

warn(message: string, ...args: unknown[]) {
log.warn(message, ...args);
},

trace(message: string, ...args: unknown[]) {
log.trace(message, ...args);
},
});
}
17 changes: 4 additions & 13 deletions yarn-project/foundation/src/log/pino-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,14 @@ import { getLogLevelFromFilters, parseEnv } from './log-filters.js';
import { type LogLevel } from './log-levels.js';
import { type LogData, type LogFn } from './log_fn.js';

export function createLogger(module: string, fixedTerms = {}): Logger {
export function createLogger(module: string): Logger {
module = logNameHandlers.reduce((moduleName, handler) => handler(moduleName), module.replace(/^aztec:/, ''));
const pinoLogger = logger.child({ module }, { level: getLogLevelFromFilters(logFilters, module) });

// Only perform copy of data if fixed terms are provided
const hasFixedTerms = Object.keys(fixedTerms).length > 0;

// We check manually for isLevelEnabled to avoid calling processLogData unnecessarily.
// Note that isLevelEnabled is missing from the browser version of pino.
const logFn = (level: LogLevel, msg: string, data?: unknown) =>
isLevelEnabled(pinoLogger, level) &&
pinoLogger[level](
hasFixedTerms
? processLogData({ ...fixedTerms, ...(data ?? {}) } as LogData)
: processLogData((data as LogData) ?? {}),
msg,
);
isLevelEnabled(pinoLogger, level) && pinoLogger[level](processLogData((data as LogData) ?? {}), msg);

return {
silent: () => {},
Expand Down Expand Up @@ -92,7 +83,7 @@ function isLevelEnabled(logger: pino.Logger<'verbose', boolean>, level: LogLevel

// Load log levels from environment variables.
const defaultLogLevel = process.env.NODE_ENV === 'test' ? 'silent' : 'info';
const [logLevel, logFilters] = parseEnv(process.env.LOG_LEVEL, defaultLogLevel);
export const [logLevel, logFilters] = parseEnv(process.env.LOG_LEVEL, defaultLogLevel);

// Define custom logging levels for pino.
const customLevels = { verbose: 25 };
Expand Down Expand Up @@ -178,7 +169,7 @@ function makeLogger() {
}
}

const logger = makeLogger();
export const logger = makeLogger();

// Log the logger configuration.
logger.verbose(
Expand Down
78 changes: 0 additions & 78 deletions yarn-project/p2p/src/services/libp2p/libp2p_logger.ts

This file was deleted.

6 changes: 2 additions & 4 deletions yarn-project/p2p/src/services/libp2p/libp2p_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
} from '@aztec/circuit-types';
import { Fr } from '@aztec/circuits.js';
import { type EpochCacheInterface } from '@aztec/epoch-cache';
import { createLogger } from '@aztec/foundation/log';
import { createLibp2pComponentLogger, createLogger } from '@aztec/foundation/log';
import { SerialQueue } from '@aztec/foundation/queue';
import { RunningPromise } from '@aztec/foundation/running-promise';
import type { AztecAsyncKVStore } from '@aztec/kv-store';
Expand Down Expand Up @@ -65,7 +65,6 @@ import { pingHandler, reqRespBlockHandler, reqRespTxHandler, statusHandler } fro
import { ReqResp } from '../reqresp/reqresp.js';
import type { P2PService, PeerDiscoveryService } from '../service.js';
import { GossipSubEvent } from '../types.js';
import { createLibp2pComponentLogger } from './libp2p_logger.js';

interface MessageValidator {
validator: {
Expand Down Expand Up @@ -268,8 +267,7 @@ export class LibP2PService<T extends P2PClientType> extends WithTracer implement
connectionManager: components.connectionManager,
}),
},
// Fix the peer id in libp2p logs so we can see the source of the log
logger: createLibp2pComponentLogger(logger.module, { sourcePeerId: peerId }),
logger: createLibp2pComponentLogger(logger.module),
});

return new LibP2PService(
Expand Down
1 change: 1 addition & 0 deletions yarn-project/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,7 @@ __metadata:
"@aztec/bb.js": "portal:../../barretenberg/ts"
"@jest/globals": "npm:^29.5.0"
"@koa/cors": "npm:^5.0.0"
"@libp2p/interface": "npm:1.3.1"
"@noble/curves": "npm:^1.2.0"
"@types/bn.js": "npm:^5.1.3"
"@types/debug": "npm:^4.1.7"
Expand Down