openfaas/of-watchdog

Support for structured logging

saginadir opened this issue · 7 comments

Expected Behaviour

In order to integrate with my org's log aggregator I need to provide pure JSON logs from all deployed apps. That means that the logs will need to be parsed as a JSON as such:

{"level":"info","ts":"2019-09-05T15:33:26.302Z","caller":"function/file.go:64","msg":"Parsing the payload from the handler", taskId: 14}
...
{"ts":"2019-09-05T15:35:35.212Z","method":"POST","path":"/","status":"200 OK","ContentLength": 121}

Current Behaviour

  1. The current behavior is that there is a prefix for the logs coming from the function
  2. The log of the function response status is unstructured.
2019/09/05 15:33:26 stdout: {"level":"info","ts":"2019-09-05T15:33:26.302Z","caller":"function/file.go:64","msg":"Parsing the payload from the handler", taskId: 14}

2019/09/05 15:33:26 POST / - 200 OK - ContentLength: 121

Possible Solution

I'm raising two issues here and I don't think they should have the same solution. So let's tackle one issue at a time:

Removing all the prefix from the log (including the timestamp)

This could be achieved either by simply using fmt.Println instead of log, or by setting the flags on the current logger to: log.SetFlags(0).

This could be optional by passing setting an environment variable like disable_logger_prefix

Formatted printing for the function's response status

We need to have an option to print the response status in a formatted log, adding a structured JSON log can be simply enough by either mocking the JSON format with Printf so it can be performant. Or importing once of the available loggers which will be able to print the JSON format.

Again this should be optional and set by an environment variable like logger_format.

Context

We are aggregating all the logs across all of our products into a log aggregator to be indexed and made queryable. At the moment, watchdog uses the default logger of golang which doesn't support a JSON structure and adds a timestamp prefix. That means we aren't able to use the watchdog as it is now, and we will need to fork the project and make the necessary adjustments

Do we have enough for this now with the PRs merged?

As I mentioned in my other comment, I forked watchdog to allow the expected behavior for our functions. I am not aware of the Merged PRs, any of them deal with with the issues I raised here? If so I'll use the official watchdog, if not I can assist with developing a solution for this issue and create a new PR.

What tooling are you using to collect logs and index them?

We are using Filebeat, logstash, and elastic search

Voziv commented

@saginadir Do you have that fork available in a public repo? I'm very interested in checking it out!

@saginadir Do you have that fork available in a public repo? I'm very interested in checking it out!

@Voziv sorry man, I don't have access to it anymore. I left the code at my previous job. That being said, it's a minor change.

  1. I added to the correct flags to the global log object to NOT print anything but the log message.
  2. I searched in the places where watchdog actually uses the log function, and there I edited the log to be in a JSON format.

Overall I think it was not more than 10 lines of code.

I'm going to close this and keep conversation focused on #75 so we can find out whether there is an issue with:

  • Lack of structured logging in JSON format from the watchdog

Or

  • A request to trim prefixes