feat(structured-logger): add @hono/structured-logger middleware#1782
feat(structured-logger): add @hono/structured-logger middleware#1782gabry-ts wants to merge 6 commits intohonojs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: ca5e4f2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/structured-logger/README.md
Outdated
| | `createLogger` | `(c: Context) => L` | Yes | | Factory that creates a request scoped logger instance | | ||
| | `contextKey` | `string` | No | `'logger'` | Key used to store the logger on `c.var` | | ||
| | `onRequest` | `(logger: L, c: Context) => void \| Promise<void>` | No | Logs method + path at info level | Called before handler execution | | ||
| | `onResponse` | `(logger: L, c: Context, elapsedMs: number) => void \| Promise<void>` | No | Logs status + elapsed at info level | Called after handler execution | |
packages/structured-logger/README.md
Outdated
| | Option | Type | Required | Default | Description | | ||
| |---|---|---|---|---| | ||
| | `createLogger` | `(c: Context) => L` | Yes | | Factory that creates a request scoped logger instance | | ||
| | `contextKey` | `string` | No | `'logger'` | Key used to store the logger on `c.var` | |
There was a problem hiding this comment.
would the API maybe be simpler if you merge contextKey option into createLogger? like:
createLogger: (c) => ({ logger: rootLogger.child({ requestId: c.var.requestId }) })
could maybe be more flexible, but also more verbose. But I think I prefer your approach...
| } | ||
|
|
||
| function defaultOnError<L extends BaseLogger>(logger: L, err: Error, c: Context): void { | ||
| logger.error({ err, method: c.req.method, path: c.req.path }, 'request error') |
There was a problem hiding this comment.
maybe also log the status code here as it could be modified by hono's onError handler
| } | ||
|
|
||
| function defaultOnResponse<L extends BaseLogger>(logger: L, c: Context, elapsedMs: number): void { | ||
| logger.info({ status: c.res.status, elapsedMs }, 'request end') |
There was a problem hiding this comment.
I would prefer to include method+path also here, so one does not need to always combine this log line with the 'request start' log line to do analytics. This would also more align defaultOnResponse and defaultOnError. But it could also be a personal preference and likely everyone will modify those settings for a production setup.
|
Thanks for the review.
Let me know if you have any other feedback. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1782 +/- ##
==========================================
+ Coverage 91.71% 91.77% +0.05%
==========================================
Files 113 114 +1
Lines 3779 3804 +25
Branches 956 964 +8
==========================================
+ Hits 3466 3491 +25
Misses 281 281
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
How about adding the introduction for adding type support for c.var.logger?
Like this:
import { Hono } from 'hono'
import { structuredLogger } from '@hono/structured-logger'
import pino from 'pino'
const rootLogger = pino()
const app = new Hono<{
Variables: {
logger: ReturnType<typeof rootLogger.child>
}
}>()
app.use(
structuredLogger({
createLogger: (c) => rootLogger.child({ foo: 'bar' })
})
)
app.get('/', (c) => {
c.var.logger.info('hello') // typed!
return c.json({ foo: 'bar' })
})There was a problem hiding this comment.
@gabry-ts Ahh, sorry! I missed the Type safe context section you wrote. Passing pino.Logger is better than ReturnType<typeof rootLogger.child>. My request is not necessary. Can you revert the change?
3 participants
Summary
Adds a new structured logging middleware for Hono that provides a request scoped logger on c.var.logger.
This middleware is library agnostic and works with pino, winston, bunyan, console, or any logger implementing the BaseLogger interface. It has zero dependencies and is compatible with all runtimes supported by Hono.
Core features:
Related: honojs/hono#3963
Test plan