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

introduce debug.unique() #544

Closed
wants to merge 1 commit into from
Closed

Conversation

indutny
Copy link

@indutny indutny commented Jan 23, 2018

When debugging the applications that work with pools of streams
or sockets, the debug stream often comes out interleaved making it
impossible to distinguish between streams. It is a common knowledge
that unique ids could be assigned to each stream and be used in
logging to solve this. However, this solution is rather ad-hoc and
usually applied only during manual debugging.

Introduce debug.unique([ format ]) method that returns function with
the same signature as debug. This function however prepends either
'%d ' or format + ' ' (depending on the presence of format) to the
first argument of original debug function, and supplies unique
integer as a second argument, shifting the rest to the right.

Example:

const debug = require('debug')('ns');

class Instance {
  constructor() {
    this.debug = debug.unique('id=%d ');
  }

  method() {
    this.debug('method data=%j', {});
    // "id=123 method data={}"
  }

  attach(other) {
    this.debug('attach to=%d', other.debug.id());
    // "id=123 attach to=456"
  }
}

Alternative to #543

When debugging the applications that work with pools of streams
or sockets, the debug stream often comes out interleaved making it
impossible to distinguish between streams. It is a common knowledge
that unique ids could be assigned to each stream and be used in
logging to solve this. However, this solution is rather ad-hoc and
usually applied only during manual debugging.

Introduce `debug.unique([ format ])` method that returns function with
the same signature as `debug`. This function however prepends either
`'%d '` or `format + ' '` (depending on the presence of `format`) to the
first argument of original `debug` function, and supplies unique
integer as a second argument, shifting the rest to the right.

Example:

```
const debug = require('debug')('ns');

class Instance {
  constructor() {
    this.debug = debug.unique('id=%d ');
  }

  method() {
    this.debug('method data=%j', {});
    // "id=123 method data={}"
  }

  attach(other) {
    this.debug('attach to=%d', other.debug.id());
    // "id=123 attach to=456"
  }
}
```
@indutny
Copy link
Author

indutny commented Jan 23, 2018

I probably prefer this one over #543. Perhaps, it might be worth adding colors to it eventually.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.4%) to 70.43% when pulling 469e961 on indutny:feature/alt-id into 22f9932 on visionmedia:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 23, 2018

Coverage Status

Coverage decreased (-3.4%) to 70.43% when pulling 469e961 on indutny:feature/alt-id into 22f9932 on visionmedia:master.

@indutny
Copy link
Author

indutny commented Jan 23, 2018

The coverage is down, because there are no tests 🤖

@Qix-
Copy link
Member

Qix- commented Jun 20, 2018

I definitely see the benefit of this one over #543, though I think the term prefix is more apt. Thoughts?

@Qix- Qix- added discussion This issue is requesting comments and discussion feature This proposes or provides a feature or enhancement change-minor This proposes or provides a change that requires a minor release labels Jun 20, 2018
@indutny
Copy link
Author

indutny commented Jun 23, 2018

I'm fine with naming it differently.

@Qix-
Copy link
Member

Qix- commented Dec 13, 2018

I see the point now, @indutny - not sure why I wasn't getting it the last time I looked at this.

Question is, why not just use another namespace? .extend() was added in #524 for similar reasons - using wildcards in namespaces should do something similar, if not the same.

Also, sorry for taking so long to get back to this.

@Qix-
Copy link
Member

Qix- commented Dec 19, 2018

Going to go ahead and close these two (this and #543) in lieu of #544 (comment) in order to clean up a bit - feel absolutely free to comment back here and we can pick up the discussion :)

@Qix- Qix- closed this Dec 19, 2018
@indutny
Copy link
Author

indutny commented Dec 19, 2018

@qix extend() is pretty good! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change-minor This proposes or provides a change that requires a minor release discussion This issue is requesting comments and discussion feature This proposes or provides a feature or enhancement
Development

Successfully merging this pull request may close these issues.

None yet

3 participants