Show additional data in debug log header

This commit is contained in:
Evan Hahn 2021-07-30 11:43:16 -05:00 committed by GitHub
parent 03874a788f
commit 689542a9b4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 278 additions and 89 deletions

View file

@ -0,0 +1,10 @@
// Copyright 2021 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
export const formatCountForLogging = (count: number): string => {
if (count === 0 || Number.isNaN(count)) {
return String(count);
}
return `at least ${10 ** Math.floor(Math.log10(count))}`;
};

View file

@ -7,7 +7,7 @@
import * as path from 'path';
import * as fs from 'fs';
import { app, ipcMain as ipc } from 'electron';
import { BrowserWindow, app, ipcMain as ipc } from 'electron';
import pinoms from 'pino-multi-stream';
import pino from 'pino';
import * as mkdirp from 'mkdirp';
@ -21,6 +21,7 @@ import { setLogAtLevel } from './log';
import { Environment, getEnvironment } from '../environment';
import {
FetchLogIpcData,
LogEntryType,
LogLevel,
cleanArgs,
@ -46,7 +47,9 @@ const isRunningFromConsole =
getEnvironment() === Environment.Test ||
getEnvironment() === Environment.TestLib;
export async function initialize(): Promise<pinoms.Logger> {
export async function initialize(
getMainWindow: () => undefined | BrowserWindow
): Promise<pinoms.Logger> {
if (globalLogger) {
throw new Error('Already called initialize!');
}
@ -80,7 +83,7 @@ export async function initialize(): Promise<pinoms.Logger> {
globalLogger = undefined;
if (shouldRestart) {
initialize();
initialize(getMainWindow);
}
};
@ -102,34 +105,44 @@ export async function initialize(): Promise<pinoms.Logger> {
timestamp: pino.stdTimeFunctions.isoTime,
});
ipc.on('fetch-log', event => {
fetch(logPath).then(
data => {
try {
event.sender.send('fetched-log', data);
} catch (err: unknown) {
// NOTE(evanhahn): We don't want to send a message to a window that's closed.
// I wanted to use `event.sender.isDestroyed()` but that seems to fail.
// Instead, we attempt the send and catch the failure as best we can.
const hasUserClosedWindow = isProbablyObjectHasBeenDestroyedError(
err
);
if (hasUserClosedWindow) {
logger.info(
'Logs were requested, but it seems the window was closed'
);
} else {
logger.error(
'Problem replying with fetched logs',
err instanceof Error && err.stack ? err.stack : err
);
}
}
},
error => {
logger.error(`Problem loading log from disk: ${error.stack}`);
ipc.on('fetch-log', async event => {
const mainWindow = getMainWindow();
if (!mainWindow) {
logger.info('Logs were requested, but the main window is missing');
return;
}
let data: FetchLogIpcData;
try {
const [logEntries, rest] = await Promise.all([
fetchLogs(logPath),
fetchAdditionalLogData(mainWindow),
]);
data = {
logEntries,
...rest,
};
} catch (error) {
logger.error(`Problem loading log data: ${error.stack}`);
return;
}
try {
event.sender.send('fetched-log', data);
} catch (err: unknown) {
// NOTE(evanhahn): We don't want to send a message to a window that's closed.
// I wanted to use `event.sender.isDestroyed()` but that seems to fail.
// Instead, we attempt the send and catch the failure as best we can.
const hasUserClosedWindow = isProbablyObjectHasBeenDestroyedError(err);
if (hasUserClosedWindow) {
logger.info('Logs were requested, but it seems the window was closed');
} else {
logger.error(
'Problem replying with fetched logs',
err instanceof Error && err.stack ? err.stack : err
);
}
);
}
});
ipc.on('delete-all-logs', async event => {
@ -293,7 +306,7 @@ export function fetchLog(logFile: string): Promise<Array<LogEntryType>> {
}
// Exported for testing only.
export function fetch(logPath: string): Promise<Array<LogEntryType>> {
export function fetchLogs(logPath: string): Promise<Array<LogEntryType>> {
const files = fs.readdirSync(logPath);
const paths = files.map(file => path.join(logPath, file));
@ -313,6 +326,16 @@ export function fetch(logPath: string): Promise<Array<LogEntryType>> {
});
}
export const fetchAdditionalLogData = (
mainWindow: BrowserWindow
): Promise<Omit<FetchLogIpcData, 'logEntries'>> =>
new Promise(resolve => {
mainWindow.webContents.send('additional-log-data-request');
ipc.once('additional-log-data-response', (_event, data) => {
resolve(data);
});
});
function logAtLevel(level: LogLevel, ...args: ReadonlyArray<unknown>) {
if (globalLogger) {
const levelString = getLogLevelString(level);

View file

@ -19,10 +19,12 @@ import {
import { uploadDebugLogs } from './debuglogs';
import { redactAll } from '../util/privacy';
import {
FetchLogIpcData,
LogEntryType,
LogLevel,
cleanArgs,
getLogLevelString,
isFetchLogIpcData,
isLogEntry,
} from './shared';
import * as log from './log';
@ -52,21 +54,49 @@ if (window.console) {
// The mechanics of preparing a log for publish
function getHeader() {
let header = window.navigator.userAgent;
const headerSectionTitle = (title: string) => `========= ${title} =========`;
header += ` node/${window.getNodeVersion()}`;
header += ` env/${window.getEnvironment()}`;
const headerSection = (
title: string,
data: Readonly<Record<string, unknown>>
): string => {
const sortedEntries = _.sortBy(Object.entries(data), ([key]) => key);
return [
headerSectionTitle(title),
...sortedEntries.map(
([key, value]) => `${key}: ${redactAll(String(value))}`
),
'',
].join('\n');
};
return header;
}
const getHeader = ({
capabilities,
remoteConfig,
statistics,
user,
}: Omit<FetchLogIpcData, 'logEntries'>): string =>
[
headerSection('System info', {
Time: Date.now(),
'User agent': window.navigator.userAgent,
'Node version': window.getNodeVersion(),
Environment: window.getEnvironment(),
'App version': window.getVersion(),
}),
headerSection('User info', user),
headerSection('Capabilities', capabilities),
headerSection('Remote config', remoteConfig),
headerSection('Statistics', statistics),
headerSectionTitle('Logs'),
].join('\n');
const getLevel = _.memoize((level: LogLevel): string => {
const text = getLogLevelString(level);
return text.toUpperCase().padEnd(levelMaxLength, ' ');
});
function formatLine(mightBeEntry: Readonly<unknown>): string {
function formatLine(mightBeEntry: unknown): string {
const entry: LogEntryType = isLogEntry(mightBeEntry)
? mightBeEntry
: {
@ -84,11 +114,15 @@ function fetch(): Promise<string> {
return new Promise(resolve => {
ipc.send('fetch-log');
ipc.on('fetched-log', (_event, logEntries: unknown) => {
ipc.on('fetched-log', (_event, data: unknown) => {
let header: string;
let body: string;
if (Array.isArray(logEntries)) {
if (isFetchLogIpcData(data)) {
const { logEntries } = data;
header = getHeader(data);
body = logEntries.map(formatLine).join('\n');
} else {
header = headerSectionTitle('Partial logs');
const entry: LogEntryType = {
level: LogLevel.Error,
msg: 'Invalid IPC data when fetching logs; dropping all logs',
@ -97,7 +131,7 @@ function fetch(): Promise<string> {
body = formatLine(entry);
}
const result = `${getHeader()}\n${redactAll(body)}`;
const result = `${header}\n${body}`;
resolve(result);
});
});

View file

@ -2,10 +2,34 @@
// SPDX-License-Identifier: AGPL-3.0-only
import * as pino from 'pino';
import { isRecord } from '../util/isRecord';
import { redactAll } from '../util/privacy';
import { missingCaseError } from '../util/missingCaseError';
import { reallyJsonStringify } from '../util/reallyJsonStringify';
export type FetchLogIpcData = {
capabilities: Record<string, unknown>;
remoteConfig: Record<string, unknown>;
statistics: Record<string, unknown>;
user: Record<string, unknown>;
// We expect `logEntries` to be `Array<LogEntryType>`, but we don't validate that
// upfront—we only validate it when we go to log each line. This improves the
// performance, because we don't have to iterate over every single log entry twice. It
// also means we can log entries if only some of them are invalid.
logEntries: Array<unknown>;
};
// We don't use Zod here because it'd be slow parsing all of the log entries.
// Unfortunately, Zod is a bit slow even with `z.array(z.unknown())`.
export const isFetchLogIpcData = (data: unknown): data is FetchLogIpcData =>
isRecord(data) &&
isRecord(data.capabilities) &&
isRecord(data.remoteConfig) &&
isRecord(data.statistics) &&
isRecord(data.user) &&
Array.isArray(data.logEntries);
// These match [Pino's recommendations][0].
// [0]: https://getpino.io/#/docs/api?id=loggerlevels-object
export enum LogLevel {
@ -29,7 +53,7 @@ export type LogEntryType = Readonly<{
// whenever we want to send the debug log. We can't use `zod` because it clones
// the data on successful parse and ruins the performance.
export const isLogEntry = (data: unknown): data is LogEntryType => {
if (data === null || typeof data !== 'object') {
if (!isRecord(data)) {
return false;
}