Skip to content

Commit

Permalink
refactor(logger): log some objects at higher depth #5984
Browse files Browse the repository at this point in the history
## Problem
Deeply nested objects fail to show up in the logger due to use of `%O`
instead of `JSON.stringify`. Some customers use this information for
debugging or other purposes.
Ex. 
``` 
2024-11-12 13:23:39.682 [info] request from tab: tab-1 conversationID: 619df9a2-3ab2-4676-9896-9eb608d80582 request: {
  conversationState: {
    currentMessage: { userInputMessage: [Object] },
    chatTriggerType: 'MANUAL',
    customizationArn: undefined
  }
}
```

## Solution
- Change codewhisper chat request/response log messages (which are very
deeply nested) back to `JSON.stringify`.
  • Loading branch information
Hweinstock authored Nov 13, 2024
1 parent b173d6c commit b753e82
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { globals, waitUntil } from '../../../shared'
import { telemetry } from '../../../shared/telemetry'
import { Auth } from '../../../auth'
import { isSsoConnection } from '../../../auth/connection'
import { inspect } from '../../../shared/utilities/collectionUtils'

export interface ChatControllerMessagePublishers {
readonly processPromptChatMessage: MessagePublisher<PromptMessage>
Expand Down Expand Up @@ -656,7 +657,11 @@ export class ChatController {

const request = triggerPayloadToChatRequest(triggerPayload)
const session = this.sessionStorage.getSession(tabID)
getLogger().info(`request from tab: ${tabID} conversationID: ${session.sessionIdentifier} request: %O`, request)
getLogger().info(
`request from tab: ${tabID} conversationID: ${session.sessionIdentifier} request: ${inspect(request, {
depth: 12,
})}`
)
let response: MessengerResponseType | undefined = undefined
session.createNewTokenSource()
try {
Expand All @@ -681,8 +686,7 @@ export class ChatController {
getLogger().info(
`response to tab: ${tabID} conversationID: ${session.sessionIdentifier} requestID: ${
response.$metadata.requestId
} metadata: %O`,
response.$metadata
} metadata: ${inspect(response.$metadata, { depth: 12 })}`
)
await this.messenger.sendAIResponse(response, session, tabID, triggerID, triggerPayload)
} catch (e: any) {
Expand Down
15 changes: 14 additions & 1 deletion packages/core/src/shared/utilities/collectionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { isWeb } from '../extensionGlobals'
import { inspect as nodeInspect } from 'util'
import { AsyncCollection, toCollection } from './asyncCollection'
import { SharedProp, AccumulableKeys, Coalesce, isNonNullable } from './tsUtils'

Expand Down Expand Up @@ -297,7 +299,6 @@ export function assign<T extends Record<any, any>, U extends Partial<T>>(data: T
* - depth=2 returns `obj` with its children and their children.
* - and so on...
*
* TODO: node's `util.inspect()` function is better, but doesn't work in web browser?
*
* @param obj Object to clone.
* @param depth
Expand Down Expand Up @@ -329,6 +330,18 @@ export function partialClone(obj: any, depth: number = 3, omitKeys: string[] = [
return clonedObj
}

/**
* Wrapper around nodes inspect function that works on web. Defaults to JSON.stringify on web.
* @param obj object to show
* @param opt options for showing (ex. depth, omitting keys)
*/
export function inspect(obj: any, opt?: { depth: number }): string {
const options = {
depth: opt?.depth ?? 3,
}
return isWeb() ? JSON.stringify(partialClone(obj, options.depth), undefined, 2) : nodeInspect(obj, options)
}

/** Recursively delete undefined key/value pairs */
export function stripUndefined<T extends Record<string, any>>(
obj: T
Expand Down
35 changes: 33 additions & 2 deletions packages/core/src/test/shared/utilities/collectionUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
joinAll,
isPresent,
partialClone,
inspect,
} from '../../../shared/utilities/collectionUtils'

import { asyncGenerator } from '../../../shared/utilities/collectionUtils'
Expand Down Expand Up @@ -511,7 +512,7 @@ describe('CollectionUtils', async function () {
const requester = async (request: { next?: string }) => pages[request.next ?? 'page1']

it('creates a new AsyncCollection', async function () {
const collection = pageableToCollection(requester, {}, 'next', 'data')
const collection = pageableToCollection(requester, {}, 'next' as never, 'data')
assert.deepStrictEqual(await collection.promise(), [[0, 1, 2], [3, 4], [5], []])
})

Expand Down Expand Up @@ -540,7 +541,7 @@ describe('CollectionUtils', async function () {

describe('last', function () {
it('it persists last element when mapped', async function () {
const collection = pageableToCollection(requester, {}, 'next', 'data')
const collection = pageableToCollection(requester, {}, 'next' as never, 'data')
const mapped = collection.map((i) => i[0] ?? -1)
assert.strictEqual(await last(mapped), -1)
})
Expand Down Expand Up @@ -679,6 +680,36 @@ describe('CollectionUtils', async function () {
})
})

describe('inspect', function () {
let testData: any
before(function () {
testData = {
root: {
A: {
B: {
C: {
D: {
E: 'data',
},
},
},
},
},
}
})

it('defaults to a depth of 3', function () {
assert.strictEqual(inspect(testData), '{\n root: { A: { B: { C: [Object] } } }\n}')
})

it('allows depth to be set manually', function () {
assert.strictEqual(
inspect(testData, { depth: 6 }),
"{\n root: {\n A: {\n B: { C: { D: { E: 'data' } } }\n }\n }\n}"
)
})
})

describe('partialClone', function () {
it('omits properties by depth', function () {
const testObj = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { AST_NODE_TYPES, ESLintUtils, TSESTree } from '@typescript-eslint/utils'
import { Rule } from 'eslint'

export const errMsg =
'Avoid using JSON.stringify within logging and error messages, prefer %O. Note: %O has a depth limit of 2'
'Avoid using JSON.stringify within logging and error messages, prefer %O in general or inspect from collectionUtils for custom depth formatting'

/**
* Check if a given expression is a JSON.stringify call.
Expand Down

0 comments on commit b753e82

Please sign in to comment.