Fix log formatting issue 132

jcollie commented 2 years ago

Signed-off-by: Jeffrey C. Ollie jeff@ocjtech.us

Fix #132

Description

Don't pass the scanned text from the function as the format string to fmt.Printf.

Motivation and Context

When not prefixing the output from the function, of-watchdog would pass the output as the format string to fmt.Printf. That would mean that anything that looked like a % verb would get misinterpreted.

[X] I have raised an issue to propose this change (required)

How Has This Been Tested?

Built Docker image with custom build of of-watchdog. Both prefixed output and non-prefixed output worked. % escapes no longer interpreted in non-prefixed output.

Types of changes

[X] Bug fix (non-breaking change which fixes an issue)
[ ] 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.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[X] I've read the CONTRIBUTION guide
[X] I have signed-off my commits with git commit -s
[ ] I have added tests to cover my changes.
[X] All new and existing tests passed.

LucasRoesler commented 2 years ago

@alexellis i have tried this out and :+1:

alexellis commented 2 years ago

Thanks @LucasRoesler, we should still have @jcollie fill out "How Has This Been Tested?" for the paper trail, for anyone landing here in the future.

A code sample for producing the error, and the before / after output.

Unless you have one and can share it below?

jcollie commented 2 years ago

Thanks @LucasRoesler, we should still have @jcollie fill out "How Has This Been Tested?" for the paper trail, for anyone landing here in the future.

Sorry it's taken a bit for me to get around to this but I the other day I did build a custom version of the watchdog with my patch and deployed it with a function.

alexellis commented 2 years ago

How Has This Been Tested?

This has still been left blank. We're looking for console output here and / or code examples for the function.

Given that @LucasRoesler has tested this independently, I'll merge it, however we would still like the paperwork to be filled out. Unless I'm misunderstanding the change, it should not take more than a few minutes?

You can run the watchdog on your host without building a container or patching a template.

Alex

alexellis commented 2 years ago

The change is now available in 0.9.4

Please feel free to send a PR to the template you were using @jcollie when you saw the problem, and update the Dockerfile for it. Unfortunately you didn't mention the template name in your issue, but you can find the repo via faas-cli template store commands.

Alex

openfaas / of-watchdog