Skip to content

feat: add request id to default log level #1221

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
May 2, 2025

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Apr 30, 2025

Problem

requestId is the single most important piece of information for the backend team in order to effectively debug these issues and its only visible at the trace level.

Solution

log it by default at info level, along with the rest of the response metadata.

Example Logs

2025-04-30 19:06:55.971 [info] [Info  - 7:06:55 PM] [2025-04-30T23:06:55.966Z] generateAssistantResponse Response: {
  "httpStatusCode": 200,
  "requestId": "1e8a80f1-4d78-47fe-933d-ffe9f29efafe",
  "attempts": 1,
  "totalRetryDelay": 0
}

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -559,8 +559,9 @@ export class AgenticChatController implements ChatHandlers {
// Phase 3: Request Execution
this.#validateRequest(currentRequestInput)
const response = await session.generateAssistantResponse(currentRequestInput)
this.#debug(`Q Model Response: ${loggingUtils.formatObj(response, { depth: 5 })}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we still keep this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is any of the non-metadata info useful? This is what I see:

{
  "$metadata": {
    "httpStatusCode": 200,
    "requestId": "046690bd-6d69-4e1f-a68a-8f302dfd0264",
    "attempts": 1,
    "totalRetryDelay": 0
  },
  "conversationId": "",
  "generateAssistantResponseResponse": {
    "options": {
      "messageStream": {
        "options": {
          "inputStream": {},
          "decoder": {
            "headerMarshaller": {},
            "messageBuffer": [],
            "isEndOfStream": false
          }
        }
      }
    }
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are right, they are not useful because the response is being streamed back, we can remove it here. Do we have logs in response streaming (during parsing)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have some for when we identify tool use events, and when errors occurs. See implementation here.

Are there any other important cases that might be helpful for debugging?

this.#debug(`Q Model Response: ${loggingUtils.formatObj(response, { depth: 5 })}`)

this.#features.logging.info(
`generateAssistantResponse Response: ${loggingUtils.formatObj(response.$metadata)}`
Copy link
Contributor

@jguoamz jguoamz Apr 30, 2025

Choose a reason for hiding this comment

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

nit: generateAssistantResponse ResponseMetadata: ${loggingUtils.formatObj(response.$metadata)}

@Hweinstock Hweinstock marked this pull request as ready for review May 1, 2025 00:11
@Hweinstock Hweinstock requested a review from a team as a code owner May 1, 2025 00:11
@Hweinstock Hweinstock marked this pull request as draft May 1, 2025 03:09
@Hweinstock Hweinstock marked this pull request as ready for review May 1, 2025 20:55
@Hweinstock Hweinstock merged commit fe31f26 into aws:main May 2, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants