New Pipe Reporter Interface

[arb]

I'm proposing that the new Good reporter interface allow users to pass an array of streams. Good would then create a stream pipeline with these streams. This would allow users to do things like use different filtering logic, clone the payloads before passing them on to other streams, and add additional implementation specific information.

I'm proposing the reporters option would look something like this:

{
  reporters: 
    'awesome_log-reporter': {
      streams: [{
        module: 'good-squeeze',
        constructor: 'Squeeze'
        args: [{ log: '*', error: ['debug', 'internal' ]}]
      }, {
        module: 'good-file',
        args: ['./test/fixtures/awesome_log']
      }]
    },
    'log-hg-received-time': {
      streams: [{
        module: 'good-squeeze',
        constructor: 'Squeeze'
        args: [{ ops: '*' }]
      }, {
        module: './lib/clone-data'
      }, {
        module: 'high-res-time',
        args: [{ timezone: 'EST', legacyMode: false, key: '__hrReceived' }, 'round-down']
      }, {
        module: 'good-file',
        args: ['./test/fixtures/awesome_log']
      }]
    }
}

The reporters key will be a key/value pair. The key will be user friendly name that this reporter will be referred to. The value will be another object that has a streams array. Each item in the streams array is an object. The module value is either a path to a stream or a npm module to a stream object. constructor is an optional string in cases where the stream constructor isn't the result of require('module') and is the function used to create a new stream. Finally, args is an array of values passed to the constructor specified by module. Each object inside the reporter object represents a stream object. When good starts, it will pipe each stream in the order of the items in the array, one into the next using pumpify.

This will give good users much more control over how things are filtered, adding additional information to log messages, cloning, and other destination options.

[gergoerdosi]

I was about to create something similar, so +1. Two questions:

1. Pumpify looks a bit outdated, it is tested only on node 0.10. We should either contribute to it, or make a similar module under the hapijs organization. What do you think?
2. How do you see the ecosystem? People will create higher level modules (eg. "awesome_log-reporter") or they will create lower level streams (eg. "good-file") that are then glued together in hapi?

[arb]

1. Why does it look outdated? Hasn't had significant updates in a while, but I don't think it really needs updating that often. I've met Mr. mafintosh at NodeConf and I don't know of anyone who knows more about streams than him. We could post an issue on there though asking about the long term status of the project though.
2. The ecosystem would be that you just create transform streams that good glues together with pumpify. They would all have to be in "object mode", but other than that, any existing transform stream could be used as long as it sticks to the basic interface.

[gergoerdosi]

1. There have been many changes to streams since v0.10. Most of them are non-breaking, but I would like to make sure the code that was written for v0.10 also works on v4 and v5 (and later). Updating .travis.yml with the new versions should be enough if the code works.
2. Sounds good! Just a note related to this. Streams will be independent and as you said they can modify objects by cloning. Would it be possible to pass both the original (frozen) and the modified objects to streams? The original could be used to get data from, the modified could be used as the final one that gets logged.

[arb]

1. Yeah I agree, we would want to verify that. I would like to avoid taking on the baggage of the stream library if possible. Every effort should be used to try to keep that stuff in user land IMO. Unless a stream master wants to tackle that and have it within the hapijs universe.
2. The signature to the stream is always like chunk or data; a single argument so I guess we would have to wrap everything up into a single massive argument to do that. That's the only way to keep the API in line with any existing streams.

[gergoerdosi]

True, I forgot that. I'm open to anything, I just would like to avoid the case when one stream breaks because the other modifies the object.

[devinivy]

I see no particular reason to recreate any of those "canonical" stream modules (generally, the mississippi modules), but I can think of no reason that they shouldn't be tested on recent versions of node. Looks like pumpify will work fine on node v4 and v5: https://travis-ci.org/devinivy/pumpify/builds/88802545 .

[arb]

I guess first steps would be to open a PR there and see if we can get some better testing on newer versions of Node there.

[arb]

Update I'm thinking that the interface should be an array of streams, and then another key that represents the end of the stream. This is so that the ends of the streams can implement an asynchronous stop method. This would be useful for tear downs.

[fluky]

I'm having trouble with this change. Using glue this is how I registered a custom reporter:

            plugin: {
               register: 'good',
               options: {
                  reporters: [{
                     reporter: reporter,
                     events: {
                        error: '*',
                        log: '*',
                        ops: '*',
                        request: '*',
                        response: '*' 
                     }   
                  }]  
               }   
            }   

I'm struggling to find something that works with the new interface. Everything I try gives me errors. Looking at the example in the first post I would assume I would use:

            plugin: {
               register: 'good',
               options: {
                  reporters: {
                     'my-stream': {
                        streams: [{
                           module: require ('../src/server/reporters/mongo'),
                           args: [{
                              error: '*',
                              log: '*',
                              ops: '*',
                              request: '*',
                              response: '*' 
                           }]  
                        }]  
                     }   
                  }   
               }   
            }   

But that errors with: Error: Invalid monitorOptions options child "reporters" fails because [child "my-stream" fails because ["my-stream" must be an array]]

If I make it an array (it's not in the example) I get: Error: Invalid monitorOptions options child "reporters" fails because [child "my-stream" fails because ["my-stream" at position 0 does not match any of the allowed types]]

[arb]

This issue was closed two months ago. If you are having problems, please open a new issue and don't bump old ones. That being said, you're probably using version 6 of good and not 7.

Either way, streams doesn't seem to be a valid option to either good 6 or good 7.

[fluky]

No I'm using 7 and 7 was released with this breaking change 11 hours ago, but thanks so much for the help.

[lock[bot]]

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.