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

Using pino/browser for Cloudflare Workers #2035

Open
marceloverdijk opened this issue Aug 26, 2024 · 6 comments
Open

Using pino/browser for Cloudflare Workers #2035

marceloverdijk opened this issue Aug 26, 2024 · 6 comments

Comments

@marceloverdijk
Copy link

marceloverdijk commented Aug 26, 2024

Hi,

I'm trying to use pino (browser) with Cloudflare Workers.
This is my config do far:

import pino from 'pino';

const logger = pino({
      base: {
        requestId: c.var.requestId,
      },
      browser: {
        asObject: true,
        formatters: {
          level (label, _number) {
            return { level: label.toUpperCase() }
          }
        },
      },
      enabled: true,
      level: 'trace',
      timestamp: pino.stdTimeFunctions.isoTime,
    });

which outputs the log messages like below and is a good start:

logger.debug('Loading customer with id=%s', 'jane-doe'):
logger.trace('Fetching customer with id=%s from database', 'jane-doe'):

{
  time: '2024-08-26T20:15:38.087Z',
  level: 'DEBUG',
  msg: 'Loading customer with id=jane-doe'
}

(1)

Although the level is defined as trace, calls to logger.trace(..) are not being logged... is that expected behaviour with pino/browser?

Now when I add the custom write option to the config, trace is being outputted...

import pino from 'pino';

const logger = pino({
      base: {
        requestId: c.var.requestId,
      },
      browser: {
        asObject: true,
        formatters: {
          level (label, _number) {
            return { level: label.toUpperCase() }
          }
        },
        write: (o) => console.log(JSON.stringify(o)),
      },
      enabled: true,
      level: 'trace',
      timestamp: pino.stdTimeFunctions.isoTime,
    });

logger.debug('Loading customer with id=%s', 'jane-doe'):
logger.trace('Fetching customer with id=%s from database', 'jane-doe'):

{ level: 'DEBUG', msg: 'Loading customer with id=jane-doe' }
{ level: 'TRACE', msg: 'Fetching customer with id=jane-doe from database' }

so with a custom write option it seems to pick up trace logs...

(2)

When creating the logger I used the following base option to set a context requestId...

base: {
    requestId: c.var.requestId,
},

but this is never logged, nor available in the write: (o) object... is it possible to use a context with pino/browser?

Note I also looked into logMethod but that does not seem to be invoked by pino/browser at all.

Now when I create a child logger and pass the requestId it does work:

import pino from 'pino';

const logger = pino({
      base: undefined,
      browser: {
        asObject: true,
        formatters: {
          level(label, _number) {
            return { level: label.toUpperCase() };
          },
        },
        write: (o) => console.log(JSON.stringify(o)),
      },
      enabled: true,
      level: 'trace',
      timestamp: pino.stdTimeFunctions.isoTime,
    });
const childLogger = logger.child({ requestId: c.var.requestId });

childLogger.debug('Loading customer with id=%s', 'jane-doe')`;

gives:

{"time":"2024-08-26T20:54:40.750Z","level":"DEBUG","requestId":"ec1eeec6-6dc7-45a8-872e-19d360844d80","msg":"Loading customer with id=jane-doe"}

But I do not need a child logger, is there a way to pass this context directly to the (parent) logger?

@marceloverdijk
Copy link
Author

Note I'm now doing this: #1969 (comment)

But above questions are still relevant.

@mcollina
Copy link
Member

We are not testing against Cloudflare Workers, so this is essentially unsupported right now on that platform.

@joshgummersall
Copy link

joshgummersall commented Nov 18, 2024

Seconding that this is not specific to Cloudflare Workers, the pino({base: {foo: 'bar'}}) feature does not appear to work/be supported in browser mode. Nor does pino({formatters: {bindings: () => {bindings.foo = "bar"; return bindings;} }})

EDIT: Code Sandbox link to demonstrate, little clunky to find the JS console:
Screenshot 2024-11-18 at 1 26 34 PM

@jsumners
Copy link
Member

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@joshgummersall
Copy link

If I find the time soon, I sure will! Thanks 😄

@joshgummersall
Copy link

joshgummersall commented Nov 19, 2024

It looks like formatters.log basically supports "dynamic bindings at log time" which is the functionality I care about, so I'm happy to say this is all good! Thanks for all your work on this library 👍

Codesandbox: https://codesandbox.io/p/sandbox/elastic-haze-997gtz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants
@mcollina @marceloverdijk @jsumners @joshgummersall and others