hapijs / good

hapi process monitoring
Other
525 stars 160 forks source link

Improve documentation to explain Reporters type #518

Closed yonjah closed 7 years ago

yonjah commented 8 years ago

It seems like good actually have two type of reporters

  1. Transform reporters whose purpose is to modify and filter the data (like good-seqeeze)
  2. Log reporters whose purpose is to do the actual logging of data either to stdout a file or external service (like good-http)

This difference is hard to understand from the Docs and some example reporters might be misleading like good-console which actually is a Transform reporter and does not actually log the data anywhere
( so you can use it to pipe formatted strings into file -

[{
    module: 'good-console'
}, {
    module: 'good-file',
    args: ['./test.log']
}]

Writing a Log reporter as a Transform reporter can have sever results since there will be no actual consumer for the stream and it will stop processing and logging once the pipe is saturated (On my testing it was after 31 logs). so developers might think log is working properly but logger will actually fail for any reasonable number of logs (good-winston is currently affected by this https://github.com/lancespeelmon/good-winston/issues/6).

arb commented 7 years ago

I'll take almost any documentation PRs. There is nothing special going on in good though, this is just boilerplate pipe and stream logic.

yonjah commented 7 years ago

I'll try to see if I can get some time to work on it and send a PR.

I'm not entirely sure it's only a documentation issue. I mean good might be able to detect that a specific chain might have a structure that doesn't make since (like chaining another reporter after a Log reporter wouldn't make since or not having a Log reporter at all at the end of the chain.

So both of this example should throw an Error

options = {
    reporters: {
        noLogReporter: [{
            module: 'good-squeeze',
            name: 'Squeeze'
        }, {
            module: 'good-console'
        }],
        ReporterAfterLog: [{
            module: 'good-squeeze',
            name: 'SafeJson'
        }, {
            module: 'good-file',
            args: ['./test/fixtures/awesome_log']
        }, '']
    }
};

It might be possible to deduce the reporter type from the interface it implements. But it would probably be better if reporters would have some kind of descriptive property that would define if the reporter is a terminal, transform or maybe even can do both (since it might be possible to hack a stream that can do both but it wouldn't be very straight forward)

arb commented 7 years ago

Yeah I thought about trying to do that, didn't seem worth the hassle to try to detect when the user is doing something wrong. good is one of those things you set up once and sort of forget about and never bother with again. ReporterAfterLog would throw an error I think because "" is available on the process object.

Additionally, it either works or it doesn't and when it doesn't work, someone posts an issue here or on Gitter to get help. Again, there isn't anything special going on here; it's JUST pipe and stream logic heavily used in the node ecosystem.

yonjah commented 7 years ago

I created a pull request. I'm not sure if made the documentation clear enough.

The thing is I'm not sure this is always the user blame for doing something wrong. The user might not be aware of the internal implementation of the reporter and might think a Transform reporter is actually a Log reporter. The Reporter maintainer might be at fault getting the stream type wrong. I saw it in two reporters now (good-winston and good-bunyan) I can only guess But I assume many reporter originally copied the implementation from good-console but when good started using streams good-console was changed to only be a string formatter which doesn't fit many of this reporters.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.