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

ecs-pino-format with convertReqRes wrong http field format #102

Open
Donorlin opened this issue Sep 28, 2021 · 10 comments
Open

ecs-pino-format with convertReqRes wrong http field format #102

Donorlin opened this issue Sep 28, 2021 · 10 comments
Labels
agent-nodejs Make available for APM Agents project planning.

Comments

@Donorlin
Copy link

Hi,

I am using NestJS with nestjs-pino and I am trying to set up ecsFormat({ convertReqRes: true }). Logging of request and response works, but format is odd.
If this is meant for nestjs-pino i am sorry just let me know. Thank you

Versions I am using:
Node: 14.17.3
npm: 7.23.0
nestjs: latest setup
@elastic/ecs-pino-format: 1.3.0
nestjs-pino: 2.2.0
pino-http: 5.7.0

Usage

@Module({
    imports: [
        LoggerModule.forRoot({
            pinoHttp: { ...ecsFormat({ convertReqRes: true }) },
        }),
    ],
})
export class AppModule {}

In this picture, you can see ecsFormat with convertReqRes set to false behaves as expected with req and res logged in their separate fields.
convertReqRes-false

In this picture, convertReqRes was set to true. As you can see response field is in HTTP, but req is not and the log is missing url and user_agent fields (maybe others as well).
convertReqRes-true

Expected behavior

Correct format as shown in elastic/pino-http-logging

Thank you for your response, please let me know if this does not belong here.

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Sep 28, 2021
@trentm
Copy link
Member

trentm commented Sep 28, 2021

Hi. Thanks for the bug report. This is probably the right place to have reported. It may be a little while before I'm able to take a look. I haven't used NestJS before, so any getting started steps or a more complete reproduction case might help me out.

@Donorlin
Copy link
Author

I will try to prepare a repository with the whole setup tonight.

@Donorlin
Copy link
Author

Hi, here (https://github.com/Donorlin/ecs-pino-nest-issue) is a starter project. Sorry for the boilerplate. It was created with nest-cli.

It is as simple as npm i and npm run start:dev.

I have been messing around with your source file (https://github.com/elastic/ecs-logging-nodejs/blob/master/loggers/pino/index.js) and I found out that log function at the line 145 where you destructure the obj object does not contain req (it's always undefined). req is actually in res object.

When I tried to add something like

       const {
          res,
          err,
          ...ecsObj
        } = obj
        const req = obj.res?.req;

Log was actually filled with what would be expected. http field correctly contained response and request fields and even user_agent and other missing fields were present (not sure if all of them).

But log also still had req attribute.

Hopefully this helps. Let me know if I can help. I did not want to create any pull request because I do not know if it is intentional for res to contain req in log(obj), you probably have better insight than I do. If you want me to take a deeper look and create PR, just let me know.

Thank you

@exejutable
Copy link

any news on this?

@gkampitakis
Copy link

I had the same problem this is how I managed to solve it if anyone is interested https://stackoverflow.com/questions/69708256/http-pino-logger-and-elastic-common-schema-ecs-format-in-nestjs.

@exejutable
Copy link

I had the same problem this is how I managed to solve it if anyone is interested https://stackoverflow.com/questions/69708256/http-pino-logger-and-elastic-common-schema-ecs-format-in-nestjs.

So bassically you format the log manually?

@gkampitakis
Copy link

Exactly, I created a formater and merge it with the ecs.

@exejutable
Copy link

Exactly, I created a formater and merge it with the ecs.

Can you share the formater function 😃 ?

thanks in advance!

@gkampitakis
Copy link

https://gist.github.com/gkampitakis/b36819f38f8886598c20ed1af7245e3a here is a gist example of how we use it. I have removed some parts as we do specific formats for our needs. Hope this helps you to achieve what you want 🤞

@exejutable
Copy link

https://gist.github.com/gkampitakis/b36819f38f8886598c20ed1af7245e3a here is a gist example of how we use it. I have removed some parts as we do specific formats for our needs. Hope this helps you to achieve what you want 🤞

thanks, works like a charm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

No branches or pull requests

4 participants