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

RFC: 3.0 API & Ideas #370

Closed
thebigredgeek opened this issue Dec 14, 2016 · 95 comments
Closed

RFC: 3.0 API & Ideas #370

thebigredgeek opened this issue Dec 14, 2016 · 95 comments
Labels
discussion This issue is requesting comments and discussion
Milestone

Comments

@thebigredgeek
Copy link
Contributor

thebigredgeek commented Dec 14, 2016

Hey everyone,

I figured it's time to start discussing the API for V3.

A few fundamental ideas to get started:

  • (No) Code written in ES2015 (minimal transpilation?, no es6 modules)
  • (Yes) Robust test coverage
  • (Yes) XO Code Styling
  • (Yes) "Replaceable middleware" pattern for formatting, output, etc. Extensible. This allows hooks for stuff like logstash and client-to-server log streaming as desired. Middleware is FIFO and debug comes with a default set for formatting, output, environment adaptation, etc. Ex:
 //Stdout would be default console.log middleware
import debug, { StdoutMiddleware } from 'debug';
// Add some sort of new middleware element
log.use((data, next) => ... )
// replace Stdout (console.log) with console.info at same position in middleware array
log.replace(StdoutMiddleware, (data, next) => {
  console.info.apply(console, data);
  next(); 
});
  • (Yes) "Child" debug instances (which inherit middleware from parent debug instances). Ex:
const log1 = debug('foo');
log1.use((data, next) => ... );
const log2 = log.child('bar');
log2('hello world'); // => foo:bar hello world

Any other ideas are welcome. I'd love to have a completed first pass draft by the end of next week.

@thebigredgeek thebigredgeek added the discussion This issue is requesting comments and discussion label Dec 14, 2016
@thebigredgeek thebigredgeek added this to the 3.0 milestone Dec 14, 2016
@thebigredgeek
Copy link
Contributor Author

thebigredgeek commented Dec 14, 2016

#343 - Middleware use case

@kilianc
Copy link

kilianc commented Dec 14, 2016

@thebigredgeek I am not sure if this came up somewhere else of there is already a solution in place but it would be nice to be able to switch between STDERR and STDOUT at the API level.

Example:

import createDebug from 'debug'

const debug = createDebug('app:upload')

Promise.reject(new Error('Something terrible happened O_O'))
  .catch((err) => debug.error(err.message))

Promise.resolve({ status: 'ok' })
  .then((data) => debug(data.status))

@thebigredgeek
Copy link
Contributor Author

@kilianc Seems like that case could be solved via a combination of child instances and middleware:

import debug, { StdoutMiddleware } from 'debug';

const appUploadLog = debug('app:upload');

const appUploadErrLog = appUploadLog
  .child('error')
  .replace(StdoutMiddleware, (data, next) => {
    console.error.apply(console, data);
    next()
  });

This was referenced Dec 14, 2016
@ibc
Copy link
Contributor

ibc commented Apr 6, 2017

Hi, sorry if I missed something but, does (No) Ability to enable/disable debug instances means that such a feature is not even under consideration?

@thebigredgeek
Copy link
Contributor Author

@ibc We are still considering (maybe) API to overwrite/set environment variables by key, which should do the exact same thing as (No) Ability to enable/disable debug instances.

@simonhac
Copy link

simonhac commented Apr 7, 2017

@thebigredgeek, following @ibc's comment, i am a contributor to a plugin for an application. the applications manages all config and hands the plugin a config object at initialisation time.

it is normal practice that users of the application turn logging on/off through the config object.

so i needed to be able to individually enable/disable debug 'channels' at runtime.

i extended @tehsenaus's excellent hot-debug (fork of debug) with this PR -- i'll fork it off into a project one day soon.

basically, i'm hoping that v3 will allow (either directly or via its own plugin middleware layer):

var debug = require('debug')('myPlugin')

...

function myInit(config) {
  debug.enable(config.debugEnabled)
}

@stefcameron
Copy link

What @VBAssassin wrote is a very good idea. I've used this to great effect in a logging class I wrote for use in many projects I've worked on. It also helps mitigate the overhead in the case where logging is disabled, or that particular logger is disabled.

@jfseb
Copy link

jfseb commented May 12, 2017

Apologizes or enlighten me if this is already in:

I often face the issue that debug instrumentation bogs down performance.

var debuglog = require('debug')('myfile') 
debuglog(" inspect this > " + JSON.stringify(bitstuff, undefined,2)); // ooh, hefty Stringify

So i end up with
debuglog( debuglog.enabled ? " inspect this > " + JSON.stringify(bitstuff, undefined,2) : "") ;
or similar.

Suggestion is to change the signature to evaluate (lazily) if passed a function

debuglog( () => `Inspect this > " + JSON.stringify(bitstuff, undefined,2) ) ;

Currently it recognizes the arg as a function and outputs:
myfile [Function] +0ms
which is not that much use. So either be incompatible or add a wrapper evalfunc

var debuglog = require('debug')('myfile').evalfunc;
debuglog( () => `Inspect this > " + JSON.stringify(bitstuff, undefined,2) ) ;

@ibc
Copy link
Contributor

ibc commented May 12, 2017

var debuglog = require('debug')('myfile').evalfunc;

Couldn't it just internally check typeof arguments[0] === 'function'?

@TooTallNate
Copy link
Contributor

TooTallNate commented May 12, 2017

@jfseb Use the %j or %O formatters:

debuglog(" inspect this > %j", bitstuff);

That way you're just passing in an argument to a noop function when debug is not enabled. Then the processing happens under the hood when debug is enabled.

@PixnBits
Copy link

@TooTallNate works for this simple JSON case, but not sure about @VBAssassin's example

@jfseb
Copy link

jfseb commented May 12, 2017

My recommendation was actually a duplicate, thanks @PixnBits and @VBAssassin.

@tehsenaus
Copy link

@VBAssassin Can you not already do this?

if ( debug.enabled ) {
    debug(mysql.format(sql,values));
}

IMO the above is more readable, and more efficient as you're not creating a closure.

@StreetStrider
Copy link

@tehsenaus your code is lack encapsulation, it requires to know about enabled and also write an if every time; while @VBAssassin follows the common idea behind debug that «debug works only in debug» (in that case a function is passed) and pretty encapsulated.

@talkingtab
Copy link

talkingtab commented May 30, 2017 via email

@tehsenaus
Copy link

@talkingtab how is it the same form as console.log?

@StreetStrider can you comment on readability? As far as encapsulation goes, I think .enabled wouldn't be a surprising or controversial addition to the public API of any logging library.

@StreetStrider
Copy link

@tehsenaus, to be honest, I think closure got better readability too. But it depends on person itself (partly). 🤔 It's like for-loops against map/forEach. Some people found for-loop more readable.

@talkingtab
Copy link

I just mean they take the same arguments:

@stefcameron
Copy link

@StreetStrider @tehsenaus Another way of writing this, given debug.enabled, would be:

debug.enabled && debug(mysql.format(sql, values);

That might be a little more friendly, visually, and it still ensures that whatever code is on the right side of && is only executed if debug is enabled. So you don't need a closure in this case. This form would also enable code optimizations with transpilers that support tree shaking. Telling the transpiler that debug.enabled === false would allow it to stip-out all the debug statements from the resulting code.

@131
Copy link

131 commented Jun 10, 2017

debug simplicity come from the initial, user defined DEBUG env contract "What i care about".
Inthe same design, I suggest to use a simple process.env.DEBUG_FORMAT (from tj/progress inspiration) that might default to :

process.env.DEBUG_FORMAT = ":timestamp :namespace :body :timediff";

with additionnal formaters like
::namespace_colored
:timestamp_iso

Registering additional formaters should be part of debug API, yet, leaving the user choosing, very simply, what he desired (the same way he choose the namespaces he cares about)

Is there anyone sharing this view ?

@Qix-
Copy link
Member

Qix- commented Aug 8, 2017

Just throwing this out there - since this will be a major release I assume, the LTS schedule for Node should be closely followed. Right now, Node 5 is in LTS, which means es2015 should be safely distributable without the need for transpilation.

@yamikuronue
Copy link
Contributor

@Qix- You mean Node 4?

Also: this is transpiled for the frontend, as well as being available on the backend. It's safest and least problematic to stick to a widely-supported subset of JS when making code available for three platforms (Node, Electron, and front-end).

@thebigredgeek
Copy link
Contributor Author

@Qix- haha yeah I wish it was that simple. This lib is used in electron, browser, etc as well as Node. So probably not going to work out so well without transpilation :(. That said, because of the number of platforms we are supporting I do think there could be value in using a compiler step to normalize interfaces between environments. @TooTallNate

@yamikuronue
Copy link
Contributor

@thebigredgeek I mean, I did remove the transpilation step by just rewriting the thing in ES5 code ;P

@Qix-
Copy link
Member

Qix- commented Aug 8, 2017

@yamikuronue Node 4 is in maintenance LTS. Node 5 is currently the active LTS version.

@yamikuronue
Copy link
Contributor

@qix meaning Node 4 still has support. My company is skipping 5 because 4 is still in support and we might as well go to 6 as we upgrade in anticipation of losing that support. So I'm hesitant to recommend the idea that 5 is safe to assume for a widely used utility.

@Qix-
Copy link
Member

Qix- commented Aug 8, 2017

Oops you're right @yamikuronue - 5.x is skipped, 6.x is now active LTS. 4.x is still maintenance, which means versions of software that still work for 4.x should backport any major bugfixes to it.

@paulrobello
Copy link

I think it would nice to be able specify my own color for a namespace or even a single call to log some output

@ghost ghost unassigned nkolban Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is requesting comments and discussion
Development

No branches or pull requests