Open guy-frontegg opened 1 year ago
iamolegga
commented
1 year ago If you create a bug issue you have to provide minimal example repo with the bug. Unfortunately I'm unable to to guess what's misconfigured in your particular case. 90% of issues is modules order (pino should go first). Also useExisting should be just skipped if you provide configuration for pino via this module and not using existing fastify's logger
guy-frontegg
commented
1 year ago I am sorry, it was on our private repo. I will try to create a repo with valid example. Regaring the useExisting we tried all just to verify this was not the issue. I already asked the Pino and the Pino-http guys and they said it's not an issue with their library.
guy-frontegg
commented
1 year ago Hi @iamolegga , I've forked your wonderful repo and changed the example to use Fastify as you requested. https://github.com/guy-frontegg/nestjs-pino/tree/nestify-test I changed the genReqId as follows
@Module({
imports: [
LoggerModule.forRoot({ pinoHttp: {
level: process.env.LOG_LEVEL,
quietReqLogger: true,
genReqId: (req) => {
return 'hello';
}
}
})
],
controllers: [AppController],
providers: [MyService]
})
export class AppModule {}Once the example is running you can see the log output
{"level":50,"time":1677579321632,"pid":21361,"hostname":"GuyL-Mac-8.local","reqId":"req-1","context":"AppController","foo":"bar","msg":"baz qux"}I hope this helps
iamolegga
commented
1 year ago Thanks, will get back to this soon
saltedsword
commented
1 year ago i'm stuck on this too, waiting for your good news!
Thanks, will get back to this soon
iamolegga
commented
1 year ago Sorry, still struggling to find time for this as I'm not using the fastify adapter (and I recommend switching to express if possible, it works more stable and has fewer open issues for this lib). In the meantime PRs are welcome.
ghost
commented
1 year ago Hi everyone, I found a root cause of this.
Fastify generates the res.id on its own, and in pino-http library, they have a check. If id already exists - then use it, otherwise generate one using a default method or custom.
also created a simple test in nest-pino, probably will be useful for someone
import { Controller, Get } from '@nestjs/common';
import { PinoLogger } from '../src';
import { platforms } from './utils/platforms';
import { TestCase } from './utils/test-case';
describe('override gen req id method', () => {
for (const PlatformAdapter of [platforms[1]]) {
describe(PlatformAdapter.name, () => {
it('check the gen req id is matched with overwritten', async () => {
@Controller('/')
class TestController {
constructor(private readonly logger: PinoLogger) {}
@Get()
get() {
// need to work in general, but due to some code specific logic in pino-http, not working in fastify, because fastify setting a default id for req on it's own
expect(this.logger.logger.bindings().req.id).toEqual(1);
}
}
const logs = await new TestCase(new PlatformAdapter(), {
controllers: [TestController],
})
.forRoot({
pinoHttp: {
genReqId: (req, res) => {
return 1;
},
quietReqLogger: true,
customAttributeKeys: {
reqId: 'requestId'
}
}
})
.run();
console.log()
});
});
}
});regarding the workaround, in my case, I'm just implementing genReqId function in fastify adapter itself.
const app = await NestFactory.create<NestFastifyApplication>(
AppModule,
new FastifyAdapter({
genReqId: (req) => {
return '1';
},
}),
{},
);
What is the current behavior?
Hello I've been trying to replace our logger in NestJs to Pino using Nestjs-pino.
Tested it on the example project and it's working but our production project it's not working. We are using NestJs 9 with Fastify Am I missing something?
This is the output
What is the expected behavior? I expect to see requestId as 'temp'
Please mention other relevant information such as Node.js version and Operating System.