RisingStack / trace-nodejs

Trace is a visualised distributed tracing platform designed for microservices.
https://trace.risingstack.com
Other
471 stars 90 forks source link

Feature request: include meta data with trace.reportError #135

Open devilleweppenaar opened 7 years ago

devilleweppenaar commented 7 years ago

We use a custom winston transport to log errors to Trace (see below).

reportError only supports passing instances of Error, so we pass on the error if we have one, or wrap our custom message in an error if we only have a message.

Winston supports the concept of meta data, which it then includes in the log entry as a string. We use this to include extra context with the log entry.

It would be awesome if Trace could do the same.

Custom winston transport:

const winston = require('winston');

class TraceLogger extends winston.Transport {
    constructor(options) {
        super(options);

        this.name = 'traceLogger';
        this.level = 'error'; // only log errors to trace

        this.trace = (options && options.trace) || require('@risingstack/trace');
    }

    log(level, msg, meta, callback) {
        let traceError;

        if (!meta && msg) {
            traceError = new Error(msg);
        } else if (meta instanceof Error) {
            traceError = meta;
        }

        if (traceError) {
            this.trace.reportError('winston_error', traceError);
        }

        callback(null, true);
    }
}

winston.transports.TraceLogger = TraceLogger;
module.exports = TraceLogger;

As you can see from the code snippet, we only use winston meta when it is of type Error.

dszakallas commented 7 years ago

Seems like a valid use case. In order for this feature to be truly useful we need changes on the backend and UI as well, e.g. make that parameter searchable. We first have to discuss internally how to generalize the reporting feature in Trace. For now, if you really want to report it, as a workaround you can assign the meta object as a property to the Error, or subclass it.

devilleweppenaar commented 7 years ago

Hi @szdavid92, thanks for the prompt reply.

It would be great if there's a defined way to include meta data via the reportError API, and even better if you could filter on the UI based on the meta data!

I was actually not aware that you can attach additional data to the error. Based on the reportError description from @risingstack/trace, I (perhaps incorrectly) assumed that it would only process instances of Error and ignore any additional information. As far as I remember this is actually what we tested before settling on the winston transport described earlier.

Since we use this in production, my preference would be to follow best practice as prescribed and supported by RisingStack. :)