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

Sending JSON is broken #34

Closed
wyqydsyq opened this issue Jun 5, 2018 · 6 comments
Closed

Sending JSON is broken #34

wyqydsyq opened this issue Jun 5, 2018 · 6 comments

Comments

@wyqydsyq
Copy link

@wyqydsyq wyqydsyq commented Jun 5, 2018

Setting the json option to true results in my log never actually reaching loggly, no error gets thrown and the Winston transport isn't emitting it's error event either, so it causes logging to silently fail.

This issue can be worked-around by not specifying json: true in the options, and manually JSON.stringifying the log body e.g:

Instead of:

function loggerFactory() {
  const logglyTransport = new (require('winston-loggly-bulk')).Loggly({
    subdomain: process.env.LOGGLY_SUBDOMAIN,
    token: process.env.LOGGLY_TOKEN,
    tags: tags || [],
    json: true,
  })

  const winston = new (require('winston')).Logger({
    transports: [logglyTransport],
  })

  return function(fields: Fields) {
    winston.log(fields.level, fields)
  }
}

Use:

function loggerFactory() {
  const logglyTransport = new (require('winston-loggly-bulk')).Loggly({
    subdomain: process.env.LOGGLY_SUBDOMAIN,
    token: process.env.LOGGLY_TOKEN,
    tags: tags || [],
  })

  const winston = new (require('winston')).Logger({
    transports: [logglyTransport],
  })

  return function(fields: Fields) {
    winston.log(fields.level, JSON.stringify(fields))
  }
}
@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Jun 6, 2018

Hi @wyqydsyq, Please give me some time to finish my ongoing issues. I'll try to reproduce once I get some time.

Thanks for reporting.

@elfilip
Copy link

@elfilip elfilip commented Jan 10, 2019

I can't reproduce this. Json option works for me fine.

@monbrey
Copy link

@monbrey monbrey commented Jan 29, 2019

I've encountered the same issue - it seems that JSON cannot be sent if no message string is provided.
Example

winston.info("Message")
// logs { json: { level: "info", message: "Message", timestamp: <time> } } to Loggly
winston.info("Message", { prop: "value" } )
// logs { json: { level: "info", message: "Message", prop: "value", timestamp: <time> } } to Loggly
winston.info({ prop: "value" })
// logs { json: { level: "info", timestamp: <time> } } to Loggly

If JSON is the only parameter, it seems to be ignored

@wyqydsyq
Copy link
Author

@wyqydsyq wyqydsyq commented Jan 30, 2019

Ah that makes sense as to why some couldn't reproduce, I think in my case I didn't specify a message property in the JSON object while I assume most people would.

Seems there's some kind of internal check/processing happening on body.message in the log handler for this transport (or somewhere along it's call stack) and the transport is silently dropping the log event if it is unable to find/extract a value from that property instead of just submitting the log event without it as would be expected.

@monbrey
Copy link

@monbrey monbrey commented Jan 30, 2019

Working on this today, I’ve found that Loggly also has a “feature” which drops fields from the parser if their type had changed (eg from string to object) which might also be causing the issue

@alestrunda
Copy link

@alestrunda alestrunda commented Apr 6, 2020

works in version 3

@alestrunda alestrunda closed this Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.