feat: add slog structured logging
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
8f2f3cf271
commit
ea754a85c5
@@ -0,0 +1,759 @@
|
||||
# slog Structured Logging Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Add structured application logging using the Go standard library `log/slog`, with JSON output by default and a global slog-only logging rule.
|
||||
|
||||
**Architecture:** `pkg/logger` becomes the single application logger factory and returns `*slog.Logger`. `cmd/server/main.go` creates one logger from config and injects it into Gin router and services. Gin request logging is implemented as custom middleware; workspace/process lifecycle logs live in service layer.
|
||||
|
||||
**Tech Stack:** Go 1.25, standard library `log/slog`, Gin, existing YAML config via `gopkg.in/yaml.v3`.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Use `log/slog` for all application logs from this change forward.
|
||||
- Default log output is JSON.
|
||||
- Support log `level` and `format` config.
|
||||
- Do not introduce any third-party logging library.
|
||||
- Do not use `gin.Default()` or `gin.Logger()`.
|
||||
- Do not use `fmt.Print*` as application logging.
|
||||
- Do not use `log.Printf`, `log.Println`, `log.Fatal`, or `log.Fatalf` in runtime application paths; only `cmd/server/main.go` may use standard library `log.Fatalf` before slog initialization exists.
|
||||
- Do not log file contents, request bodies, response bodies, OpenCode stdout/stderr, environment variables, tokens, secrets, or workspace root in request/process/workspace lifecycle logs.
|
||||
- Do not add request ID, tracing, OpenTelemetry, Prometheus, ELK, Loki, auth, CORS, rate limiting, Git, LSP, watcher streaming, frontend, or AI Agent features.
|
||||
- Do not change existing API paths, methods, or response JSON shapes.
|
||||
- Code generation/refactoring must be delegated to Codex MCP using exactly `model: "kimi/kimi-k2.7-code"`, `sandbox: "danger-full-access"`, and `approval-policy: "on-failure"`.
|
||||
- Do not commit unless the user explicitly asks.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
### Modify
|
||||
|
||||
- `configs/config.yaml` — add `log.level` and `log.format`.
|
||||
- `pkg/config/config.go` — add `LogConfig`, defaults, YAML load, env overrides.
|
||||
- `pkg/config/config_test.go` — extend config tests for log defaults/YAML/env.
|
||||
- `pkg/logger/logger.go` — replace `log.Logger` factory with `slog.Logger` factory.
|
||||
- `cmd/server/main.go` — initialize slog logger and use structured logs.
|
||||
- `internal/api/router.go` — accept logger, install custom request logging middleware.
|
||||
- `internal/api/router_test.go` — update `NewRouter` calls to pass nil logger.
|
||||
- `internal/service/workspace_service.go` — inject logger and add workspace lifecycle logs.
|
||||
- `internal/service/process_service.go` — inject logger and add process lifecycle logs.
|
||||
- Existing tests under `internal/api` and packages that construct services — update constructors to include logger parameter or nil.
|
||||
- `README.md` — document log config and slog-only logging rule.
|
||||
|
||||
### Create
|
||||
|
||||
- `pkg/logger/logger_test.go` — tests logger factory success/failure.
|
||||
- `internal/api/logging_middleware.go` — Gin request logging middleware.
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Add log config
|
||||
|
||||
**Files:**
|
||||
- Modify: `configs/config.yaml`
|
||||
- Modify: `pkg/config/config.go`
|
||||
- Modify: `pkg/config/config_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `config.LogConfig` with fields `Level string` and `Format string`.
|
||||
- Produces: `Config.Log LogConfig`.
|
||||
- Produces env overrides `CODESPACE_LOG_LEVEL` and `CODESPACE_LOG_FORMAT`.
|
||||
|
||||
- [ ] **Step 1: Extend config tests first**
|
||||
|
||||
Update `pkg/config/config_test.go` to add these assertions to existing tests:
|
||||
|
||||
```go
|
||||
func TestDefaultIncludesLogConfig(t *testing.T) {
|
||||
cfg := Default()
|
||||
if cfg.Log.Level != "info" {
|
||||
t.Fatalf("Log.Level = %q, want info", cfg.Log.Level)
|
||||
}
|
||||
if cfg.Log.Format != "json" {
|
||||
t.Fatalf("Log.Format = %q, want json", cfg.Log.Format)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadParsesLogConfigFromYAML(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "config.yaml")
|
||||
data := []byte(`log:
|
||||
level: "debug"
|
||||
format: "text"
|
||||
`)
|
||||
if err := os.WriteFile(path, data, 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
cfg, err := Load(path)
|
||||
if err != nil {
|
||||
t.Fatalf("Load returned error: %v", err)
|
||||
}
|
||||
if cfg.Log.Level != "debug" {
|
||||
t.Fatalf("Log.Level = %q, want debug", cfg.Log.Level)
|
||||
}
|
||||
if cfg.Log.Format != "text" {
|
||||
t.Fatalf("Log.Format = %q, want text", cfg.Log.Format)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadAppliesLogEnvironmentOverrides(t *testing.T) {
|
||||
t.Setenv("CODESPACE_LOG_LEVEL", "warn")
|
||||
t.Setenv("CODESPACE_LOG_FORMAT", "json")
|
||||
|
||||
cfg, err := Load(filepath.Join(t.TempDir(), "missing.yaml"))
|
||||
if err != nil {
|
||||
t.Fatalf("Load returned error: %v", err)
|
||||
}
|
||||
if cfg.Log.Level != "warn" {
|
||||
t.Fatalf("Log.Level = %q, want warn", cfg.Log.Level)
|
||||
}
|
||||
if cfg.Log.Format != "json" {
|
||||
t.Fatalf("Log.Format = %q, want json", cfg.Log.Format)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run config tests to see failure**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./pkg/config
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Fails because `Config.Log` and `LogConfig` do not exist yet.
|
||||
|
||||
- [ ] **Step 3: Implement log config**
|
||||
|
||||
Modify `pkg/config/config.go`:
|
||||
|
||||
```go
|
||||
type Config struct {
|
||||
Server ServerConfig `yaml:"server"`
|
||||
Workspace WorkspaceConfig `yaml:"workspace"`
|
||||
Process ProcessConfig `yaml:"process"`
|
||||
Log LogConfig `yaml:"log"`
|
||||
}
|
||||
|
||||
type LogConfig struct {
|
||||
Level string `yaml:"level"`
|
||||
Format string `yaml:"format"`
|
||||
}
|
||||
```
|
||||
|
||||
Update `rawConfig`:
|
||||
|
||||
```go
|
||||
type rawConfig struct {
|
||||
Server rawServerConfig `yaml:"server"`
|
||||
Workspace WorkspaceConfig `yaml:"workspace"`
|
||||
Process ProcessConfig `yaml:"process"`
|
||||
Log LogConfig `yaml:"log"`
|
||||
}
|
||||
```
|
||||
|
||||
Update defaults:
|
||||
|
||||
```go
|
||||
Log: LogConfig{Level: "info", Format: "json"},
|
||||
```
|
||||
|
||||
Update `applyRaw`:
|
||||
|
||||
```go
|
||||
if raw.Log.Level != "" {
|
||||
cfg.Log.Level = raw.Log.Level
|
||||
}
|
||||
if raw.Log.Format != "" {
|
||||
cfg.Log.Format = raw.Log.Format
|
||||
}
|
||||
```
|
||||
|
||||
Update `applyEnv`:
|
||||
|
||||
```go
|
||||
if v := os.Getenv("CODESPACE_LOG_LEVEL"); v != "" {
|
||||
cfg.Log.Level = v
|
||||
}
|
||||
if v := os.Getenv("CODESPACE_LOG_FORMAT"); v != "" {
|
||||
cfg.Log.Format = v
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update config YAML**
|
||||
|
||||
Modify `configs/config.yaml`:
|
||||
|
||||
```yaml
|
||||
log:
|
||||
level: "info"
|
||||
format: "json"
|
||||
```
|
||||
|
||||
Keep existing `server`, `workspace`, and `process` blocks unchanged.
|
||||
|
||||
- [ ] **Step 5: Verify config task**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
gofmt -w pkg/config
|
||||
go test ./pkg/config
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Config tests pass.
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Replace logger package with slog
|
||||
|
||||
**Files:**
|
||||
- Modify: `pkg/logger/logger.go`
|
||||
- Create: `pkg/logger/logger_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `config.LogConfig`.
|
||||
- Produces: `logger.New(cfg config.LogConfig) (*slog.Logger, error)`.
|
||||
- Produces: slog-only application logging factory.
|
||||
|
||||
- [ ] **Step 1: Write logger tests first**
|
||||
|
||||
Create `pkg/logger/logger_test.go`:
|
||||
|
||||
```go
|
||||
package logger
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"codespace/pkg/config"
|
||||
)
|
||||
|
||||
func TestNewJSONInfoLogger(t *testing.T) {
|
||||
lg, err := New(config.LogConfig{Level: "info", Format: "json"})
|
||||
if err != nil {
|
||||
t.Fatalf("New returned error: %v", err)
|
||||
}
|
||||
if lg == nil {
|
||||
t.Fatal("logger is nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewTextDebugLogger(t *testing.T) {
|
||||
lg, err := New(config.LogConfig{Level: "debug", Format: "text"})
|
||||
if err != nil {
|
||||
t.Fatalf("New returned error: %v", err)
|
||||
}
|
||||
if lg == nil {
|
||||
t.Fatal("logger is nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewRejectsInvalidLevel(t *testing.T) {
|
||||
if _, err := New(config.LogConfig{Level: "verbose", Format: "json"}); err == nil {
|
||||
t.Fatal("expected invalid level error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewRejectsInvalidFormat(t *testing.T) {
|
||||
if _, err := New(config.LogConfig{Level: "info", Format: "xml"}); err == nil {
|
||||
t.Fatal("expected invalid format error")
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run logger tests to see failure**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
go test ./pkg/logger
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Fails because `logger.New` still has the old signature and returns `*log.Logger`.
|
||||
|
||||
- [ ] **Step 3: Implement slog logger factory**
|
||||
|
||||
Replace `pkg/logger/logger.go` with:
|
||||
|
||||
```go
|
||||
package logger
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
"codespace/pkg/config"
|
||||
)
|
||||
|
||||
func New(cfg config.LogConfig) (*slog.Logger, error) {
|
||||
level, err := parseLevel(cfg.Level)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
opts := &slog.HandlerOptions{Level: level}
|
||||
var handler slog.Handler
|
||||
switch strings.ToLower(cfg.Format) {
|
||||
case "json", "":
|
||||
handler = slog.NewJSONHandler(os.Stdout, opts)
|
||||
case "text":
|
||||
handler = slog.NewTextHandler(os.Stdout, opts)
|
||||
default:
|
||||
return nil, fmt.Errorf("invalid log format %q", cfg.Format)
|
||||
}
|
||||
|
||||
lg := slog.New(handler)
|
||||
slog.SetDefault(lg)
|
||||
return lg, nil
|
||||
}
|
||||
|
||||
func parseLevel(value string) (slog.Level, error) {
|
||||
switch strings.ToLower(value) {
|
||||
case "debug":
|
||||
return slog.LevelDebug, nil
|
||||
case "info", "":
|
||||
return slog.LevelInfo, nil
|
||||
case "warn":
|
||||
return slog.LevelWarn, nil
|
||||
case "error":
|
||||
return slog.LevelError, nil
|
||||
default:
|
||||
return slog.LevelInfo, fmt.Errorf("invalid log level %q", value)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Verify logger package**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
gofmt -w pkg/logger
|
||||
go test ./pkg/logger
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Logger tests pass.
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Add Gin request logging middleware
|
||||
|
||||
**Files:**
|
||||
- Create: `internal/api/logging_middleware.go`
|
||||
- Modify: `internal/api/router.go`
|
||||
- Modify: `internal/api/router_test.go`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `*slog.Logger`.
|
||||
- Produces: `requestLogger(lg *slog.Logger) gin.HandlerFunc`.
|
||||
- Produces: `api.NewRouter(..., lg *slog.Logger) *gin.Engine`.
|
||||
|
||||
- [ ] **Step 1: Update router test setup**
|
||||
|
||||
Modify `internal/api/router_test.go` setup to pass nil logger:
|
||||
|
||||
```go
|
||||
return NewRouter(wsSvc, fileSvc, procSvc, nil)
|
||||
```
|
||||
|
||||
Existing tests must keep the same response assertions.
|
||||
|
||||
- [ ] **Step 2: Implement request logging middleware**
|
||||
|
||||
Create `internal/api/logging_middleware.go`:
|
||||
|
||||
```go
|
||||
package api
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
func requestLogger(lg *slog.Logger) gin.HandlerFunc {
|
||||
if lg == nil {
|
||||
lg = slog.Default()
|
||||
}
|
||||
return func(c *gin.Context) {
|
||||
if c.FullPath() == "/healthz" || c.Request.URL.Path == "/healthz" {
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
|
||||
start := time.Now()
|
||||
c.Next()
|
||||
|
||||
status := c.Writer.Status()
|
||||
path := c.FullPath()
|
||||
if path == "" {
|
||||
path = c.Request.URL.Path
|
||||
}
|
||||
attrs := []any{
|
||||
"method", c.Request.Method,
|
||||
"path", path,
|
||||
"status", status,
|
||||
"latency_ms", time.Since(start).Milliseconds(),
|
||||
"client_ip", c.ClientIP(),
|
||||
}
|
||||
if len(c.Errors) > 0 {
|
||||
attrs = append(attrs, "error", c.Errors.String())
|
||||
}
|
||||
|
||||
switch {
|
||||
case status >= http.StatusInternalServerError:
|
||||
lg.Error("http request completed", attrs...)
|
||||
case status >= http.StatusBadRequest:
|
||||
lg.Warn("http request completed", attrs...)
|
||||
default:
|
||||
lg.Info("http request completed", attrs...)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Update router signature and middleware**
|
||||
|
||||
Modify `internal/api/router.go`:
|
||||
|
||||
```go
|
||||
func NewRouter(workspaces *service.WorkspaceService, files *service.FileService, processes *service.ProcessService, lg *slog.Logger) *gin.Engine
|
||||
```
|
||||
|
||||
Add imports:
|
||||
|
||||
```go
|
||||
"log/slog"
|
||||
```
|
||||
|
||||
After `r.Use(gin.Recovery())`, add:
|
||||
|
||||
```go
|
||||
r.Use(requestLogger(lg))
|
||||
```
|
||||
|
||||
Keep `gin.New()` and do not use `gin.Default()` or `gin.Logger()`.
|
||||
|
||||
- [ ] **Step 4: Verify API package**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
gofmt -w internal/api
|
||||
go test ./internal/api
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- API tests pass.
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Inject slog into services and main
|
||||
|
||||
**Files:**
|
||||
- Modify: `cmd/server/main.go`
|
||||
- Modify: `internal/service/workspace_service.go`
|
||||
- Modify: `internal/service/process_service.go`
|
||||
- Modify tests that call changed constructors.
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `*slog.Logger`.
|
||||
- Produces: `service.NewWorkspaceService(workspaces workspace.Manager, processes process.Manager, lg *slog.Logger) *WorkspaceService`.
|
||||
- Produces: `service.NewProcessService(workspaces workspace.Manager, processes process.Manager, lg *slog.Logger) *ProcessService`.
|
||||
|
||||
- [ ] **Step 1: Update WorkspaceService**
|
||||
|
||||
Modify `internal/service/workspace_service.go`:
|
||||
|
||||
- Add `log/slog` import.
|
||||
- Add `logger *slog.Logger` field.
|
||||
- Change constructor to accept logger.
|
||||
- If nil, use `slog.Default()`.
|
||||
- Log successful create/delete.
|
||||
- Log failure to stop process before delete.
|
||||
|
||||
Required log messages:
|
||||
|
||||
```go
|
||||
s.logger.Info("workspace created", "workspace_id", id)
|
||||
s.logger.Error("failed to stop workspace process before delete", "workspace_id", id, "error", err)
|
||||
s.logger.Info("workspace deleted", "workspace_id", id)
|
||||
```
|
||||
|
||||
Do not log workspace root.
|
||||
|
||||
- [ ] **Step 2: Update ProcessService**
|
||||
|
||||
Modify `internal/service/process_service.go`:
|
||||
|
||||
- Add `log/slog` import.
|
||||
- Add `logger *slog.Logger` field.
|
||||
- Change constructor to accept logger.
|
||||
- If nil, use `slog.Default()`.
|
||||
- Log start/stop/restart success and failure.
|
||||
|
||||
Required log messages:
|
||||
|
||||
```go
|
||||
s.logger.Info("process started", "workspace_id", workspaceID, "pid", status.PID)
|
||||
s.logger.Error("process start failed", "workspace_id", workspaceID, "error", err)
|
||||
s.logger.Info("process stopped", "workspace_id", workspaceID)
|
||||
s.logger.Error("process stop failed", "workspace_id", workspaceID, "error", err)
|
||||
s.logger.Info("process restarted", "workspace_id", workspaceID, "pid", status.PID)
|
||||
s.logger.Error("process restart failed", "workspace_id", workspaceID, "error", err)
|
||||
```
|
||||
|
||||
Do not log command, env, or workspace root.
|
||||
|
||||
- [ ] **Step 3: Update main wiring**
|
||||
|
||||
Modify `cmd/server/main.go`:
|
||||
|
||||
- Keep standard library `log` only for config/logger initialization failure before slog exists.
|
||||
- Initialize logger:
|
||||
|
||||
```go
|
||||
lg, err := logger.New(cfg.Log)
|
||||
if err != nil {
|
||||
log.Fatalf("failed to initialize logger: %v", err)
|
||||
}
|
||||
```
|
||||
|
||||
- Pass logger into services/router:
|
||||
|
||||
```go
|
||||
workspaceSvc := service.NewWorkspaceService(workspaces, processes, lg)
|
||||
fileSvc := service.NewFileService(workspaces)
|
||||
processSvc := service.NewProcessService(workspaces, processes, lg)
|
||||
router := api.NewRouter(workspaceSvc, fileSvc, processSvc, lg)
|
||||
```
|
||||
|
||||
- Replace `lg.Printf` with slog calls:
|
||||
|
||||
```go
|
||||
lg.Info("server starting",
|
||||
"addr", cfg.Server.Addr,
|
||||
"workspace_root", cfg.Workspace.Root,
|
||||
"read_timeout", cfg.Server.ReadTimeout.String(),
|
||||
"write_timeout", cfg.Server.WriteTimeout.String(),
|
||||
"idle_timeout", cfg.Server.IdleTimeout.String(),
|
||||
"max_header_bytes", cfg.Server.MaxHeaderBytes,
|
||||
)
|
||||
```
|
||||
|
||||
- Replace server goroutine fatal path:
|
||||
|
||||
```go
|
||||
lg.Error("server failed", "error", err)
|
||||
os.Exit(1)
|
||||
```
|
||||
|
||||
- Replace shutdown logs:
|
||||
|
||||
```go
|
||||
lg.Info("server shutting down")
|
||||
lg.Error("server forced shutdown", "error", err)
|
||||
os.Exit(1)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update constructor call sites**
|
||||
|
||||
Update all call sites in tests and code:
|
||||
|
||||
```go
|
||||
service.NewWorkspaceService(wsMgr, procMgr, nil)
|
||||
service.NewProcessService(wsMgr, procMgr, nil)
|
||||
api.NewRouter(wsSvc, fileSvc, procSvc, nil)
|
||||
```
|
||||
|
||||
Run this grep to find remaining old signatures:
|
||||
|
||||
```sh
|
||||
grep -R "NewWorkspaceService\|NewProcessService\|NewRouter" -n cmd internal pkg | grep -v vendor
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- All constructor calls include logger where required.
|
||||
|
||||
- [ ] **Step 5: Verify packages**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
gofmt -w cmd internal pkg
|
||||
go test ./internal/service ./internal/api ./cmd/server
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Tests/compilation pass.
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Documentation and global slog-only validation
|
||||
|
||||
**Files:**
|
||||
- Modify: `README.md`
|
||||
- Modify: docs/spec already updated; no new spec edits needed unless implementation differs.
|
||||
|
||||
**Interfaces:**
|
||||
- Produces documented logging config and slog-only rule.
|
||||
|
||||
- [ ] **Step 1: Update README**
|
||||
|
||||
Add a logging configuration section:
|
||||
|
||||
```markdown
|
||||
## Logging
|
||||
|
||||
codespace uses the Go standard library `log/slog` for application logs.
|
||||
|
||||
Default config:
|
||||
|
||||
```yaml
|
||||
log:
|
||||
level: "info"
|
||||
format: "json"
|
||||
```
|
||||
|
||||
Environment overrides:
|
||||
|
||||
- `CODESPACE_LOG_LEVEL` (`debug`, `info`, `warn`, `error`)
|
||||
- `CODESPACE_LOG_FORMAT` (`json`, `text`)
|
||||
|
||||
All application logs must go through `log/slog`. Do not use `fmt.Print*`, `log.Printf`, `gin.Logger()`, or third-party logging libraries in server runtime code.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run final validation**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
gofmt -w cmd internal pkg
|
||||
go mod tidy
|
||||
go test ./...
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- All tests pass.
|
||||
|
||||
- [ ] **Step 3: Check slog-only constraint**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
grep -R "log\.Printf\|log\.Println\|log\.Fatal\|fmt\.Print\|gin\.Default\|gin\.Logger" cmd internal pkg || true
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Only allowed match is the logger-initialization fallback in `cmd/server/main.go`, if present.
|
||||
- No `fmt.Print*`, `gin.Default`, `gin.Logger`, or runtime `log.Printf`/`log.Println` matches.
|
||||
|
||||
- [ ] **Step 4: Run server smoke test**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
(go run ./cmd/server > /tmp/codespace-server.log 2>&1 & pid=$!; \
|
||||
for i in $(seq 1 30); do \
|
||||
if curl -fsS http://localhost:8080/healthz; then \
|
||||
kill $pid; wait $pid 2>/dev/null || true; exit 0; \
|
||||
fi; \
|
||||
sleep 0.2; \
|
||||
done; \
|
||||
kill $pid 2>/dev/null || true; \
|
||||
wait $pid 2>/dev/null || true; \
|
||||
cat /tmp/codespace-server.log; exit 1)
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
```json
|
||||
{"status":"ok"}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Inspect git diff scope**
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
git diff --stat
|
||||
git status --short
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- Changes are limited to logging config, logger package, API middleware/router signature, workspace/process service logging, docs, and tests.
|
||||
- No unrelated auth/CORS/Git/LSP/frontend changes.
|
||||
|
||||
---
|
||||
|
||||
## Codex Execution Prompt
|
||||
|
||||
Use this exact structure when dispatching Codex:
|
||||
|
||||
```text
|
||||
## Context
|
||||
- Project root: /Users/taochen/llm/codespace
|
||||
- Current branch: main
|
||||
- Go baseline: 1.25
|
||||
- Existing API router uses Gin and explicit http.Server composition.
|
||||
- Design spec: docs/superpowers/specs/2026-07-02-slog-logging-design.md
|
||||
- Implementation plan: docs/superpowers/plans/2026-07-02-slog-logging.md
|
||||
|
||||
## Task
|
||||
Add structured logging with Go standard library log/slog.
|
||||
Steps:
|
||||
1. Add log config (`log.level`, `log.format`) with YAML/env support.
|
||||
2. Replace pkg/logger with slog logger factory.
|
||||
3. Add Gin request logging middleware, skipping /healthz.
|
||||
4. Inject slog logger into main, API router, WorkspaceService, and ProcessService.
|
||||
5. Add workspace/process lifecycle logs.
|
||||
6. Update README and tests.
|
||||
7. Run final validation.
|
||||
|
||||
## Constraints
|
||||
- All application logs must use log/slog from now on.
|
||||
- Do not use fmt.Print* as logging.
|
||||
- Do not use gin.Default() or gin.Logger().
|
||||
- Do not use log.Printf/log.Println/log.Fatal/log.Fatalf in runtime paths; only cmd/server/main.go may use log.Fatalf before slog initialization exists.
|
||||
- Do not log file contents, request bodies, response bodies, OpenCode stdout/stderr, environment variables, tokens, secrets, command, env, or workspace root in request/process/workspace lifecycle logs.
|
||||
- Do not change API paths, methods, or response JSON shapes.
|
||||
- Do not add third-party logging dependencies or unrelated features.
|
||||
- Do not commit.
|
||||
|
||||
## Acceptance
|
||||
- gofmt -w cmd internal pkg succeeds.
|
||||
- go mod tidy succeeds.
|
||||
- go test ./... passes.
|
||||
- slog-only grep has no disallowed runtime logging.
|
||||
- go run ./cmd/server starts and /healthz returns {"status":"ok"}.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-Review
|
||||
|
||||
- Spec coverage: Plan covers config, logger package, request middleware, service lifecycle logs, main wiring, README, and slog-only validation.
|
||||
- Placeholder scan: No TBD/TODO/ambiguous implementation steps remain.
|
||||
- Type consistency: `logger.New(config.LogConfig) (*slog.Logger, error)`, `api.NewRouter(..., *slog.Logger)`, `NewWorkspaceService(..., *slog.Logger)`, and `NewProcessService(..., *slog.Logger)` are consistent across tasks.
|
||||
- Scope check: Plan is limited to logging. It does not add request IDs, tracing, third-party logging, auth, CORS, Git, LSP, frontend, watcher streaming, or Agent features.
|
||||
@@ -0,0 +1,350 @@
|
||||
# slog Structured Logging Design
|
||||
|
||||
日期:2026-07-02
|
||||
|
||||
## 目标
|
||||
|
||||
为 codespace 后端增加基于 Go 标准库 `log/slog` 的结构化日志。目标是建立清晰、低噪音、不会泄露用户内容的日志边界,而不是到处撒日志。
|
||||
|
||||
本次目标:
|
||||
|
||||
- 使用 `log/slog` 替代当前 `log.Logger`。
|
||||
- 默认输出 JSON 日志。
|
||||
- 支持日志 level 和 format 配置。
|
||||
- 为 Gin 增加 request logging middleware。
|
||||
- 记录 workspace/process 生命周期关键事件。
|
||||
- 建立全局约束:后续所有应用日志都必须使用 `log/slog`。
|
||||
- 不记录文件内容、请求 body、token、环境变量、OpenCode 输出、宿主机绝对路径。
|
||||
|
||||
## 当前状态
|
||||
|
||||
当前代码:
|
||||
|
||||
- `pkg/logger.New()` 返回 `*log.Logger`。
|
||||
- `cmd/server/main.go` 使用 `log.Fatalf` 和 `lg.Printf`。
|
||||
- `internal/api.NewRouter(...)` 没有 logger 参数。
|
||||
- Gin router 使用 `gin.New()` + `gin.Recovery()`。
|
||||
- service/process/workspace 层基本没有日志。
|
||||
|
||||
## 非目标
|
||||
|
||||
本次不做:
|
||||
|
||||
- 不引入第三方日志库。
|
||||
- 不接入 OpenTelemetry、Prometheus、ELK、Loki。
|
||||
- 不实现 request ID / trace ID。
|
||||
- 不记录文件内容、请求 body、OpenCode stdout/stderr。
|
||||
- 不记录 workspace root 绝对路径。
|
||||
- 不在高频文件读写路径里打业务日志。
|
||||
- 不增加认证、CORS、rate limit、Git、LSP、Agent 功能。
|
||||
|
||||
## 全局日志约束
|
||||
|
||||
从本次改动开始,后续所有应用日志必须走 `log/slog`。
|
||||
|
||||
禁止在应用代码中重新引入这些旁路日志方式:
|
||||
|
||||
- `log.Printf` / `log.Println` / `log.Fatal` / `log.Fatalf`,除非是在 logger 初始化失败前的最后兜底。
|
||||
- `fmt.Print*` 用作日志。
|
||||
- `gin.Logger()` 或 `gin.Default()` 隐式 request logger。
|
||||
- 第三方日志库。
|
||||
|
||||
允许:
|
||||
|
||||
- 测试中使用 `t.Log*`。
|
||||
- CLI/工具脚本中显式面向用户的 stdout 输出,但不能混进服务端运行路径。
|
||||
- logger 初始化失败之前,`cmd/server/main.go` 可以使用标准库 `log.Fatalf` 作为兜底,因为此时 slog logger 尚未存在。
|
||||
|
||||
验证方式:
|
||||
|
||||
```sh
|
||||
grep -R "log\.Printf\|log\.Println\|log\.Fatal\|fmt\.Print\|gin\.Default\|gin\.Logger" cmd internal pkg || true
|
||||
```
|
||||
|
||||
预期只允许出现 logger 初始化失败前的兜底 `log.Fatalf`,不允许出现运行期旁路日志。
|
||||
|
||||
## 配置设计
|
||||
|
||||
`configs/config.yaml` 增加:
|
||||
|
||||
```yaml
|
||||
log:
|
||||
level: "info"
|
||||
format: "json"
|
||||
```
|
||||
|
||||
`pkg/config` 增加:
|
||||
|
||||
```go
|
||||
type LogConfig struct {
|
||||
Level string `yaml:"level"`
|
||||
Format string `yaml:"format"`
|
||||
}
|
||||
```
|
||||
|
||||
`Config` 增加:
|
||||
|
||||
```go
|
||||
type Config struct {
|
||||
Server ServerConfig `yaml:"server"`
|
||||
Workspace WorkspaceConfig `yaml:"workspace"`
|
||||
Process ProcessConfig `yaml:"process"`
|
||||
Log LogConfig `yaml:"log"`
|
||||
}
|
||||
```
|
||||
|
||||
默认值:
|
||||
|
||||
- `level`: `info`
|
||||
- `format`: `json`
|
||||
|
||||
环境变量:
|
||||
|
||||
- `CODESPACE_LOG_LEVEL`
|
||||
- `CODESPACE_LOG_FORMAT`
|
||||
|
||||
支持 level:
|
||||
|
||||
- `debug`
|
||||
- `info`
|
||||
- `warn`
|
||||
- `error`
|
||||
|
||||
支持 format:
|
||||
|
||||
- `json`
|
||||
- `text`
|
||||
|
||||
虽然默认使用 JSON,但保留 text 是低成本本地调试能力。
|
||||
|
||||
## Logger 包设计
|
||||
|
||||
`pkg/logger` 改为创建 `*slog.Logger`:
|
||||
|
||||
```go
|
||||
func New(cfg config.LogConfig) (*slog.Logger, error)
|
||||
```
|
||||
|
||||
行为:
|
||||
|
||||
- `format=json` 使用 `slog.NewJSONHandler(os.Stdout, opts)`。
|
||||
- `format=text` 使用 `slog.NewTextHandler(os.Stdout, opts)`。
|
||||
- `level` 解析成 `slog.LevelDebug/Info/Warn/Error`。
|
||||
- 非法 level 或 format 返回 error。
|
||||
- 成功创建后调用 `slog.SetDefault(lg)`。
|
||||
|
||||
不保留旧 `*log.Logger` 包装层。项目还小,直接切干净。
|
||||
|
||||
## main.go 设计
|
||||
|
||||
启动流程:
|
||||
|
||||
1. 先加载 config。
|
||||
2. 用 `logger.New(cfg.Log)` 初始化 slog。
|
||||
3. 初始化失败时使用标准库 `log.Fatalf`,因为此时结构化 logger 还不存在。
|
||||
4. 后续全部使用 `lg.Info/Warn/Error`。
|
||||
|
||||
启动日志:
|
||||
|
||||
```go
|
||||
lg.Info("server starting",
|
||||
"addr", cfg.Server.Addr,
|
||||
"workspace_root", cfg.Workspace.Root,
|
||||
"read_timeout", cfg.Server.ReadTimeout.String(),
|
||||
"write_timeout", cfg.Server.WriteTimeout.String(),
|
||||
"idle_timeout", cfg.Server.IdleTimeout.String(),
|
||||
"max_header_bytes", cfg.Server.MaxHeaderBytes,
|
||||
)
|
||||
```
|
||||
|
||||
注意:启动日志可以记录配置里的 workspace root,因为这是服务启动配置;但 request/process/workspace lifecycle 日志不记录宿主机绝对路径。
|
||||
|
||||
错误日志:
|
||||
|
||||
```go
|
||||
lg.Error("server failed", "error", err)
|
||||
os.Exit(1)
|
||||
```
|
||||
|
||||
`slog` 没有 `Fatal`,不要自己造复杂 wrapper。
|
||||
|
||||
## Gin request logging middleware
|
||||
|
||||
新增 middleware:
|
||||
|
||||
```go
|
||||
func requestLogger(lg *slog.Logger) gin.HandlerFunc
|
||||
```
|
||||
|
||||
`api.NewRouter` 签名改为:
|
||||
|
||||
```go
|
||||
func NewRouter(
|
||||
workspaces *service.WorkspaceService,
|
||||
files *service.FileService,
|
||||
processes *service.ProcessService,
|
||||
lg *slog.Logger,
|
||||
) *gin.Engine
|
||||
```
|
||||
|
||||
如果 `lg == nil`,使用 `slog.Default()`。
|
||||
|
||||
middleware 字段:
|
||||
|
||||
- `method`
|
||||
- `path`
|
||||
- `status`
|
||||
- `latency_ms`
|
||||
- `client_ip`
|
||||
- `error`
|
||||
|
||||
level 规则:
|
||||
|
||||
- 2xx/3xx:`Info`
|
||||
- 4xx:`Warn`
|
||||
- 5xx:`Error`
|
||||
|
||||
`/healthz` 不记录,避免健康检查刷屏。
|
||||
|
||||
request logging 不记录:
|
||||
|
||||
- request body
|
||||
- response body
|
||||
- query string 中的敏感内容
|
||||
|
||||
`path` 使用路由模板或 URL path,不包含 query string。
|
||||
|
||||
## 业务日志边界
|
||||
|
||||
### WorkspaceService
|
||||
|
||||
增加 logger 字段:
|
||||
|
||||
```go
|
||||
type WorkspaceService struct {
|
||||
workspaces workspace.Manager
|
||||
processes process.Manager
|
||||
logger *slog.Logger
|
||||
}
|
||||
```
|
||||
|
||||
构造函数接受 logger:
|
||||
|
||||
```go
|
||||
func NewWorkspaceService(workspaces workspace.Manager, processes process.Manager, lg *slog.Logger) *WorkspaceService
|
||||
```
|
||||
|
||||
如果 `lg == nil`,使用 `slog.Default()`。
|
||||
|
||||
记录:
|
||||
|
||||
- create 成功:`workspace created`,字段 `workspace_id`
|
||||
- delete 成功:`workspace deleted`,字段 `workspace_id`
|
||||
- delete 前停止进程失败:`failed to stop workspace process before delete`,字段 `workspace_id`、`error`
|
||||
|
||||
不记录 workspace root。
|
||||
|
||||
### ProcessService
|
||||
|
||||
增加 logger 字段:
|
||||
|
||||
```go
|
||||
type ProcessService struct {
|
||||
workspaces workspace.Manager
|
||||
processes process.Manager
|
||||
logger *slog.Logger
|
||||
}
|
||||
```
|
||||
|
||||
构造函数接受 logger:
|
||||
|
||||
```go
|
||||
func NewProcessService(workspaces workspace.Manager, processes process.Manager, lg *slog.Logger) *ProcessService
|
||||
```
|
||||
|
||||
如果 `lg == nil`,使用 `slog.Default()`。
|
||||
|
||||
记录:
|
||||
|
||||
- start 成功:`process started`,字段 `workspace_id`、`pid`
|
||||
- start 失败:`process start failed`,字段 `workspace_id`、`error`
|
||||
- stop 成功:`process stopped`,字段 `workspace_id`
|
||||
- stop 失败:`process stop failed`,字段 `workspace_id`、`error`
|
||||
- restart 成功:`process restarted`,字段 `workspace_id`、`pid`
|
||||
- restart 失败:`process restart failed`,字段 `workspace_id`、`error`
|
||||
|
||||
不记录 command、env、workspace root。
|
||||
|
||||
### FileService
|
||||
|
||||
第一版不增加业务日志。文件操作高频且容易泄露用户路径/内容。request logging 已足够覆盖 HTTP 访问。
|
||||
|
||||
## API 兼容性
|
||||
|
||||
现有 API 路径、method、响应 JSON 不变。
|
||||
|
||||
唯一函数签名变化是内部构造函数:
|
||||
|
||||
- `api.NewRouter(..., lg *slog.Logger)`
|
||||
- `service.NewWorkspaceService(..., lg *slog.Logger)`
|
||||
- `service.NewProcessService(..., lg *slog.Logger)`
|
||||
|
||||
测试可传 `nil`,由实现回退到 `slog.Default()`。
|
||||
|
||||
## 测试策略
|
||||
|
||||
### logger tests
|
||||
|
||||
覆盖:
|
||||
|
||||
- `logger.New(config.LogConfig{Level: "info", Format: "json"})` 成功。
|
||||
- `logger.New(config.LogConfig{Level: "debug", Format: "text"})` 成功。
|
||||
- 非法 level 返回 error。
|
||||
- 非法 format 返回 error。
|
||||
|
||||
### config tests
|
||||
|
||||
覆盖:
|
||||
|
||||
- 默认 log config 是 `info/json`。
|
||||
- YAML 能覆盖 log config。
|
||||
- 环境变量能覆盖 log config。
|
||||
|
||||
### API tests
|
||||
|
||||
现有 `httptest` 行为不变。
|
||||
|
||||
额外覆盖:
|
||||
|
||||
- `api.NewRouter(..., nil)` 不 panic。
|
||||
- `/healthz` 正常返回,不受 logging middleware 影响。
|
||||
|
||||
### 全局验证
|
||||
|
||||
必须通过:
|
||||
|
||||
```sh
|
||||
gofmt -w cmd internal pkg
|
||||
go test ./...
|
||||
go run ./cmd/server
|
||||
curl http://localhost:8080/healthz
|
||||
```
|
||||
|
||||
## 风险与处理
|
||||
|
||||
- 风险:日志泄露用户内容。处理:禁止记录 body、文件内容、OpenCode 输出、env、token。
|
||||
- 风险:日志噪音过大。处理:不记录 `/healthz`,不记录 FileService 高频业务日志。
|
||||
- 风险:logger 注入让构造函数变多。处理:只给 router、workspace service、process service 加 logger;FileService 不加。
|
||||
- 风险:测试输出被 JSON 日志污染。处理:测试传 nil 或使用 `slog.New(slog.NewTextHandler(io.Discard, nil))`。
|
||||
|
||||
## 完成标准
|
||||
|
||||
完成后满足:
|
||||
|
||||
- `pkg/logger` 使用 `log/slog`。
|
||||
- `cmd/server/main.go` 不再使用 `lg.Printf`。
|
||||
- `api.NewRouter` 接受 logger 并安装 request logging middleware。
|
||||
- workspace/process 生命周期有结构化日志。
|
||||
- log 配置支持 YAML 和环境变量。
|
||||
- 全局约束检查中无运行期旁路日志;只允许 logger 初始化失败前的兜底 `log.Fatalf`。
|
||||
- server smoke test 通过。
|
||||
Reference in New Issue
Block a user