Repository navigation
Add configurable persistent logging and per-replica storage - #16
sanchosancho wants to merge 1 commit into
Conversation
| } | ||
|
|
||
| func ServeMetrics(gatherer prometheus.Gatherer) error { | ||
| func ServeMetrics(gatherer prometheus.Gatherer, logger ...*SimpleLogger) error { |
There was a problem hiding this comment.
why? :)
just logger *SimpleLogger
and
if logger != nil {
startupLogger.Infof("metrics HTTP starts listening on :%d", metricsPort)
}
| tls bool, | ||
| authEnabled bool, | ||
| timeoutConfig TaskProxyTimeoutConfig, | ||
| accessLogConfig AccessLogConfig, |
There was a problem hiding this comment.
[P1] Update the caller added by PR #15 when rebasing onto main. After resolving the import conflict in updater_test.go by keeping both imports, TestDiscoveryRetainedAndFreshTasksReachAllUpdaterOutputs still calls the old seven-argument CreateTaskUpdater and go test ./... fails with 'not enough arguments in call to CreateTaskUpdater'. Pass the access log config and path there as well, preserving that regression test.
| ) | ||
|
|
||
| func ServeGRPC(s serverv3.Server, authServer *authServer) error { | ||
| func ServeGRPC(s serverv3.Server, authServer *authServer, logger ...*SimpleLogger) error { |
There was a problem hiding this comment.
Same as the existing ServeMetrics comment: use a single logger *SimpleLogger parameter and call logger.Infof when logger != nil. Every call in this repository already passes one logger; the variadic parameter silently ignores additional loggers and adds unnecessary fallback/first-element handling.
| } | ||
|
|
||
| func DefaultLoggingConfig() LoggingConfig { | ||
| return LoggingConfig{Directory: DefaultLogDirectory, Server: LogConfig{WriterType: "stderr", MinLogLevel: "debug"}, Proxy: LogConfig{WriterType: "stderr", MinLogLevel: "info"}, AccessLog: AccessLogConfig{Enabled: true, WriterType: "stderr"}} |
There was a problem hiding this comment.
Please match the existing project formatting: expand this nested literal into named fields on separate lines, as in DefaultTaskProxyTimeoutConfig and other constructors. Also keep blank lines between declarations/functions and multiline method bodies in the new logging.go and log_rotation.go files. gofmt alone does not expand these compressed constructs.
| name, values, server, proxy, access string | ||
| }{ | ||
| {"default", "", "stderr", "stderr", "stderr"}, | ||
| {"stdout", "server:\n logging:\n writerType: stdout\nproxy:\n logging:\n writerType: stdout\n accessLog:\n writerType: stdout\n", "stdout", "stdout", "stdout"}, |
There was a problem hiding this comment.
Please format the test table like the existing auth/discovery tests: named fields on separate lines and raw multiline strings for YAML fixtures, rather than positional entries containing long strings with escaped newlines. Apply the same formatting to the other chart cases so configuration changes are easy to review.
2 participants
Task proxy logging was tied to console output and had no managed file retention. This change adds independent server, Envoy system, and listener access log configuration with stdout, stderr, or file writers, severity controls, and size/time rotation with archive retention limits.
File logs use stable paths and append across restarts. The server rotates its own files and asks Envoy to reopen renamed files through its local admin endpoint; pending archives remain protected until reopening is confirmed. Existing listener access-log placement and routing/auth behavior are preserved.
The Helm workload becomes a StatefulSet with one retained PVC per replica. Claims are allocated in every logging mode so console-to-file changes do not alter immutable claim templates. Both containers mount the same replica-local storage, and logging configuration changes trigger a rollout.
Upgrade considerations
Existing Deployment installations require a planned migration with downtime: stop the old Deployment before upgrading, then verify the StatefulSet and its claims. The README documents migration, storage ownership requirements, retained claims, and soft retention limits. Storage template changes require a separate migration.
Validation
make testpassed, including process lifecycle and Helm render tests.go vet, Linux amd64 build, and strict Helm lint passed.