diff --git a/packages/common/logger/createLogger.ts b/packages/common/logger/createLogger.ts index f7e8ba9ec..cd62a96d1 100644 --- a/packages/common/logger/createLogger.ts +++ b/packages/common/logger/createLogger.ts @@ -1,4 +1,5 @@ import * as Sentry from "@sentry/nextjs" +import { flatten } from "flat" const logLevels = ["debug", "info", "warn", "error"] as const const minimumLogLevel = (() => { @@ -23,6 +24,19 @@ function shouldLog(level: (typeof logLevels)[number]) { return logLevels.indexOf(level) >= logLevels.indexOf(minimumLogLevel) } +function getLogValue(args: unknown[]): Record | undefined { + if (!args || args.length === 0) { + return undefined + } + if (args.length === 1 && typeof args[0] === "object") { + return (args[0] as Record) ?? undefined + } + if (args.length === 1) { + return { value: args[0] } + } + return flatten(args) +} + export function createLogger(loggerPrefix: string | (() => Promise)) { const asyncWrapper: () => Promise = typeof loggerPrefix === "string" ? async () => loggerPrefix : loggerPrefix @@ -45,9 +59,12 @@ export function createLogger(loggerPrefix: string | (() => Promise)) { return } - Sentry.logger[level](`${await getLoggerPrefix()} ${message}`.trim(), { - ...args, - }) + const logValue = getLogValue(args) + + Sentry.logger[level]( + `${await getLoggerPrefix()} ${message}`.trim(), + logValue + ) console[level](`${await getLoggerPrefix()} ${message}`.trim(), ...args) } @@ -55,7 +72,6 @@ export function createLogger(loggerPrefix: string | (() => Promise)) { async debug(message: string, ...args: unknown[]): Promise { await log("debug", message, ...args) }, - async info(message: string, ...args: unknown[]): Promise { await log("info", message, ...args) }, diff --git a/packages/common/telemetry/index.test.ts b/packages/common/telemetry/index.test.ts index 1349e75b3..9666f02a9 100644 --- a/packages/common/telemetry/index.test.ts +++ b/packages/common/telemetry/index.test.ts @@ -42,18 +42,6 @@ describe("sanitize", () => { expect(sanitize(input)).toEqual(expected) }) - test("should stringify non-valid attributes", () => { - const input = { - key1: new Date("2024-08-08T12:00:00Z"), - key2: { nested: "object" }, - } - const expected = { - key1: '"2024-08-08T12:00:00.000Z"', - "key2.nested": "object", - } - expect(sanitize(input)).toEqual(expected) - }) - test("should handle nested valid attributes", () => { const input = { key1: "Example", @@ -82,54 +70,42 @@ describe("sanitize", () => { nestedKey1: "Value", nestedKey2: { nestedKey2Key1: true, - nestedKey2Key2: new Date("2024-08-08T12:00:00Z"), }, nestedKey3: { reallyNested: "hello", }, }, - nonPrimitive: new Date("2024-08-08T13:00:00Z"), } const expected = { key1: "Example", key2: 10, "nested.nestedKey1": "Value", "nested.nestedKey2.nestedKey2Key1": true, - "nested.nestedKey2.nestedKey2Key2": '"2024-08-08T12:00:00.000Z"', "nested.nestedKey3.reallyNested": "hello", - nonPrimitive: '"2024-08-08T13:00:00.000Z"', } expect(sanitize(input)).toEqual(expected) }) test("should throw an error when a function is passed", () => { + const key1 = () => {} const input = { - key1: () => {}, + key1, + } + const expected = { + key1, } - expect(() => sanitize(input)).toThrowError("Cannot sanitize function") - }) - - test("should throw an error when input not an object", () => { - // @ts-expect-error: array not allowed. We do this here to make sure the - // function not only relies on TS but actively blocks arrays as input. - expect(() => sanitize(null)).toThrowError() - - // @ts-expect-error: array not allowed. We do this here to make sure the - // function not only relies on TS but actively blocks arrays as input. - expect(() => sanitize(undefined)).toThrowError() - - // @ts-expect-error: array not allowed. We do this here to make sure the - // function not only relies on TS but actively blocks arrays as input. - expect(() => sanitize("")).toThrowError() - - // @ts-expect-error: array not allowed. We do this here to make sure the - // function not only relies on TS but actively blocks arrays as input. - expect(() => sanitize([1, 2, 3])).toThrowError() - }) - - test("should handle empty input", () => { - const input = {} - const expected = {} expect(sanitize(input)).toEqual(expected) }) + + test("should handle when input is not an object", () => { + expect(sanitize(null)).toEqual({}) + + expect(sanitize(undefined)).toEqual({}) + + expect(sanitize("")).toEqual({}) + + expect(sanitize([1, 2, 3])).toEqual({ "0": 1, "1": 2, "2": 3 }) + + expect(sanitize("Test string")).toEqual({ value: "Test string" }) + }) }) diff --git a/packages/common/telemetry/index.ts b/packages/common/telemetry/index.ts index bfce226b1..611afd465 100644 --- a/packages/common/telemetry/index.ts +++ b/packages/common/telemetry/index.ts @@ -1,11 +1,7 @@ // Central place for telemetry // TODO: Replace all of this with proper tracers and events -import { - type Attributes, - type AttributeValue, - metrics, -} from "@opentelemetry/api" +import { type Attributes, metrics } from "@opentelemetry/api" import deepmerge from "deepmerge" import { flatten } from "flat" @@ -13,43 +9,6 @@ import { logger } from "../logger" import type { ZodError } from "zod" -type AttributesInput = Record - -function isAttributesInput(value: unknown): value is AttributesInput { - return ( - isObject(value) && - !Array.isArray(value) && - !isNull(value) && - Object.keys(value).length > 0 && - Object.keys(value).every(isString) - ) -} - -/** - * Checks if a given value is a valid OpenTelemetry `AttributeValue`. - * An `AttributeValue` can be a `string`, `number`, `boolean`, or a homogenous - * array containing only `null`, `undefined`, `string`, `number`, or `boolean`. - * - * @param value The value to check. - * @returns `true` if the value is a valid `AttributeValue`, `false` otherwise. - */ -export function isValidAttributeValue(value: unknown): value is AttributeValue { - if (isString(value) || isNumber(value) || isBoolean(value)) { - return true - } - if (Array.isArray(value)) { - return value.every( - (item) => - isNull(item) || - isUndefined(item) || - isString(item) || - isNumber(item) || - isBoolean(item) - ) - } - return false -} - /** * Sanitizes an input object, ensuring its values are valid OpenTelemetry * `AttributeValue` or `JSON.stringify()` representations as a fallback. @@ -84,24 +43,12 @@ export function isValidAttributeValue(value: unknown): value is AttributeValue { * // } * ``` */ -export function sanitize(data: AttributesInput): Attributes { - if (!isPlainObject(data)) { - throw new Error(`Input must be an object, got ${JSON.stringify(data)}`) +export function sanitize(data: any): Attributes { + if (!data) return {} + if (typeof data === "string") { + return { value: data } } - - return flatten( - mapValues(data, (value) => { - if (isFunction(value)) { - throw new Error("Cannot sanitize function") - } else if (isValidAttributeValue(value)) { - return value - } else if (isAttributesInput(value)) { - return sanitize(value) - } - - return JSON.stringify(value) - }) - ) + return flatten(data) } /** @@ -137,19 +84,19 @@ export function createCounter(meterName: string, counterName: string) { * @param baseAttrs - The base attributes to associate with the counter. Defaults to an empty object. * @returns An object with methods to record specific counter events. */ - init(baseAttrs: AttributesInput = {}) { + init(baseAttrs: object = {}) { return { /** * Records an event for the main counter. * * @param attrs - Additional attributes specific to this 'start' event. Defaults to an empty object. */ - start(attrs: AttributesInput = {}) { - const mergedAttrs = deepmerge.all([baseAttrs, attrs]) + start(attrs: object = {}) { + const mergedAttrs = deepmerge.all([baseAttrs, attrs]) const finalAttrs = sanitize(mergedAttrs) counter.add(1, finalAttrs) - logger.debug(`[${fullName}] start:`, finalAttrs) + logger.debug(`[${fullName}] start`, mergedAttrs) }, /** @@ -157,12 +104,13 @@ export function createCounter(meterName: string, counterName: string) { * * @param attrs - Additional attributes specific to this 'success' event. Defaults to an empty object. */ - success(attrs: AttributesInput = {}) { - const mergedAttrs = deepmerge.all([baseAttrs, attrs]) + success(attrs: object = {}) { + console.log("telemetry attrs:", attrs, baseAttrs) + const mergedAttrs = deepmerge.all([baseAttrs, attrs]) const finalAttrs = sanitize(mergedAttrs) success.add(1, finalAttrs) - logger.debug(`[${fullName}] success:`, finalAttrs) + logger.debug(`[${fullName}] success`, mergedAttrs) }, /** @@ -174,8 +122,8 @@ export function createCounter(meterName: string, counterName: string) { * @param errorMsg - A message describing the data error. * @param attrs - Additional attributes specific to this 'dataError' event. Defaults to an empty object. */ - dataError(errorMsg: string, attrs: AttributesInput = {}) { - const mergedAttrs = deepmerge.all([ + dataError(errorMsg: string, attrs: object = {}) { + const mergedAttrs = deepmerge.all([ baseAttrs, attrs, { @@ -186,7 +134,7 @@ export function createCounter(meterName: string, counterName: string) { const finalAttrs = sanitize(mergedAttrs) fail.add(1, finalAttrs) - logger.error(`[${fullName}] dataError:`, finalAttrs) + logger.error(`[${fullName}] dataError: ${errorMsg}`, mergedAttrs) }, /** @@ -197,8 +145,8 @@ export function createCounter(meterName: string, counterName: string) { * * @param attrs - Additional attributes specific to this 'noDataError' event. Defaults to an empty object. */ - noDataError(attrs: AttributesInput = {}) { - const mergedAttrs = deepmerge.all([ + noDataError(attrs: object = {}) { + const mergedAttrs = deepmerge.all([ baseAttrs, attrs, { @@ -208,7 +156,7 @@ export function createCounter(meterName: string, counterName: string) { const finalAttrs = sanitize(mergedAttrs) fail.add(1, finalAttrs) - logger.error(`[${fullName}] noDataError:`, finalAttrs) + logger.error(`[${fullName}] noDataError:`, mergedAttrs) }, /** @@ -218,17 +166,17 @@ export function createCounter(meterName: string, counterName: string) { * @param zodError - The {@link ZodError} object representing the validation error. */ validationError(zodError: ZodError) { - const mergedAttrs = deepmerge.all([ + const mergedAttrs = deepmerge.all([ baseAttrs, { error_type: "validation_error", - error: zodError.format(), + ...sanitize(zodError.format()), }, ]) const finalAttrs = sanitize(mergedAttrs) fail.add(1, finalAttrs) - logger.error(`[${fullName}] validationError:`, finalAttrs) + logger.error(`[${fullName}] validationError`, mergedAttrs) }, /** @@ -244,21 +192,22 @@ export function createCounter(meterName: string, counterName: string) { const res = response.clone() const text = await res.text() - const mergedAttrs = deepmerge.all([ + const mergedAttrs = deepmerge.all([ baseAttrs, { error_type: "http_error", - error: { - status: res.status, - statusText: res.statusText, - text, - }, + "error.status": res.status, + "error.statusText": res.statusText, + "error.text": text, }, ]) const finalAttrs = sanitize(mergedAttrs) fail.add(1, finalAttrs) - logger.error(`[${fullName}] httpError:`, finalAttrs) + logger.error( + `[${fullName}] httpError ${res.status}, ${res.statusText}:`, + mergedAttrs + ) }, /** @@ -278,7 +227,7 @@ export function createCounter(meterName: string, counterName: string) { msg = err } - const mergedAttrs = deepmerge.all([ + const mergedAttrs = deepmerge.all([ baseAttrs, { error_type: "error", @@ -288,37 +237,9 @@ export function createCounter(meterName: string, counterName: string) { const finalAttrs = sanitize(mergedAttrs) fail.add(1, finalAttrs) - logger.error(`[${fullName}] fail:`, finalAttrs) + logger.error(`[${fullName}] fail message: ${msg}`, mergedAttrs) }, } }, } } - -const isBoolean = (v: unknown): v is boolean => typeof v === "boolean" -const isFunction = (v: unknown): v is Function => typeof v === "function" -const isNull = (v: unknown): v is null => v === null -const isNumber = (v: unknown): v is number => - typeof v === "number" && Number.isFinite(v as number) -const isObject = (v: unknown): v is Record => - typeof v === "object" && v !== null -const isString = (v: unknown): v is string => typeof v === "string" -const isUndefined = (v: unknown): v is undefined => typeof v === "undefined" -const isPlainObject = (v: unknown): v is Record => { - if (!isObject(v)) return false - const proto = Object.getPrototypeOf(v) - return proto === Object.prototype || proto === null -} - -const mapValues = , R>( - obj: T, - iteratee: (value: T[keyof T], key: keyof T) => R -): Record => { - const out: Partial> = {} - for (const k in obj) { - if (Object.prototype.hasOwnProperty.call(obj, k)) { - out[k] = iteratee(obj[k], k) - } - } - return out as Record -}