serdnam / pino-cloudwatch-transport

MIT License
18 stars 3 forks source link

Does not work with timestamp formatting options #5

Open noe opened 1 year ago

noe commented 1 year ago

Pino allows to set the format of the timestamp by specifying e.g.

const logger = pino(
    {
        level: ...,
        timestamp: pino.stdTimeFunctions.isoTime, // <-- makes pino-cloudwatch-transport fail
    },
    {
        targets: [{
            target: '@serdnam/pino-cloudwatch-transport',
            options: { ... }
        }]
    }
);

However, it seems that pino-cloudwatch-transport assumes the time property to be a number, because it does not send any message to AWS Cloudwatch if the above time format property is present.

serdnam commented 1 year ago

Thanks for writing an issue! I was thinking of maybe doing a typeof number check, but that would break if stdTimeFunctions.unixTIme is used.

https://getpino.io/#/docs/api?id=pino-stdtimefunctions

I'm thinking of maybe adding an optional option of type string | false, if it is set to false, all logs will just be sent with Date.now() in the timestamp; otherwise, if it is a string, it will look for a property with the given property name on the log line, would this work for your use case?

noe commented 1 year ago

If it can't work out of the box (I assume that it's because of the need to order the events based on the time property), I would prefer to have a new option mimicking the timestamp option from pino, that is, accepting a function to format the time. Something like this:


const transport = pino.transport({
    target: '@serdnam/pino-cloudwatch-transport',
    options: {
        logGroupName: 'pino-cloudwatch-test',
        logStreamName: 'pino-cloudwatch-test-stream',
        awsRegion: process.env.AWS_REGION,
        awsAccessKeyId: process.env.AWS_ACCESS_KEY_ID,
        awsSecretAccessKey: process.env.AWS_SECRET_ACCESS_KEY,
        interval: 1_000,
        timestamp: pino.stdTimeFunctions.isoTime, // <--- suggested new option
    }
});

By the way, thanks a lot for this project, it has been very useful now that Amazon has deprecated the AWS SDK for JavaScript v2 used by pino-cloudwatch, which seems to be abandoned.

serdnam commented 1 year ago

Unfortunately, functions can't be passed in the options object, see https://getpino.io/#/docs/api?id=pinotransportoptions-gt-threadstream.

And no problem! It is for those exact reasons why I built this in the first place :)

noe commented 1 year ago

I see. Then maybe a timestamp_format property with a formatting string?

tushar32 commented 3 months ago

@serdnam IS there any solution for time formating?

tushar32 commented 3 months ago

@noe

export const pinoLogger = pino({ mixin(_context, level) { return { 'level-label': pinoLogger.levels.labels[level], dateTime :new Date().toISOString() }; } }, transport);

Try using mixing. It works!