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

export the bunyan serializers #65

Closed
trentm opened this issue Jun 27, 2016 · 1 comment
Closed

export the bunyan serializers #65

trentm opened this issue Jun 27, 2016 · 1 comment

Comments

@trentm
Copy link
Contributor

trentm commented Jun 27, 2016

Before restify-clients was split from restify, one would get access to Restify's Bunyan reserializers (especially its client_req and client_res serializers) via:

var restify = require('restify');
var log = bunyan.createLogger({
    ...
    serializers: restify.bunyan.serializers
});

This is required to create a Bunyan logger for restify clients so that you don't get big garbage in trace-level logs when client_req and/or client_res is logged (e.g. here:

log.trace({client_req: opts}, 'request sent');
).

There is some history and subtlety here:

  • If you pass a Bunyan logger with no serializers to the restify clients, then it will add the necessarily serializers:

    clients/lib/HttpClient.js

    Lines 401 to 405 in 8765244

    if (!this.log.serializers) {
    // Ensure logger has a reasonable serializer for `client_res`
    // and `client_req` logged in this module.
    this.log = this.log.child({serializers: serializers});
    }
    However that code is insufficient if your logger has other serializers on it.
  • That limitation was due to: fix empty serializers node-restify#501

However, even if that case is fixed (I'll try to do a PR for that), I think it would be good citizenship for restify-clients to export .bunyan.serializers for users that want to control their Bunyan logger serializers carefully. I'd be careful to NOT export the RequestCaptureStream class that was carried over from restify (the server) itself because it isn't used or needed in the client.

trentm added a commit to trentm/clients that referenced this issue Jun 27, 2016
trentm added a commit to trentm/clients that referenced this issue Jun 28, 2016
trentm added a commit that referenced this issue Jun 28, 2016
@trentm
Copy link
Contributor Author

trentm commented Oct 5, 2016

see #80 to correct a mistake made in this commit

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

1 participant