Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Log level not honored with multi transport #10643

Merged
merged 1 commit into from
Dec 11, 2024
Merged
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
29 changes: 16 additions & 13 deletions yarn-project/foundation/src/log/pino-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ function isLevelEnabled(logger: pino.Logger<'verbose', boolean>, level: LogLevel
const defaultLogLevel = process.env.NODE_ENV === 'test' ? 'silent' : 'info';
const [logLevel, logFilters] = parseEnv(process.env.LOG_LEVEL, defaultLogLevel);

// Define custom logging levels for pino.
const customLevels = { verbose: 25 };
const pinoOpts = { customLevels, useOnlyCustomLevels: false, level: logLevel };

export const levels = {
labels: { ...pino.levels.labels, ...Object.fromEntries(Object.entries(customLevels).map(e => e.reverse())) },
values: { ...pino.levels.values, ...customLevels },
};

// Transport options for pretty logging to stderr via pino-pretty.
const useColor = true;
const { bold, reset } = createColors({ useColor });
Expand All @@ -79,24 +88,17 @@ export const pinoPrettyOpts = {
singleLine: !['1', 'true'].includes(process.env.LOG_MULTILINE ?? ''),
};

const prettyTransport: pino.TransportSingleOptions = {
const prettyTransport: pino.TransportTargetOptions = {
target: 'pino-pretty',
options: pinoPrettyOpts,
level: 'trace',
};

// Transport for vanilla stdio logging as JSON.
const stdioTransport: pino.TransportSingleOptions = {
const stdioTransport: pino.TransportTargetOptions = {
target: 'pino/file',
options: { destination: 2 },
};

// Define custom logging levels for pino.
const customLevels = { verbose: 25 };
const pinoOpts = { customLevels, useOnlyCustomLevels: false, level: logLevel };

export const levels = {
labels: { ...pino.levels.labels, ...Object.fromEntries(Object.entries(customLevels).map(e => e.reverse())) },
values: { ...pino.levels.values, ...customLevels },
level: 'trace',
};

// Transport for OpenTelemetry logging. While defining this here is an abstraction leakage since this
Expand All @@ -107,9 +109,10 @@ export const levels = {
// since pino will load this transport separately on a worker thread, to minimize disruption to the main loop.
const otlpEndpoint = process.env.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT;
const otelOpts = { levels };
const otelTransport: pino.TransportSingleOptions = {
const otelTransport: pino.TransportTargetOptions = {
target: '@aztec/telemetry-client/otel-pino-stream',
options: otelOpts,
level: 'trace',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the logs get filtered elsewhere or is this transport always going to publish >trace logs? (sorry, not familiar with the new pino logging backend yet)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, they get filtered by the top-level pino instance that wraps the transport.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that we could set this level to logLevel as well, but then when we create a child logger with a different level, the logs wouldn't make it past the transport.

};

function makeLogger() {
Expand All @@ -128,7 +131,7 @@ function makeLogger() {
['1', 'true', 'TRUE'].includes(process.env.LOG_JSON ?? '') ? stdioTransport : prettyTransport,
otlpEndpoint ? otelTransport : undefined,
]);
return pino(pinoOpts, pino.transport({ targets }));
return pino(pinoOpts, pino.transport({ targets, levels: levels.values }));
}
}

Expand Down
Loading