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

Multiple data types / channels support #30

Open
mgibeau opened this issue Sep 18, 2018 · 7 comments
Open

Multiple data types / channels support #30

mgibeau opened this issue Sep 18, 2018 · 7 comments

Comments

@mgibeau
Copy link
Member

mgibeau commented Sep 18, 2018

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 #29 (comment)

@mgibeau
Copy link
Member Author

mgibeau commented Sep 18, 2018

@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
Copy link

binarymist commented Sep 18, 2018

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
Copy link

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
Copy link
Member Author

mgibeau commented Sep 19, 2018

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
Copy link

binarymist commented Sep 24, 2018

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

  • progressLogger - transports [redis, standard transports] (Need to be able to control which messages go through redis and which go throuh standard winston transports)
    • app-adminUser
    • app-lowPrivUser
    • server-NA
    • tls-NA
    • etc
  • pctCompleteLogger - transports [redis]
    • app-adminUser
    • app-lowPrivUser
    • server-NA
    • tls-NA
    • etc
  • bugCountLogger - transports [redis]
    • app-adminUser
    • app-lowPrivUser
    • server-NA
    • tls-NA
    • etc
  • nonRedisLogger - transports [standard stransports]

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:

  • pubLog for events that need to go to both redis and winston
  • publish for events that just need to go to redis

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
Copy link
Member Author

mgibeau commented Sep 27, 2018

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
Copy link

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

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

2 participants