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

Use Messenger for logging and telemetry #2173

Merged
merged 5 commits into from
Dec 20, 2021
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
2 changes: 0 additions & 2 deletions src/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@ import "@/development/autoreload";
import "@/background/installer";
import "@/messaging/external";
import "@/background/locator";
import "@/background/logging";
import "@/background/contextMenus";
import "@/background/devtools";
import "@/background/browserAction";
import "@/background/trace";

import initGoogle from "@/contrib/google/initGoogle";
import initFrames from "@/background/iframes";
Expand Down
4 changes: 2 additions & 2 deletions src/background/installer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
*/

import browser, { Runtime } from "webextension-polyfill";
import { reportEvent, initTelemetry } from "@/telemetry/events";
import { getUID } from "@/background/telemetry";
import { reportEvent } from "@/telemetry/events";
import { getUID, initTelemetry } from "@/background/telemetry";
import { DNT_STORAGE_KEY, allowsTrack } from "@/telemetry/dnt";

const UNINSTALL_URL = "https://www.pixiebrix.com/uninstall/";
Expand Down
120 changes: 51 additions & 69 deletions src/background/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

import { uuidv4 } from "@/types/helpers";
import { liftBackground } from "@/background/protocol";
import { rollbar } from "@/telemetry/rollbar";
import { MessageContext, Logger as ILogger, SerializedError } from "@/core";
import { Except, JsonObject } from "type-fest";
Expand Down Expand Up @@ -197,71 +196,63 @@ function buildContext(
return context;
}

export const recordError = liftBackground(
"RECORD_ERROR",
async (
error: SerializedError,
context: MessageContext,
data: JsonObject | undefined
): Promise<void> => {
try {
const message = getErrorMessage(error);

if (await allowsTrack()) {
// Deserialize the error before passing it to rollbar, otherwise rollbar will assume the object is the custom
// payload data. WARNING: the prototype chain is lost during deserialization, so make sure any predicate you
// call here also handles deserialized errors properly.
// See https://docs.rollbar.com/docs/rollbarjs-configuration-reference#rollbarlog
// See https://github.com/sindresorhus/serialize-error/issues/48
const errorObj = deserializeError(error);

if (hasCancelRootCause(error)) {
// NOP - no reason to send to Rollbar
} else if (hasBusinessRootCause(error)) {
rollbar.debug(message, errorObj);
} else {
rollbar.error(message, errorObj);
}
export async function recordError(
error: SerializedError,
context: MessageContext,
data: JsonObject | undefined
): Promise<void> {
try {
const message = getErrorMessage(error);

if (await allowsTrack()) {
// Deserialize the error before passing it to rollbar, otherwise rollbar will assume the object is the custom
// payload data. WARNING: the prototype chain is lost during deserialization, so make sure any predicate you
// call here also handles deserialized errors properly.
// See https://docs.rollbar.com/docs/rollbarjs-configuration-reference#rollbarlog
// See https://github.com/sindresorhus/serialize-error/issues/48
const errorObj = deserializeError(error);

if (hasCancelRootCause(error)) {
// NOP - no reason to send to Rollbar
} else if (hasBusinessRootCause(error)) {
rollbar.debug(message, errorObj);
} else {
rollbar.error(message, errorObj);
}

await appendEntry({
uuid: uuidv4(),
timestamp: Date.now().toString(),
level: "error",
context: buildContext(error, context),
message,
error,
data,
});
} catch {
console.error("An error occurred while recording another error", {
error,
context,
});
}
},
{ asyncResponse: false }
);

export const recordLog = liftBackground(
"RECORD_LOG",
async (
context: MessageContext,
level: MessageLevel,
message: string,
data: JsonObject
): Promise<void> => {

await appendEntry({
uuid: uuidv4(),
timestamp: Date.now().toString(),
level,
level: "error",
context: buildContext(error, context),
message,
error,
data,
context: context ?? {},
});
},
{ asyncResponse: false }
);
} catch {
console.error("An error occurred while recording another error", {
error,
context,
});
}
}

export async function recordLog(
context: MessageContext,
level: MessageLevel,
message: string,
data: JsonObject
): Promise<void> {
await appendEntry({
uuid: uuidv4(),
timestamp: Date.now().toString(),
level,
message,
data,
context: context ?? {},
});
}

export class BackgroundLogger implements ILogger {
readonly context: MessageContext;
Expand Down Expand Up @@ -321,7 +312,7 @@ export type LoggingConfig = {
const LOG_CONFIG_STORAGE_KEY = "LOG_OPTIONS" as ManualStorageKey;
let _config: LoggingConfig = null;

export async function _getLoggingConfig(): Promise<LoggingConfig> {
export async function getLoggingConfig(): Promise<LoggingConfig> {
expectContext("background");

if (_config != null) {
Expand All @@ -333,18 +324,9 @@ export async function _getLoggingConfig(): Promise<LoggingConfig> {
});
}

export async function _setLoggingConfig(config: LoggingConfig): Promise<void> {
export async function setLoggingConfig(config: LoggingConfig): Promise<void> {
expectContext("background");

await setStorage(LOG_CONFIG_STORAGE_KEY, config);
_config = config;
}

export const getLoggingConfig = liftBackground("GET_LOGGING_CONFIG", async () =>
_getLoggingConfig()
);

export const setLoggingConfig = liftBackground(
"SET_LOGGING_CONFIG",
async (config: LoggingConfig) => _setLoggingConfig(config)
);
15 changes: 15 additions & 0 deletions src/background/messenger/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,18 @@ export const proxyService = getMethod("PROXY", bg) as <TData>(
serviceConfig: SanitizedServiceConfiguration | null,
requestConfig: AxiosRequestConfig
) => Promise<RemoteResponse<TData>>;

export const recordLog = getNotifier("RECORD_LOG", bg);
export const recordError = getNotifier("RECORD_ERROR", bg);
export const recordEvent = getNotifier("RECORD_EVENT", bg);
Comment on lines +110 to +112
Copy link
Contributor Author

@fregante fregante Dec 18, 2021

Choose a reason for hiding this comment

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

One note about these, since I mentioned them as blockers in:

I found:

  • recordLog isn't used at all in the app
  • recordEvent appears in the app, but it isn't called
  • recordError may be called, but since this is a "notifier" it will not throw any errors:

export const getLoggingConfig = getMethod("GET_LOGGING_CONFIG", bg);
export const setLoggingConfig = getMethod("SET_LOGGING_CONFIG", bg);

export const traces = {
addEntry: getNotifier("ADD_TRACE_ENTRY", bg),
addExit: getNotifier("ADD_TRACE_EXIT", bg),
clear: getNotifier("CLEAR_TRACES", bg),
};

export const initTelemetry = getNotifier("INIT_TELEMETRY", bg);
export const sendDeploymentAlert = getNotifier("SEND_DEPLOYMENT_ALERT", bg);
Comment on lines +110 to +123
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test report

Working

  • GET_LOGGING_CONFIG
  • SET_LOGGING_CONFIG
  • RECORD_EVENT
  • ADD_TRACE_ENTRY
  • ADD_TRACE_EXIT
  • CLEAR_TRACES
  • RECORD_ERROR

Seen

  • INIT_TELEMETRY (old INIT_UID)

Unable to reproduce

  • SEND_DEPLOYMENT_ALERT
  • RECORD_LOG

42 changes: 42 additions & 0 deletions src/background/messenger/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@ import { getAvailableVersion } from "@/background/installer";
import { locator, refreshServices } from "@/background/locator";
import { reactivateEveryTab } from "@/background/navigation";
import { canAccessTab } from "webext-tools";
import {
getLoggingConfig,
recordError,
recordLog,
setLoggingConfig,
} from "@/background/logging";
import {
addTraceEntry,
addTraceExit,
clearExtensionTraces,
} from "@/telemetry/trace";
import {
initTelemetry,
recordEvent,
sendDeploymentAlert,
} from "@/background/telemetry";

expectContext("background");

Expand Down Expand Up @@ -97,6 +113,19 @@ declare global {

GET_DATA_STORE: typeof getRecord;
SET_DATA_STORE: typeof setRecord;

RECORD_LOG: typeof recordLog;
RECORD_ERROR: typeof recordError;
RECORD_EVENT: typeof recordEvent;
GET_LOGGING_CONFIG: typeof getLoggingConfig;
SET_LOGGING_CONFIG: typeof setLoggingConfig;

ADD_TRACE_ENTRY: typeof addTraceEntry;
ADD_TRACE_EXIT: typeof addTraceExit;
CLEAR_TRACES: typeof clearExtensionTraces;

INIT_TELEMETRY: typeof initTelemetry;
SEND_DEPLOYMENT_ALERT: typeof sendDeploymentAlert;
}
}

Expand Down Expand Up @@ -145,4 +174,17 @@ registerMethods({

GET_DATA_STORE: getRecord,
SET_DATA_STORE: setRecord,

RECORD_LOG: recordLog,
RECORD_ERROR: recordError,
RECORD_EVENT: recordEvent,
GET_LOGGING_CONFIG: getLoggingConfig,
SET_LOGGING_CONFIG: setLoggingConfig,

ADD_TRACE_ENTRY: addTraceEntry,
ADD_TRACE_EXIT: addTraceExit,
CLEAR_TRACES: clearExtensionTraces,

INIT_TELEMETRY: initTelemetry,
SEND_DEPLOYMENT_ALERT: sendDeploymentAlert,
});
74 changes: 32 additions & 42 deletions src/background/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,57 +112,47 @@ async function userSummary() {
};
}

async function _init(): Promise<void> {
if (await isLinked()) {
await (await getLinkedApiClient()).post("/api/identify/", {
async function init(): Promise<void> {
if ((await isLinked()) && (await allowsTrack())) {
const client = await getLinkedApiClient();
await client.post("/api/identify/", {
uid: await uid(),
data: await userSummary(),
});
}
}

// Up to every 30 min
const throttledInit = throttle(_init, 30 * 60 * 1000, {
export const initTelemetry = throttle(init, 30 * 60 * 1000, {
leading: true,
trailing: true,
});

export const initUID = liftBackground(
"INIT_UID",
async (): Promise<void> => {
if (await allowsTrack()) {
void throttledInit();
}
},
{ asyncResponse: false }
);

export const recordEvent = liftBackground(
"RECORD_EVENT",
async ({
event,
data = {},
}: {
event: string;
data: JsonObject | undefined;
}): Promise<void> => {
if (await allowsTrack()) {
buffer.push({
uid: await uid(),
event,
timestamp: Date.now(),
data,
});
void debouncedFlush();
}
},
{ asyncResponse: false }
);

export const sendDeploymentAlert = liftBackground(
"SEND_DEPLOYMENT_ALERT",
async ({ deploymentId, data }: { deploymentId: string; data: Data }) => {
const url = `/api/deployments/${deploymentId}/alerts/`;
await (await getLinkedApiClient()).post(url, data);
export async function recordEvent({
event,
data = {},
}: {
event: string;
data: JsonObject | undefined;
}): Promise<void> {
if (await allowsTrack()) {
buffer.push({
uid: await uid(),
event,
timestamp: Date.now(),
data,
});
void debouncedFlush();
}
);
}

export async function sendDeploymentAlert({
deploymentId,
data,
}: {
deploymentId: string;
data: Data;
}) {
const client = await getLinkedApiClient();
await client.post(`/api/deployments/${deploymentId}/alerts/`, data);
}
36 changes: 0 additions & 36 deletions src/background/trace.ts

This file was deleted.

Loading