Add WIP template which vendors the watchdog instead of using a separate process

[alexellis]

Description

Add WIP template which vendors the watchdog instead of using a separate process

The watchdog is imported as an SDK to avoid an extra hop and to reduce total memory usage of one Go process vs two.

How Has This Been Tested?

Tested in-cluster on OpenFaaS for Enterprises, the RAM usage was reporting about 7-10Mi after a number of invocations, vs. up to 15Mi with the version that contains two separate processes.

How are existing users impacted? What migration steps/scripts do we need?

Any prefixing to log.Printf must be done with an overridden logger in the handler since there is only one process, log redirection isn't possible for stderr/stdout transparently.

Checklist:

I have:

• updated the documentation and/or roadmap (if required)
• read the CONTRIBUTION guide
• signed-off my commits with git commit -s
• added unit tests

[reviewfn]

Review

Code Review: PR #90 - Add WIP template which vendors the watchdog instead of using a separate process

Summary

This PR introduces a new Golang template (golang-middleware-inproc) that embeds the OpenFaaS watchdog as a library instead of running it as a separate process. This reduces memory usage by eliminating the need for inter-process communication. The changes also update existing templates with newer versions of dependencies and improve consistency in template.yml files.

Overall Assessment

Approval Recommended with Minor Revisions

The core idea of in-process watchdog integration is sound and addresses a valid performance concern (reduced RAM usage from ~15Mi to ~7-10Mi). The implementation is technically correct and follows existing patterns in the codebase. However, there are concerns around testing coverage, logging migration guidance, and potential edge cases in configuration handling. The lack of unit tests for the new template is a notable gap given the complexity of the watchdog integration.

Severity: Medium - Proceed with fixes for testing and documentation.

---

Detailed Review Comments

Correctness

• Positive: The watchdog configuration and handler setup in main.go correctly uses the config.ModeInproc mode and properly integrates with the watchdog library.
• Issue: The healthcheck_interval logic in main.go has a potential bug. When the env var is not set, it defaults to write_timeout, but write_timeout may not be initialized yet at that point. The code should ensure proper initialization order.
• Suggestion: Reorder the environment variable handling to set defaults in a logical sequence, or use a separate variable for the healthcheck default.

Impact and Migration

• Critical: The PR correctly identifies that logging behavior changes significantly. In separate-process mode, stdout/stderr redirection is transparent; in inproc mode, all logs share the same output stream.
• Issue: The migration guidance in the PR description is minimal. Users need clear documentation on how to adapt their logging (e.g., using structured logging with call IDs, avoiding direct log.Printf without prefixes).
• Suggestion: Expand the migration section in the PR description and consider adding a code example showing how to handle logging in the inproc template.

Testing

• Issue: The checklist indicates no unit tests were added, despite the new template introducing complex watchdog integration logic.
• Risk: Without tests, regressions in watchdog behavior (timeouts, health checks, signal handling) could go undetected.
• Suggestion: Add unit tests for the main.go logic, particularly around configuration parsing and signal handling. The existing go test in the Dockerfile is a good start but insufficient for this level of integration.

Efficiency and Performance

• Positive: The memory reduction (7-10Mi vs 15Mi) is a significant improvement validated through in-cluster testing.
• Issue: The new handler includes a 1ms sleep for testing purposes. This should be removed or made configurable for production use.
• Suggestion: Either remove the sleep entirely or add an environment variable to control it for debugging.

Security

• Positive: The code maintains the existing security model with non-root user execution and proper signal handling.
• Issue: No changes to security posture identified, but the inproc mode could potentially expose more attack surface if the watchdog library has vulnerabilities.
• Suggestion: Ensure the watchdog dependency is pinned to a stable, audited version.

Consistency

• Positive: The template.yml updates standardize messaging across templates and correctly reflect the Go version changes.
• Issue: The new template uses slog for logging while existing templates use basic log. This inconsistency could confuse users migrating between templates.
• Suggestion: Consider updating other templates to use slog for consistency, or document the logging differences clearly.

Potential Regressions

• Risk: The version updates (Go 1.25, watchdog 0.10.12, Alpine 3.22.2) could introduce compatibility issues with existing functions.
• Suggestion: Test the updated templates against a broader set of existing functions to ensure no regressions.

Code Quality

• Positive: The code follows Go conventions and uses proper error handling.
• Issue: The io.ReadAll error is ignored in the handler. While this may be acceptable for a template, it could mask issues.
• Suggestion: At minimum, log the error for debugging purposes.

Dependencies

• Positive: The go.mod includes all necessary dependencies for the watchdog integration.
• Issue: The replace directive for github.com/openfaas/of-watchdog is commented out, suggesting local development. Ensure this is removed for production.
• Suggestion: Verify the replace directive is not present in the final version.

/home/ubuntu/workdir/REVIEW.md

AI Agent output.

The review has been completed and written to ~/workdir/REVIEW.md. Exiting with status code 0.

[Copilot]

Pull Request Overview

This PR adds a new WIP template golang-middleware-inproc that vendors the of-watchdog as an SDK instead of running it as a separate process, reducing memory usage from ~15Mi to ~7-10Mi. Additionally, it updates version dependencies across existing templates.

• Introduces the new golang-middleware-inproc template with integrated watchdog
• Updates Go runtime from 1.24.0 to 1.25 and Alpine Linux from 3.22.0 to 3.22.2 in existing templates
• Bumps of-watchdog from 0.10.11 to 0.10.12

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
template/golang-middleware/template.yml	Updated welcome message to reflect Go 1.25 and added go.mod version note
template/golang-middleware/Dockerfile	Upgraded of-watchdog to 0.10.12, Go to 1.25, and Alpine to 3.22.2
template/golang-middleware-inproc/template.yml	New template configuration for in-process watchdog variant
template/golang-middleware-inproc/main.go	Main entry point that configures and starts the vendored watchdog
template/golang-middleware-inproc/go.work	Go workspace configuration for multi-module setup
template/golang-middleware-inproc/go.sum	Dependency checksums including watchdog SDK
template/golang-middleware-inproc/go.mod	Module dependencies with watchdog as a direct dependency
template/golang-middleware-inproc/function/handler.go	Example handler implementation with structured logging
template/golang-middleware-inproc/function/go.mod	Function module definition
template/golang-middleware-inproc/Dockerfile	Multi-stage build with Go 1.25 and Alpine 3.22.1
template/golang-middleware-inproc/.gitignore	Excludes compiled handler binary
template/golang-http/Dockerfile	Updated to use of-watchdog 0.10.12, Go 1.25, and Alpine 3.22.2

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Grammatical error: '1 milliseconds' should be '1 millisecond' (singular).

Suggested change

	log.Info("Sleeping for 1 milliseconds")
	log.Info("Sleeping for 1 millisecond")