Skip to content

Commit 9d3ecc3

Browse files
committed
[CC-24092] ui: add an overridable logger
A setLogger and getLogger method are added to cluster-ui so that the cockroach cloud console can use them to pass a custom logger which embellishes each log invocation with the cluster version and cluster id. Epic: CC-24047 Release Note: none
1 parent c566dbc commit 9d3ecc3

File tree

9 files changed

+123
-11
lines changed

9 files changed

+123
-11
lines changed

pkg/ui/workspaces/cluster-ui/src/api/contentionApi.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from "./sqlApi";
2020
import { ContentionDetails } from "src/insights";
2121
import moment from "moment-timezone";
22+
import { getLogger } from "../util";
2223

2324
export type ContentionFilters = {
2425
waitingTxnID?: string;
@@ -62,8 +63,9 @@ export async function getContentionDetailsApi(
6263
if (sqlResultsAreEmpty(result)) {
6364
if (result.error) {
6465
// We don't return an error if it failed to retrieve the contention information.
65-
console.error(
66-
`Insights encounter an error while retrieving contention information: ${result.error}`,
66+
getLogger().error(
67+
"Insights encounter an error while retrieving contention information.",
68+
{ resultError: result.error },
6769
);
6870
}
6971
return formatApiResult<ContentionDetails[]>(

pkg/ui/workspaces/cluster-ui/src/api/databaseDetailsApi.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { Format, Identifier, QualifiedIdentifier } from "./safesql";
2626
import moment from "moment-timezone";
2727
import { fromHexString, withTimeout } from "./util";
2828
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
29+
import { getLogger } from "../util";
2930

3031
const { ZoneConfig } = cockroach.config.zonepb;
3132
const { ZoneConfigurationLevel } = cockroach.server.serverpb;
@@ -272,8 +273,10 @@ const getDatabaseZoneConfig: DatabaseDetailsQuery<DatabaseZoneConfigRow> = {
272273
);
273274
resp.zoneConfigResp.zone_config_level = ZoneConfigurationLevel.DATABASE;
274275
} catch (e) {
275-
console.error(
276+
getLogger().error(
276277
`Database Details API - encountered an error decoding zone config string: ${zoneConfigHexString}`,
278+
/* additional context */ undefined,
279+
e,
277280
);
278281
// Catch and assign the error if we encounter one decoding.
279282
resp.zoneConfigResp.error = e;

pkg/ui/workspaces/cluster-ui/src/api/tableDetailsApi.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { fromHexString, withTimeout } from "./util";
2626
import { Format, Identifier, Join, SQL } from "./safesql";
2727
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
2828
import { IndexUsageStatistic, recommendDropUnusedIndex } from "../insights";
29+
import { getLogger } from "../util";
2930

3031
const { ZoneConfig } = cockroach.config.zonepb;
3132
const { ZoneConfigurationLevel } = cockroach.server.serverpb;
@@ -314,8 +315,10 @@ const getTableZoneConfig: TableDetailsQuery<TableZoneConfigRow> = {
314315
);
315316
resp.zoneConfigResp.zone_config_level = configLevel;
316317
} catch (e) {
317-
console.error(
318+
getLogger().error(
318319
`Table Details API - encountered an error decoding zone config string: ${hexString}`,
320+
/* additional context */ undefined,
321+
e,
319322
);
320323
// Catch and assign the error if we encounter one decoding.
321324
resp.zoneConfigResp.error = e;

pkg/ui/workspaces/cluster-ui/src/loading/loading.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
Spinner,
1818
InlineAlertIntent,
1919
} from "@cockroachlabs/ui-components";
20-
import { adminUIAccess, isForbiddenRequestError } from "src/util";
20+
import { adminUIAccess, getLogger, isForbiddenRequestError } from "src/util";
2121
import styles from "./loading.module.scss";
2222
import { Anchor } from "../anchor";
2323

@@ -68,7 +68,14 @@ export const Loading: React.FC<LoadingProps> = props => {
6868
// Check for `error` before `loading`, since tests for `loading` often return
6969
// true even if CachedDataReducer has an error and is no longer really "loading".
7070
if (errors) {
71-
console.error(`Error Loading ${props.page}: ${errors}`);
71+
getLogger().error(
72+
errors.length === 1
73+
? `Error Loading ${props.page}`
74+
: `Multiple errors seen Loading ${props.page}: ${errors}`,
75+
/* additional context */ undefined,
76+
errors[0],
77+
);
78+
7279
// - map Error to InlineAlert props. RestrictedPermissions handled as "info" message;
7380
// - group errors by intend to show separate alerts per intent.
7481
const errorAlerts = chain(errors)

pkg/ui/workspaces/cluster-ui/src/store/uiConfig/uiConfig.sagas.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { getUserSQLRoles } from "../../api/userApi";
1414
import { CACHE_INVALIDATION_PERIOD, throttleWithReset } from "../utils";
1515
import { rootActions } from "../reducers";
1616
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
17+
import { getLogger } from "../../util";
1718

1819
export function* refreshUserSQLRolesSaga(): any {
1920
yield put(actions.requestUserSQLRoles());
@@ -26,7 +27,7 @@ export function* requestUserSQLRolesSaga(): any {
2627
);
2728
yield put(actions.receivedUserSQLRoles(result.roles));
2829
} catch (e) {
29-
console.warn(e.message);
30+
getLogger().warn(e.message, /* additional context */ undefined, e);
3031
}
3132
}
3233

pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleDropdown.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { defaultTimeScaleOptions, findClosestTimeScale } from "./utils";
2626

2727
import styles from "./timeScale.module.scss";
2828
import { TimezoneContext } from "../contexts";
29-
import { FormatWithTimezone } from "../util";
29+
import { FormatWithTimezone, getLogger } from "../util";
3030

3131
const cx = classNames.bind(styles);
3232

@@ -186,7 +186,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
186186
isMoving = true;
187187
break;
188188
default:
189-
console.error("Unknown direction: ", direction);
189+
getLogger().error("Unknown direction: ", direction);
190190
}
191191

192192
// If the timescale extends into the future then fallback to a default

pkg/ui/workspaces/cluster-ui/src/util/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export * from "./docs";
1717
export * from "./fixLong";
1818
export * from "./format";
1919
export * from "./formatDate";
20+
export * from "./logger";
2021
export * from "./requestError";
2122
export * from "./sql/summarize";
2223
export * from "./query";
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Copyright 2023 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the Business Source License
4+
// included in the file licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with
7+
// the Business Source License, use of this software will be governed
8+
// by the Apache License, Version 2.0, included in the file
9+
// licenses/APL.txt.
10+
11+
type JSONValue =
12+
| null
13+
| boolean
14+
| number
15+
| string
16+
| JSONValue[]
17+
| { [key: string]: JSONValue };
18+
19+
interface Logger {
20+
/**
21+
* Logs a message with additional context and an optional error at debug
22+
* level.
23+
* @param msg - the string message to log
24+
* @param context - additional structured context to include with the
25+
* message when shipped to a remote service
26+
* @param error - a possible JS Error to include with the message when
27+
* shipped to a remote service. Typed as unknown for convenience but
28+
* anything passed here which is not instanceof Error will not be attached.
29+
*/
30+
debug(
31+
msg: string,
32+
context?: Record<string, JSONValue>,
33+
error?: unknown,
34+
): void;
35+
/**
36+
* Logs a message with additional context and an optional error at info
37+
* level.
38+
* @param msg - the string message to log
39+
* @param context - additional structured context to include with the
40+
* message when shipped to a remote service
41+
* @param error - a possible JS Error to include with the message when
42+
* shipped to a remote service. Typed as unknown for convenience but
43+
* anything passed here which is not instanceof Error will not be attached.
44+
*/
45+
info(msg: string, context?: Record<string, JSONValue>, error?: unknown): void;
46+
/**
47+
* Logs a message with additional context and an optional error at warn
48+
* level.
49+
* @param msg - the string message to log
50+
* @param context - additional structured context to include with the
51+
* message when shipped to a remote service
52+
* @param error - a possible JS Error to include with the message when
53+
* shipped to a remote service. Typed as unknown for convenience but
54+
* anything passed here which is not instanceof Error will not be attached.
55+
*/
56+
warn(msg: string, context?: Record<string, JSONValue>, error?: unknown): void;
57+
/**
58+
* Logs a message with additional context and an optional error at error
59+
* level.
60+
* @param msg - the string message to log
61+
* @param context - additional structured context to include with the
62+
* message when shipped to a remote service
63+
* @param error - a possible JS Error to include with the message when
64+
* shipped to a remote service. Typed as unknown for convenience but
65+
* anything passed here which is not instanceof Error will not be attached.
66+
*/
67+
error(
68+
msg: string,
69+
context?: Record<string, JSONValue>,
70+
error?: unknown,
71+
): void;
72+
}
73+
74+
let logger: Logger = console;
75+
76+
/**
77+
* Sets the logger returned by {@link getLogger}. It was added to allow
78+
* cockroach cloud to pass in a custom logger which attaches additional metadata
79+
* to each call and sends errors up to datadog.
80+
* @param newLogger the Logger to set
81+
*/
82+
export function setLogger(newLogger: Logger) {
83+
logger = newLogger;
84+
}
85+
86+
/**
87+
* @returns the most recent logger set by {@link setLogger}, or console if one was never set.
88+
*/
89+
export function getLogger(): Logger {
90+
return logger;
91+
}

pkg/ui/workspaces/cluster-ui/src/util/versions.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
// by the Apache License, Version 2.0, included in the file
99
// licenses/APL.txt.
1010

11+
import { getLogger } from "./logger";
12+
1113
const versionPrefix = "10000";
1214
export type SemVersion = [number, number, number];
1315

@@ -51,7 +53,9 @@ export function parseStringToVersion(
5153
if (inputString.startsWith(versionPrefix)) {
5254
inputString = inputString.split(versionPrefix)[1];
5355
if (!inputString) {
54-
console.log("Unable to split version string while parsing", inputString);
56+
getLogger().warn(
57+
"Unable to split version string while parsing: " + inputString,
58+
);
5559
return [0, 0, 0];
5660
}
5761
}
@@ -68,7 +72,7 @@ export function parseStringToVersion(
6872

6973
return [parsedMajorVersion, parsedMinorVersion, parsedPatchVersion];
7074
} else {
71-
console.log("Version string did not match", inputString);
75+
getLogger().warn("Version string did not match: " + inputString);
7276
return [0, 0, 0];
7377
}
7478
}

0 commit comments

Comments
 (0)