Allow for very long log lines

alexellis commented 2 years ago

Signed-off-by: Alex Ellis (OpenFaaS Ltd) alexellis2@gmail.com

Description

Motivation and Context

With this environment-variable (log_buffer_size), very large logs can be scanned and printed out to stdio.

This was reported by RateHub (an OpenFaaS Pro) user, but has also been worked on several times in the past.

cc @LucasRoesler @everesio @ghostbody

Fixes #105 #100 Closes #99 #107

How Has This Been Tested?

Tested with a 5MB JSON file and a Node.js process that printed the value to console.log/console.error.

It worked as expected. After reducing the value to the default, an error was returned.

'use strict'
const fs = require("fs")

module.exports = async (event, context) => {

  let v=  fs.readFileSync("./function/5mb.json", "utf8")

  console.error(v)

  return context
    .status(200)
    .succeed(v)
}

http_upstream_url="http://127.0.0.1:3000" mode=http exec_timeout=30s log_buffer_size=31457280 write_timeout=30s fprocess="node index.js" read_timeout=30s write_debug=true ../../of-watchdog

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[x] My change requires a change to the documentation.
[x] I have updated the documentation accordingly.
[x] I've read the CONTRIBUTION guide
[x] I have signed-off my commits with git commit -s
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

alexellis commented 2 years ago

In an older PR #107 the code changed to s, err := reader.ReadString('\n'), but it's not explained why that change was made.

This PR appears to resolve the issue and retain compatibility with the existing behaviour.

alexellis commented 2 years ago

Resolved your requests @LucasRoesler. If you can think of a simpler and easier to read test than that in #107 then perhaps we could add it at a later time.

alexellis commented 2 years ago

cc @chrisrichard @voziv from RateHub

openfaas / of-watchdog