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

How to serialize the whole object, not properties? #362

Closed
PavelPolyakov opened this issue Feb 20, 2018 · 13 comments
Closed

How to serialize the whole object, not properties? #362

PavelPolyakov opened this issue Feb 20, 2018 · 13 comments
Labels

Comments

@PavelPolyakov
Copy link
Contributor

PavelPolyakov commented Feb 20, 2018

Hello,

Please, advice what is expected way to achieve the desired behaviour.

I use pino through fastify, therefore I use it usually through the request directly:

request.log.info('message');
request.log.error(error);

Imagine the next code:

const axios = require('axios');

(async () => {
    try {
        const r = await axios.get('https://google.com/404');
    }catch(error) {
        console.log(error);
    }
})()

axios will throw the error with the GIANT object, like:
https://pastebin.com/ZXccMRvw

pino will try to log it, while I want to work on the serialisation of the logged object.

BUT, serializers object supports only per property serialization. In my case these are three properties request, response, config only the fact that three of them are present is the evidence of the axios error.

serializers (object): an object containing functions for custom serialization of objects. These functions should return an JSONifiable object and they should never throw. When logging an object, each top-level property matching the exact key of a serializer will be serialized using the defined serializer.

How can I put the logic on top three of them together, not one by one? Let's say if only config is present - I want to log it as is.

I actually achieved it with prettyPrint, but when I tried to test this using unit-tests, and declared the custom write stream - pino init fails, since prettyPrint is not available if you are not using process.stdout.

What can be done about it?

Regards,

@jsumners jsumners added the bug label Feb 20, 2018
@jsumners
Copy link
Member

This is the same thing as pinojs/pino-http#42 (pinojs/pino-http#43). The solution is to split out the standard serializers into pino-std-serializers. This will also solve #265.

I'll start work on this.

@PavelPolyakov
Copy link
Contributor Author

PavelPolyakov commented Feb 20, 2018

@jsumners
I might be wrong, but it seems that you are talking about placing the serializers aside, into the separate module. (how would it help? in axios error the property is called request, while standard serialiser is req)

While my question is more like - maybe we should have the control over the whole object serialisation. Like providing the option to register serializer called * (or array of them) which will handle the whole object?

@jsumners
Copy link
Member

  1. Pino would be updated to use the pino-std-serializers package to supply the fixed serializers.
  2. Fastify would be updated to use the new version of Pino

The solution is merely centralizing this code so that we don't have to update it in 15 places every time someone finds a bug.

@jsumners
Copy link
Member

As for:

While my question is more like - maybe we should have the control over the whole object serialisation. Like providing the option to register serializer called * (or array of them) which will handle the whole object?

You can supply your own serializers any time you like -- https://getpino.io/#/docs/API?id=constructor (serializers property). Fastify supports passing them along as well.

@PavelPolyakov
Copy link
Contributor Author

@jsumners
True, but these serealizers are invoked ONLY if property matches, right?
While I think it's reasonable to have an option to serialize the WHOLE object.

in my example I made an assumption that I want to do certain serializations ONLY if all the three parameters are present in the object config, request, response. So by testing this, I assume this is axios error log and I want to do something specific with it.

This is not achievable currently, if I'm right, because current serialiizer implementation is 1 to 1 (object which is being logged property vs serializers.property)

@jsumners
Copy link
Member

I'm sorry, I do not understand what you are asking.

@PavelPolyakov
Copy link
Contributor Author

no problem, code speaks better:

const pino = require('pino')({
    serializers: {
        // this works, if a is there, it will be invoked
        a: function(v) {
            console.log(arguments);
            return {I: "am", not: "the a", you: "are", looking: "for"}
        },
        // this is not real, this is what I suggest to have
        '*': function(obj) {
            if(obj.b == 99) {
                obj.a = 88;
                return obj;
            }

            return obj;
        }
    }
});

pino.info({a:'hello'});
pino.info({a:'hello', b: 99});

What I'm articulating right now, that with the current structure of the serializers it's not possible to serialise something depending on the other something. While I think this can be useful.

@mcollina
Copy link
Member

I think this should be closed as well.

@PavelPolyakov
Copy link
Contributor Author

@mcollina
my question/suggestion above was - should we be able to build serialisation logic based on the complete object, not per-properties?

@mcollina mcollina reopened this Feb 23, 2018
@mcollina
Copy link
Member

@PavelPolyakov that's interesting. Would you like to send a PR?

@PavelPolyakov
Copy link
Contributor Author

@mcollina
don't have a certain plans for this, but if this feature is considered as required, I might do that at some point.

for now I solved my issue by conditioning in properties serialisers.

@jsumners
Copy link
Member

jsumners commented Apr 5, 2018

Solved by #365.

@jsumners jsumners closed this as completed Apr 5, 2018
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants