winstonjs / winston-redis

A fixed-length Redis transport for winston
http://github.com/winstonjs/winston-redis
MIT License
41 stars 41 forks source link

Multiple data types / channels support #30

Open mgibeau opened 6 years ago

mgibeau commented 6 years ago

It looks like the transport doesn't quite fit my requirements of pushing three (at this stage) types of data:

  1. text message
  2. integer message as a %
  3. integer message as a bug count

All of which will be sent on three different channels minimum at different times, in the form of:

{ id: update.timestamp, event: update.event, data: update.data }

which is consumed by subscribbers converting the messages into server sent events (SSE) (AKA EventSource) to be sent to a CLI somewhere on the internet.

Originally posted by @binarymist in https://github.com/winstonjs/winston-redis/pull/29#issuecomment-422364175

mgibeau commented 6 years ago

@binarymist

Just to be clear, is this something that use to work before the upgrade to winston3?

If I understand this correctly, you should now be able to pass arbitrary metadata to the .log method, e.g.: winston.log({ level: "info", message, id, event, data }), it will then be inserted in redis as {"level":"info","message":"Some message","meta":{"id":"1","event":"my_event","data":"my_data"}}.

Otherwise, looking at the channel implementation it looks like it only support a single channel per logger, so you'd have to create separate loggers, e.g.:

const channelA = winston.createLogger({
  format: format.timestamp(),
  transports: [new Redis({ redis: client, channel: 'a' })]
})

const channelB = winston.createLogger({
  format: format.timestamp(),
  transports: [new Redis({ redis: client, channel: 'b' })]
})

channelA.log({ level: 'info', message: 'Channel A message', id: update.timestamp, event: update.event, data: update.data })
channelB.log({ level: 'info', message: 'Channel B message', id: update.timestamp, event: update.event, data: update.data })
binarymist commented 6 years ago

Just to be clear, is this something that use to work before the upgrade to winston3?

Quickly looking at the API and code, I doubt it.

If I understand this correctly, you should now be able to pass arbitrary metadata to the .log method

Right, but that arbitary data would be sent to all transports hooked up to winston, so for example, I need the:

  1. text message (for my project this is the 'testerProgress' event) to go to winston logging transports such as file, console, etc
  2. but the integer message as % (for my project this is the 'testerPctComplete' event) needs to only go through redis -> to a listening subscribber which creates a SSE from the data to send else where
  3. and integer message as bug count (for my project this is the 'testerBugCount' event) needs to only go through redis -> to a listening subscribber which creates a SSE from the data to send else where

All three different event types 'testerProgress', 'testerPctComplete', and 'testerBugCount' need to go through three+ channels at this stage depending on which microservice is sending them.

I've created a redis client wrapper which also aggregates (has a) winston logger wrapper which I'm currently usilng to log any 'testerProgress' event data (text messages). I know this seems to be the wrong way around, but it's the simplest, and considering 2 of the 3 events should only go through Redis and not any other form of logging, it seems to make sense. We'll see if I outgrow it when additional complexity turns up :-)

Thanks for picking up the winston-redis transport anyway @mgibeau :-)

binarymist commented 6 years ago

If you have ideas on how we could, and whether in face we should, extend this transport to work for this type of scenario I'm all ears, but it may be convoluting the transport to much? I don't know, what do you think?

mgibeau commented 6 years ago

Thanks, I think I understand better now... Would you be okay with having multiple Winston loggers instances? You could go with an approach like:

import winston, { createLogger, format, transports } from "winston";
import redisTransport from "winston-redis";

class MyLogger {
  constructor() {
    this.messageLogger = createLogger({
      format: format.timestamp(),
      transports: [
        new transports.Console(options),
        new transports.File(options)
      ]
    })

    this.progressLogger = createLogger({
      format: format.timestamp(),
      transports: [new redisTransport({
        host: 'localhost',
        port: 6379,
        channel: 'progress'
      })]
    })

    this.bugLogger = createLogger({
      format: format.timestamp(),
      transports: [new redisTransport({
        host: 'localhost',
        port: 6379,
        channel: 'bug'
      })]
    })
  }

  message(text) {
    return this.messageLogger.info(text)
  }

  progress(percent) {
    return this.progressLogger.info(percent)
  }

  bug(count) {
    return this.bugLogger.info(count)
  }
}

Then you can use it as:

MyLogger.message('Starting process...')
MyLogger.progress(55)
MyLogger.bug(3)

On the other hand, I haven't used the channel option yet, but I think having the liberty to define a channel per message would be useful. It would allow using a single Redis connection (and logger instance) to publish across multiple channels.

Hopefully, that makes sense? Let me know if you think it's worthwhile to implement or if I'm still completely off track :)

binarymist commented 5 years ago

Multiple loggers could be OK (currently I have one logger and one redis client), each logger needs to support multiple channels also.

Loggers for example, with the channels they need to support

There could be many more channels, this depends on how many test sessions a user wants, each test session can have many routes that a user wants to test. For the purpose of this discussion, just know that there can be many channels

I need the ability to dictate event type, channel, transport per message.

event type: testerProgress goes to the specified redis channel and also standard winston transports event type: testerPctComplete goest to the specified redis channel (channels are dependant on the user at runtime) event type: testerBugCount: same requirements as testerPctComplete

Currently I'm pushing events to my messagePublisher.

messagePublisher methods:

There's a good number of examples of how pubLog is used in app_scan_steps

There are a couple of examples of how publish is used in app_scan_steps here

this.log.notice are the calls to winston which is currently just logging to non redis transports... also seen in app_scan_steps.

Am I making sense? Thoughts @mgibeau?

mgibeau commented 5 years ago

If you're okay with multiple loggers, I would be willing to implement support for specifying a channel per message. Changing the transport seems like something that would need to happen upstream in winston itself.

binarymist commented 5 years ago

Yeah, that's what I was thinking, so unlikely to happen. Thanks for your feedback @mgibeau :-)