Skip to content
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

chore: add request/response metadata to logs #5728

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Conversation

hobbsh
Copy link
Contributor

@hobbsh hobbsh commented Dec 9, 2024

Making inferences from the logs is difficult at the moment because the request/response metadata is not structured (just dumped to message). This makes the logs much more searchable/sortable by changing the log format from this:

{"meta":{},"level":"info","message":"HTTP 200 GET /UI_REVISION 663ms"}

To this:

{"meta":{"req":{"url":"/UI_REVISION","headers":{"host":"localhost:8888"}},"res":{"statusCode":200},"responseTime":221},"level":"info","message":"HTTP 200 GET /UI_REVISION 221ms"}

@hobbsh hobbsh requested review from a team December 9, 2024 16:52
Copy link
Collaborator

@mcstover mcstover left a comment

Choose a reason for hiding this comment

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

Noting, the format we currently use was set to match other log formats across our stack. Are we / should we change those as well? CPS and Monolith come to mind.

There's also a util method that is used widely that we may want to start using differently in cases where this type of meta is available.

@hobbsh
Copy link
Contributor Author

hobbsh commented Dec 9, 2024

Noting, the format we currently use was set to match other log formats across our stack. Are we / should we change those as well? CPS and Monolith come to mind.

There's also a util method that is used widely that we may want to start using differently in cases where this type of meta is available.

So the monolith has this metadata accessible since the Caddy logs are fully structured JSON with individual request/response fields, which we can easily pull out in Loki. I am not overly concerned about everything being in the same exact format, more that we are able to do JSON field extraction in a straightforward way. So short answer, yes I think we should also update CPS so we can more easily extract request/response fields. Our Kotlin services could also probably use an update.

The reason this came up at all was because I was trying to correlate specific request urls with extended slowness in the application last night.

@hobbsh hobbsh merged commit 552ace7 into main Dec 9, 2024
5 checks passed
@kiva-robot
Copy link
Collaborator

🎉 This PR is included in version 3.6.0-rc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@kiva-robot
Copy link
Collaborator

🎉 This PR is included in version 3.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants