Skip to content

[dashboard] unify error reporting #18726

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

Merged
merged 3 commits into from
Sep 18, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
*/

import { FC, useCallback } from "react";
import { ErrorBoundary, FallbackProps, ErrorBoundaryProps } from "react-error-boundary";
import { ErrorBoundary, ErrorBoundaryProps, FallbackProps } from "react-error-boundary";
import gitpodIcon from "../../icons/gitpod.svg";
import { getGitpodService } from "../../service/service";
import { Heading1, Subheading } from "../typography/headings";
import { reportError } from "../../service/metrics";

export type CaughtError = Error & { code?: number };

Expand Down Expand Up @@ -64,11 +64,4 @@ export const ReloadPageErrorFallback: FC<Pick<FallbackProps, "error">> = ({ erro
);
};

export const handleError: ErrorBoundaryProps["onError"] = async (error, info) => {
const url = window.location.toString();
try {
await getGitpodService().server.reportErrorBoundary(url, error.message || "Unknown Error");
} catch (e) {
console.error(e);
}
};
export const handleError: ErrorBoundaryProps["onError"] = (error, info) => reportError("Error boundary", error, info);
37 changes: 23 additions & 14 deletions components/dashboard/src/service/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@ console.error = function (...args) {
reportError(...args);
};

function reportError(...args: any[]) {
let commonDetails = {};
export function updateCommonErrorDetails(update: { [key: string]: string | undefined }) {
Object.assign(commonDetails, update);
}

export function reportError(...args: any[]) {
let err = undefined;
let details = undefined;
if (args[0] instanceof Error) {
Expand All @@ -61,22 +66,26 @@ function reportError(...args: any[]) {
}
}

let data = undefined;
let data = {};
if (details && typeof details === "object") {
data = Object.fromEntries(
Object.entries(details)
.filter(([key, value]) => {
return (
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean" ||
value === null ||
typeof value === "undefined"
);
})
.map(([key, value]) => [key, String(value)]),
data = Object.assign(
data,
Object.fromEntries(
Object.entries(details)
.filter(([key, value]) => {
return (
typeof value === "string" ||
typeof value === "number" ||
typeof value === "boolean" ||
value === null ||
typeof value === "undefined"
);
})
.map(([key, value]) => [key, String(value)]),
),
);
}
data = Object.assign(data, commonDetails);

if (err) {
metricsReporter.reportError(err, data);
Expand Down
7 changes: 7 additions & 0 deletions components/dashboard/src/user-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { User } from "@gitpod/gitpod-protocol";
import { useQueryClient } from "@tanstack/react-query";
import React, { createContext, useState, useContext, useMemo, useCallback } from "react";
import { updateCommonErrorDetails } from "./service/metrics";

const UserContext = createContext<{
user?: User;
Expand All @@ -18,10 +19,16 @@ const UserContext = createContext<{
const UserContextProvider: React.FC = ({ children }) => {
const [user, setUser] = useState<User>();

const updateErrorUserDetails = (user?: User) => {
updateCommonErrorDetails({ userId: user?.id });
};
updateErrorUserDetails(user);

const client = useQueryClient();

const doSetUser = useCallback(
(updatedUser: User) => {
updateErrorUserDetails(updatedUser);
if (user?.id !== updatedUser.id) {
client.clear();
}
Expand Down
10 changes: 7 additions & 3 deletions components/public-api/typescript/src/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,10 @@ export class MetricsReporter {
properties["error_name"] = error.name;
properties["error_message"] = error.message;

const workspaceId = properties["workspaceId"];
const instanceId = properties["instanceId"];
const userId = properties["userId"];

delete properties["workspaceId"];
delete properties["instanceId"];
delete properties["userId"];
Expand All @@ -381,9 +385,9 @@ export class MetricsReporter {
component: this.options.clientName,
errorStack: error.stack ?? String(error),
version: this.options.clientVersion,
workspaceId: properties["workspaceId"] ?? "",
instanceId: properties["instanceId"] ?? "",
userId: properties["userId"] ?? "",
workspaceId: workspaceId ?? "",
instanceId: instanceId ?? "",
userId: userId ?? "",
properties,
});
}
Expand Down
10 changes: 0 additions & 10 deletions components/server/src/prometheus-metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export function registerServerMetrics(registry: prometheusClient.Registry) {
registry.registerMetric(imageBuildsStartedTotal);
registry.registerMetric(imageBuildsCompletedTotal);
registry.registerMetric(spicedbClientLatency);
registry.registerMetric(dashboardErrorBoundary);
registry.registerMetric(jwtCookieIssued);
registry.registerMetric(jobStartedTotal);
registry.registerMetric(jobsCompletedTotal);
Expand Down Expand Up @@ -264,15 +263,6 @@ export function observeSpicedbClientLatency(operation: string, outcome: Error |
);
}

export const dashboardErrorBoundary = new prometheusClient.Counter({
Copy link
Member Author

Choose a reason for hiding this comment

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

it is already available in GCP if one search for the message

name: "gitpod_dashboard_error_boundary_total",
help: "Total number of errors caught by an error boundary in the dashboard",
});

export function increaseDashboardErrorBoundaryCounter() {
dashboardErrorBoundary.inc();
}

export const jobStartedTotal = new prometheusClient.Counter({
name: "gitpod_server_jobs_started_total",
help: "Total number of errors caught by an error boundary in the dashboard",
Expand Down
13 changes: 5 additions & 8 deletions components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution";
import { CostCenterJSON } from "@gitpod/gitpod-protocol/lib/usage";
import { createCookielessId, maskIp } from "../analytics";
import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
import { increaseDashboardErrorBoundaryCounter } from "../prometheus-metrics";
import { LinkedInService } from "../linkedin-service";
import { SnapshotService, WaitForSnapshotOptions } from "./snapshot-service";
import { IncrementalPrebuildsService } from "../prebuilds/incremental-prebuilds-service";
Expand Down Expand Up @@ -3464,14 +3463,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
//
//#endregion

/**
* TODO(ak)
* @deprecated remove it after dashboard is deployed. It was replaced with error reporting in GCP.
*/
async reportErrorBoundary(ctx: TraceContextWithSpan, url: string, message: string): Promise<void> {
// Cap message and url length so the entries aren't of unbounded length
log.warn("dashboard error boundary", {
message: (message || "").substring(0, 200),
url: (url || "").substring(0, 200),
Copy link
Member Author

Choose a reason for hiding this comment

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

it was leaking sensitive data in some URLs

userId: this.userID,
});
increaseDashboardErrorBoundaryCounter();
// no-op
}

async getIDToken(): Promise<void> {}
Expand Down