Merged in feat/sw-3560-sanitize2 (pull request #3001)

feat(SW-3560): send parameters to sentry logs

* feat(SW-3560): send parameters to sentry logs

* Use flatten instead of sanitize when logging


Approved-by: Joakim Jäderberg

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<string, unknown> | undefined {
+  if (!args || args.length === 0) {
+    return undefined
+  }
+  if (args.length === 1 && typeof args[0] === "object") {
+    return (args[0] as Record<string, unknown>) ?? undefined
+  }
+  if (args.length === 1) {
+    return { value: args[0] }
+  }
+  return flatten(args)
+}
+
 export function createLogger(loggerPrefix: string | (() => Promise<string>)) {
   const asyncWrapper: () => Promise<string> =
     typeof loggerPrefix === "string" ? async () => loggerPrefix : loggerPrefix
@@ -45,9 +59,12 @@ export function createLogger(loggerPrefix: string | (() => Promise<string>)) {
       return
     }
 
-    Sentry.logger[level](`${await getLoggerPrefix()} ${message}`.trim(), {
+    const logValue = getLogValue(args)
-      ...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<string>)) {
     async debug(message: string, ...args: unknown[]): Promise<void> {
       await log("debug", message, ...args)
     },
-
     async info(message: string, ...args: unknown[]): Promise<void> {
       await log("info", message, ...args)
     },

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" })
+  })
 })

packages/common/telemetry/index.ts

@@ -1,11 +1,7 @@
 // Central place for telemetry
 // TODO: Replace all of this with proper tracers and events
 
-import {
+import { type Attributes, metrics } from "@opentelemetry/api"
-  type Attributes,
-  type AttributeValue,
-  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<string, unknown>
-
-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 {
+export function sanitize(data: any): Attributes {
-  if (!isPlainObject(data)) {
+  if (!data) return {}
-    throw new Error(`Input must be an object, got ${JSON.stringify(data)}`)
+  if (typeof data === "string") {
+    return { value: data }
   }
-
+  return flatten(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)
-    })
-  )
 }
 
 /**
@@ -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 = {}) {
+        start(attrs: object = {}) {
-          const mergedAttrs = deepmerge.all<AttributesInput>([baseAttrs, attrs])
+          const mergedAttrs = deepmerge.all<object>([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 = {}) {
+        success(attrs: object = {}) {
-          const mergedAttrs = deepmerge.all<AttributesInput>([baseAttrs, attrs])
+          console.log("telemetry attrs:", attrs, baseAttrs)
+          const mergedAttrs = deepmerge.all<object>([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 = {}) {
+        dataError(errorMsg: string, attrs: object = {}) {
-          const mergedAttrs = deepmerge.all<AttributesInput>([
+          const mergedAttrs = deepmerge.all<object>([
             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 = {}) {
+        noDataError(attrs: object = {}) {
-          const mergedAttrs = deepmerge.all<AttributesInput>([
+          const mergedAttrs = deepmerge.all<object>([
             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<AttributesInput>([
+          const mergedAttrs = deepmerge.all<object>([
             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<AttributesInput>([
+          const mergedAttrs = deepmerge.all<object>([
             baseAttrs,
             {
               error_type: "http_error",
-              error: {
+              "error.status": res.status,
-                status: res.status,
+              "error.statusText": res.statusText,
-                statusText: res.statusText,
+              "error.text": 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<AttributesInput>([
+          const mergedAttrs = deepmerge.all<object>([
             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<string, unknown> =>
-  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<string, unknown> => {
-  if (!isObject(v)) return false
-  const proto = Object.getPrototypeOf(v)
-  return proto === Object.prototype || proto === null
-}
-
-const mapValues = <T extends Record<string, any>, R>(
-  obj: T,
-  iteratee: (value: T[keyof T], key: keyof T) => R
-): Record<keyof T, R> => {
-  const out: Partial<Record<keyof T, R>> = {}
-  for (const k in obj) {
-    if (Object.prototype.hasOwnProperty.call(obj, k)) {
-      out[k] = iteratee(obj[k], k)
-    }
-  }
-  return out as Record<keyof T, R>
-}