From 60336cf5a7c83469be5fb8b8d677c4d3bdc06d06 Mon Sep 17 00:00:00 2001 From: Etai Lev Ran Date: Wed, 8 Jul 2026 18:08:36 +0300 Subject: [PATCH 1/4] refactor: eliminate small-package smell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delete internal/httputil: inline writeJSON/writeError as unexported helpers in cmd/proxy (httputil.go) and internal/server (httputil.go) - Delete internal/metrics: fold HTTPRequestsTotal, HTTPRequestDuration, and Register() into internal/server as unexported vars + RegisterMetrics() - Move internal/inference → cmd/proxy/inference/ (proxy-only demo code; no other package used it) - Split internal/config into internal/charonconfig (CharonOptions + ByteSize) and internal/proxyconfig (ProxyOptions); each holds its own private YAML loader and TelemetryOptions; charon.listen is passed as a plain string to proxyconfig so there is no cross-package dependency Signed-off-by: Etai Lev Ran --- cmd/charon/main.go | 7 +- cmd/charon/main_test.go | 4 +- cmd/proxy/assemble.go | 2 +- cmd/proxy/assemble_test.go | 2 +- cmd/proxy/disruptive_test.go | 2 +- cmd/proxy/handler.go | 33 +- cmd/proxy/httputil.go | 16 + {internal => cmd/proxy}/inference/client.go | 0 .../proxy}/inference/client_test.go | 2 +- {internal => cmd/proxy}/inference/mock.go | 0 {internal => cmd/proxy}/inference/types.go | 0 cmd/proxy/main.go | 6 +- cmd/proxy/sse.go | 7 +- cmd/proxy/stateless_parity_test.go | 2 +- cmd/proxy/testhelpers_test.go | 2 +- cmd/proxy/ws.go | 6 +- internal/{config => charonconfig}/bytesize.go | 16 +- .../{config => charonconfig}/bytesize_test.go | 23 +- internal/charonconfig/charonconfig.go | 189 +++++++++++ internal/charonconfig/charonconfig_test.go | 102 ++++++ .../testdata/bytesize.yaml | 0 .../testdata/config.yaml | 0 .../testdata/invalid.yaml | 0 internal/config/config.go | 17 - internal/config/config_test.go | 75 ----- internal/config/options.go | 314 ------------------ internal/httputil/http.go | 18 - internal/proxyconfig/proxyconfig.go | 209 ++++++++++++ .../proxyconfig_test.go} | 100 ++---- internal/proxyconfig/testdata/bytesize.yaml | 3 + internal/proxyconfig/testdata/config.yaml | 6 + internal/proxyconfig/testdata/invalid.yaml | 3 + internal/server/handlers.go | 45 ++- internal/server/httputil.go | 16 + internal/{metrics => server}/metrics.go | 14 +- internal/server/middleware.go | 6 +- test/integration/integration_test.go | 3 +- 37 files changed, 652 insertions(+), 598 deletions(-) create mode 100644 cmd/proxy/httputil.go rename {internal => cmd/proxy}/inference/client.go (100%) rename {internal => cmd/proxy}/inference/client_test.go (97%) rename {internal => cmd/proxy}/inference/mock.go (100%) rename {internal => cmd/proxy}/inference/types.go (100%) rename internal/{config => charonconfig}/bytesize.go (75%) rename internal/{config => charonconfig}/bytesize_test.go (77%) create mode 100644 internal/charonconfig/charonconfig.go create mode 100644 internal/charonconfig/charonconfig_test.go rename internal/{config => charonconfig}/testdata/bytesize.yaml (100%) rename internal/{config => charonconfig}/testdata/config.yaml (100%) rename internal/{config => charonconfig}/testdata/invalid.yaml (100%) delete mode 100644 internal/config/config.go delete mode 100644 internal/config/config_test.go delete mode 100644 internal/config/options.go delete mode 100644 internal/httputil/http.go create mode 100644 internal/proxyconfig/proxyconfig.go rename internal/{config/options_test.go => proxyconfig/proxyconfig_test.go} (58%) create mode 100644 internal/proxyconfig/testdata/bytesize.yaml create mode 100644 internal/proxyconfig/testdata/config.yaml create mode 100644 internal/proxyconfig/testdata/invalid.yaml create mode 100644 internal/server/httputil.go rename internal/{metrics => server}/metrics.go (72%) diff --git a/cmd/charon/main.go b/cmd/charon/main.go index fe9b25f..bd4ccd1 100644 --- a/cmd/charon/main.go +++ b/cmd/charon/main.go @@ -14,8 +14,7 @@ import ( "github.com/elevran/charon/internal/chainstore" pebblebe "github.com/elevran/charon/internal/chainstore/pebble" - "github.com/elevran/charon/internal/config" - "github.com/elevran/charon/internal/metrics" + "github.com/elevran/charon/internal/charonconfig" "github.com/elevran/charon/internal/server" "github.com/elevran/charon/internal/telemetry" ) @@ -29,7 +28,7 @@ func main() { func run() error { log := slog.New(slog.NewJSONHandler(os.Stdout, nil)) - opts := config.NewCharonOptions() + opts := charonconfig.NewCharonOptions() fs := flag.NewFlagSet("charon", flag.ExitOnError) opts.AddFlags(fs) _ = fs.Parse(os.Args[1:]) @@ -43,7 +42,7 @@ func run() error { } reg := prometheus.NewRegistry() - if err := metrics.Register(reg, ""); err != nil { + if err := server.RegisterMetrics(reg, ""); err != nil { log.Error("register metrics", "err", err) return err } diff --git a/cmd/charon/main_test.go b/cmd/charon/main_test.go index 0821a61..31a9ed9 100644 --- a/cmd/charon/main_test.go +++ b/cmd/charon/main_test.go @@ -6,11 +6,11 @@ import ( "github.com/stretchr/testify/require" - "github.com/elevran/charon/internal/config" + "github.com/elevran/charon/internal/charonconfig" ) func TestConfigDefaults(t *testing.T) { - opts := config.NewCharonOptions() + opts := charonconfig.NewCharonOptions() require.Equal(t, ":8081", opts.Listen) require.Equal(t, 30, opts.TTLDays) } diff --git a/cmd/proxy/assemble.go b/cmd/proxy/assemble.go index b0228ca..e27f004 100644 --- a/cmd/proxy/assemble.go +++ b/cmd/proxy/assemble.go @@ -5,7 +5,7 @@ import ( "fmt" "time" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" ) // marshalStoredResponse serialises an inference response and request metadata diff --git a/cmd/proxy/assemble_test.go b/cmd/proxy/assemble_test.go index 46c313f..0fd8cf0 100644 --- a/cmd/proxy/assemble_test.go +++ b/cmd/proxy/assemble_test.go @@ -5,7 +5,7 @@ import ( "testing" "time" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" ) // helper: decode json.RawMessage value for a key from map. diff --git a/cmd/proxy/disruptive_test.go b/cmd/proxy/disruptive_test.go index 4a46b53..97754fb 100644 --- a/cmd/proxy/disruptive_test.go +++ b/cmd/proxy/disruptive_test.go @@ -26,7 +26,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" ) // failingCharonMux returns an http.Handler that returns 507 for all PUT diff --git a/cmd/proxy/handler.go b/cmd/proxy/handler.go index aeab055..70e67b0 100644 --- a/cmd/proxy/handler.go +++ b/cmd/proxy/handler.go @@ -8,8 +8,7 @@ import ( "net/http" "time" - "github.com/elevran/charon/internal/httputil" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" "github.com/elevran/charon/pkg/charon" ) @@ -46,23 +45,23 @@ func RegisterHandlers(mux *http.ServeMux, h *Handler) { func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { bodyBytes, err := io.ReadAll(r.Body) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "failed to read request body") + writeError(w, http.StatusBadRequest, "failed to read request body") return } var req CreateRequest if err := json.Unmarshal(bodyBytes, &req); err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid request body") + writeError(w, http.StatusBadRequest, "invalid request body") return } if req.Model == "" { - httputil.WriteError(w, http.StatusBadRequest, "model is required") + writeError(w, http.StatusBadRequest, "model is required") return } var rawReq map[string]json.RawMessage if err := json.Unmarshal(bodyBytes, &rawReq); err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid request body") + writeError(w, http.StatusBadRequest, "invalid request body") return } @@ -76,7 +75,7 @@ func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { inputItems, err := inputToItems(req.Input) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid input") + writeError(w, http.StatusBadRequest, "invalid input") return } @@ -101,7 +100,7 @@ func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { infResp, err := h.inf.Complete(ctx, infMap) if err != nil { h.log.Error("inference complete", "err", err) - httputil.WriteError(w, http.StatusBadGateway, "inference error") + writeError(w, http.StatusBadGateway, "inference error") return } completedAt := time.Now() @@ -110,13 +109,13 @@ func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { responseBlob := marshalStoredResponse(infResp, req.PreviousResponseID, req.Instructions, req.Background) if err := h.charon.Store(ctx, infResp.ID, stagingID, tenantKey, responseBlob); err != nil { h.log.Error("charon store", "id", infResp.ID, "err", err) - httputil.WriteError(w, http.StatusInternalServerError, "storage error") + writeError(w, http.StatusInternalServerError, "storage error") return } } resource := buildResponseResource(infResp, req.PreviousResponseID, req.ShouldStore(), req.Background, createdAt, &completedAt) - httputil.WriteJSON(w, http.StatusOK, resource) + writeJSON(w, http.StatusOK, resource) } // HandleRetrieve handles GET /responses/{id}. @@ -133,7 +132,7 @@ func (h *Handler) HandleRetrieve(w http.ResponseWriter, r *http.Request) { var stored storedResponse if err := json.Unmarshal(retrieved.ResponseBlob, &stored); err != nil { h.log.Error("unmarshal stored response", "id", id, "err", err) - httputil.WriteError(w, http.StatusInternalServerError, "internal server error") + writeError(w, http.StatusInternalServerError, "internal server error") return } @@ -160,7 +159,7 @@ func (h *Handler) HandleRetrieve(w http.ResponseWriter, r *http.Request) { Metadata: map[string]string{}, ServiceTier: "default", } - httputil.WriteJSON(w, http.StatusOK, resource) + writeJSON(w, http.StatusOK, resource) } // HandleDelete handles DELETE /responses/{id}. @@ -181,10 +180,10 @@ func (h *Handler) HandleCompact(w http.ResponseWriter, r *http.Request) { } _ = json.NewDecoder(r.Body).Decode(&body) if body.Model == "" { - httputil.WriteError(w, http.StatusBadRequest, "model is required") + writeError(w, http.StatusBadRequest, "model is required") return } - httputil.WriteError(w, http.StatusNotImplemented, "compact not implemented") + writeError(w, http.StatusNotImplemented, "compact not implemented") } // HandleListOrWS is implemented in ws.go. @@ -196,17 +195,17 @@ func (h *Handler) HandleCompact(w http.ResponseWriter, r *http.Request) { func (h *Handler) mapCharonError(w http.ResponseWriter, err error, notFoundCode string) { switch { case errors.Is(err, charon.ErrNotFound): - httputil.WriteJSON(w, http.StatusNotFound, map[string]string{ + writeJSON(w, http.StatusNotFound, map[string]string{ "code": notFoundCode, "message": err.Error(), }) case errors.Is(err, charon.ErrChainCorrupted): - httputil.WriteJSON(w, http.StatusConflict, map[string]string{ + writeJSON(w, http.StatusConflict, map[string]string{ "code": "chain_corrupted", "message": err.Error(), }) default: h.log.Error("charon error", "err", err) - httputil.WriteError(w, http.StatusInternalServerError, "internal server error") + writeError(w, http.StatusInternalServerError, "internal server error") } } diff --git a/cmd/proxy/httputil.go b/cmd/proxy/httputil.go new file mode 100644 index 0000000..85ff3b7 --- /dev/null +++ b/cmd/proxy/httputil.go @@ -0,0 +1,16 @@ +package main + +import ( + "encoding/json" + "net/http" +) + +func writeJSON(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(v) +} + +func writeError(w http.ResponseWriter, status int, msg string) { + writeJSON(w, status, map[string]string{"error": msg}) +} diff --git a/internal/inference/client.go b/cmd/proxy/inference/client.go similarity index 100% rename from internal/inference/client.go rename to cmd/proxy/inference/client.go diff --git a/internal/inference/client_test.go b/cmd/proxy/inference/client_test.go similarity index 97% rename from internal/inference/client_test.go rename to cmd/proxy/inference/client_test.go index fa6e144..c46b815 100644 --- a/internal/inference/client_test.go +++ b/cmd/proxy/inference/client_test.go @@ -6,7 +6,7 @@ import ( "strings" "testing" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" ) var ctx = context.Background() diff --git a/internal/inference/mock.go b/cmd/proxy/inference/mock.go similarity index 100% rename from internal/inference/mock.go rename to cmd/proxy/inference/mock.go diff --git a/internal/inference/types.go b/cmd/proxy/inference/types.go similarity index 100% rename from internal/inference/types.go rename to cmd/proxy/inference/types.go diff --git a/cmd/proxy/main.go b/cmd/proxy/main.go index d9fdab3..9763a2d 100644 --- a/cmd/proxy/main.go +++ b/cmd/proxy/main.go @@ -10,8 +10,8 @@ import ( "syscall" "time" - "github.com/elevran/charon/internal/config" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" + "github.com/elevran/charon/internal/proxyconfig" "github.com/elevran/charon/internal/server" "github.com/elevran/charon/internal/telemetry" "github.com/elevran/charon/pkg/charon" @@ -26,7 +26,7 @@ func main() { func run() error { log := slog.New(slog.NewJSONHandler(os.Stdout, nil)) - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() fs := flag.NewFlagSet("proxy", flag.ExitOnError) opts.AddFlags(fs) _ = fs.Parse(os.Args[1:]) diff --git a/cmd/proxy/sse.go b/cmd/proxy/sse.go index 5ef44c2..c0f8204 100644 --- a/cmd/proxy/sse.go +++ b/cmd/proxy/sse.go @@ -7,8 +7,7 @@ import ( "strconv" "time" - "github.com/elevran/charon/internal/httputil" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" "github.com/elevran/charon/pkg/charon" ) @@ -64,7 +63,7 @@ func (h *Handler) handleStream(w http.ResponseWriter, r *http.Request, req Creat inputItems, err := inputToItems(req.Input) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid input") + writeError(w, http.StatusBadRequest, "invalid input") return } @@ -90,7 +89,7 @@ func (h *Handler) handleStream(w http.ResponseWriter, r *http.Request, req Creat ch, err := h.inf.Stream(ctx, infMap) if err != nil { h.log.Error("inference stream", "err", err) - httputil.WriteError(w, http.StatusBadGateway, "inference error") + writeError(w, http.StatusBadGateway, "inference error") return } diff --git a/cmd/proxy/stateless_parity_test.go b/cmd/proxy/stateless_parity_test.go index b146f41..0819302 100644 --- a/cmd/proxy/stateless_parity_test.go +++ b/cmd/proxy/stateless_parity_test.go @@ -28,7 +28,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" ) // capturedInfReq records a single request body received by the inference server. diff --git a/cmd/proxy/testhelpers_test.go b/cmd/proxy/testhelpers_test.go index 9fc9228..5b65461 100644 --- a/cmd/proxy/testhelpers_test.go +++ b/cmd/proxy/testhelpers_test.go @@ -21,9 +21,9 @@ import ( "github.com/gorilla/websocket" "github.com/stretchr/testify/require" + "github.com/elevran/charon/cmd/proxy/inference" "github.com/elevran/charon/internal/chainstore" pebblebe "github.com/elevran/charon/internal/chainstore/pebble" - "github.com/elevran/charon/internal/inference" "github.com/elevran/charon/internal/server" "github.com/elevran/charon/pkg/charon" ) diff --git a/cmd/proxy/ws.go b/cmd/proxy/ws.go index 67dd359..b6e3061 100644 --- a/cmd/proxy/ws.go +++ b/cmd/proxy/ws.go @@ -9,11 +9,9 @@ import ( "sync" "time" - "github.com/elevran/charon/internal/httputil" - "github.com/gorilla/websocket" - "github.com/elevran/charon/internal/inference" + "github.com/elevran/charon/cmd/proxy/inference" "github.com/elevran/charon/pkg/charon" ) @@ -78,7 +76,7 @@ func (h *Handler) HandleListOrWS(w http.ResponseWriter, r *http.Request) { h.HandleWebSocket(w, r) return } - httputil.WriteJSON(w, http.StatusOK, map[string]interface{}{ + writeJSON(w, http.StatusOK, map[string]interface{}{ "object": "list", "data": []interface{}{}, "has_more": false, diff --git a/internal/config/bytesize.go b/internal/charonconfig/bytesize.go similarity index 75% rename from internal/config/bytesize.go rename to internal/charonconfig/bytesize.go index 28eedfa..b4f9c84 100644 --- a/internal/config/bytesize.go +++ b/internal/charonconfig/bytesize.go @@ -1,4 +1,4 @@ -package config +package charonconfig import ( "encoding/json" @@ -7,9 +7,8 @@ import ( "strings" ) -// ByteSize is an int64 that unmarshals from either a plain integer (bytes) or -// a string with an optional unit suffix: B, KB, MB, GB. K=1024. -type ByteSize int64 +// byteSizeType is the concrete type; ByteSize aliases it so tests can reference it. +type byteSizeType int64 var unitMultipliers = map[string]int64{ "b": 1, @@ -18,15 +17,13 @@ var unitMultipliers = map[string]int64{ "gb": 1024 * 1024 * 1024, } -func (b *ByteSize) UnmarshalJSON(data []byte) error { - // Try plain number first. +func (b *byteSizeType) UnmarshalJSON(data []byte) error { var n int64 if err := json.Unmarshal(data, &n); err == nil { - *b = ByteSize(n) + *b = byteSizeType(n) return nil } - // Try quoted string. var s string if err := json.Unmarshal(data, &s); err != nil { return fmt.Errorf("bytesize: expected number or string, got %s", data) @@ -35,7 +32,6 @@ func (b *ByteSize) UnmarshalJSON(data []byte) error { s = strings.TrimSpace(s) lower := strings.ToLower(s) - // Find where the digits end. i := 0 for i < len(s) && (s[i] == '-' || s[i] == '+' || (s[i] >= '0' && s[i] <= '9')) { i++ @@ -63,6 +59,6 @@ func (b *ByteSize) UnmarshalJSON(data []byte) error { return fmt.Errorf("bytesize: negative size %q", s) } - *b = ByteSize(n * mult) + *b = byteSizeType(n * mult) return nil } diff --git a/internal/config/bytesize_test.go b/internal/charonconfig/bytesize_test.go similarity index 77% rename from internal/config/bytesize_test.go rename to internal/charonconfig/bytesize_test.go index ddc70b0..cb37d99 100644 --- a/internal/config/bytesize_test.go +++ b/internal/charonconfig/bytesize_test.go @@ -1,4 +1,4 @@ -package config_test +package charonconfig_test import ( "encoding/json" @@ -8,12 +8,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/elevran/charon/internal/config" + "github.com/elevran/charon/internal/charonconfig" ) -func unmarshalByteSize(t *testing.T, input string) config.ByteSize { +func unmarshalByteSize(t *testing.T, input string) charonconfig.ByteSize { t.Helper() - var b config.ByteSize + var b charonconfig.ByteSize require.NoError(t, json.Unmarshal([]byte(input), &b)) return b } @@ -23,27 +23,20 @@ func TestByteSizeUnmarshal(t *testing.T) { input string want int64 }{ - // bare integer {"0", 0}, {"1024", 1024}, {"1048576", 1048576}, - // string: no unit → bytes {`"0"`, 0}, {`"1024"`, 1024}, - // B suffix {`"512B"`, 512}, {`"512b"`, 512}, - // KB {`"1KB"`, 1024}, {`"2kb"`, 2048}, {`"1 KB"`, 1024}, - // MB {`"10MB"`, 10 * 1024 * 1024}, {`"10mb"`, 10 * 1024 * 1024}, - // GB {`"2GB"`, 2 * 1024 * 1024 * 1024}, {`"2gb"`, 2 * 1024 * 1024 * 1024}, - // mixed case {`"1Mb"`, 1024 * 1024}, {`"1gB"`, 1024 * 1024 * 1024}, } @@ -51,7 +44,7 @@ func TestByteSizeUnmarshal(t *testing.T) { for _, tc := range tests { t.Run(tc.input, func(t *testing.T) { got := unmarshalByteSize(t, tc.input) - assert.Equal(t, config.ByteSize(tc.want), got) + assert.Equal(t, charonconfig.ByteSize(tc.want), got) }) } } @@ -70,7 +63,7 @@ func TestByteSizeUnmarshalErrors(t *testing.T) { for _, tc := range tests { t.Run(tc.input, func(t *testing.T) { - var b config.ByteSize + var b charonconfig.ByteSize err := json.Unmarshal([]byte(tc.input), &b) require.Error(t, err) assert.Contains(t, err.Error(), tc.errFrag) @@ -79,10 +72,10 @@ func TestByteSizeUnmarshalErrors(t *testing.T) { } func TestByteSizeInConfig(t *testing.T) { - opts := config.NewCharonOptions() + opts := charonconfig.NewCharonOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/bytesize.yaml"})) require.NoError(t, opts.Complete(fs)) - assert.Equal(t, config.ByteSize(10*1024*1024), opts.MaxPayload) + assert.Equal(t, charonconfig.ByteSize(10*1024*1024), opts.MaxPayload) } diff --git a/internal/charonconfig/charonconfig.go b/internal/charonconfig/charonconfig.go new file mode 100644 index 0000000..72523e0 --- /dev/null +++ b/internal/charonconfig/charonconfig.go @@ -0,0 +1,189 @@ +package charonconfig + +import ( + "encoding/json" + "flag" + "fmt" + "os" + "time" + + "sigs.k8s.io/yaml" +) + +// ByteSize is an int64 that unmarshals from either a plain integer (bytes) or +// a string with an optional unit suffix: B, KB, MB, GB. K=1024. +type ByteSize = byteSizeType + +// TelemetryOptions holds OpenTelemetry settings. +type TelemetryOptions struct { + ExporterURL string // OTLP HTTP endpoint; empty = disabled + ServiceName string // identifies this binary in traces +} + +// AddFlags registers the --telemetry-exporter-url flag on fs. +func (o *TelemetryOptions) AddFlags(fs *flag.FlagSet) { + fs.StringVar(&o.ExporterURL, "telemetry-exporter-url", o.ExporterURL, "OTLP HTTP exporter URL (e.g. http://localhost:4318); empty disables tracing") +} + +// CharonOptions holds configuration for the Charon response-storage server. +type CharonOptions struct { + // Config file path — set by --config flag. + ConfigFile string + + // Listen address for the Charon internal API. + Listen string + + // Storage settings. + DataDir string + // TTLDays is the maximum age of a stored response. + TTLDays int + MaxResponses int64 + MaxPayload ByteSize + MaxChainDepth int + MaxContextBytes ByteSize + + // WorkerTTLInterval is how often the background TTL reaper runs (not the TTL itself). + WorkerTTLInterval time.Duration + + Telemetry TelemetryOptions +} + +// NewCharonOptions returns a CharonOptions pre-filled with built-in defaults. +func NewCharonOptions() *CharonOptions { + return &CharonOptions{ + Listen: ":8081", + DataDir: "./data", + TTLDays: 30, + WorkerTTLInterval: time.Hour, + Telemetry: TelemetryOptions{ServiceName: "charon"}, + } +} + +// AddFlags registers CLI flags on fs for the Charon server. +func (o *CharonOptions) AddFlags(fs *flag.FlagSet) { + fs.StringVar(&o.ConfigFile, "config", "", "path to config file") + fs.StringVar(&o.Listen, "listen", o.Listen, "charon internal API listen address") + fs.StringVar(&o.DataDir, "storage-data-dir", o.DataDir, "data directory for Pebble storage") + o.Telemetry.AddFlags(fs) +} + +// Complete loads the config file (if --config was set) and merges file values +// into the options struct. CLI flags take precedence over config-file values. +func (o *CharonOptions) Complete(fs *flag.FlagSet) error { + if o.ConfigFile == "" { + return nil + } + + fc, err := loadFileConfig(o.ConfigFile) + if err != nil { + return err + } + + setFlags := visitedFlags(fs) + + if !setFlags["listen"] { + o.Listen = fc.Charon.Listen + } + if !setFlags["storage-data-dir"] { + o.DataDir = fc.Charon.Storage.DataDir + } + + // Storage fields are config-file-only. + o.TTLDays = fc.Charon.Storage.TTLDays + o.MaxResponses = fc.Charon.Storage.MaxResponses + o.MaxPayload = fc.Charon.Storage.MaxPayload + o.MaxChainDepth = fc.Charon.Storage.MaxChainDepth + o.MaxContextBytes = fc.Charon.Storage.MaxContextBytes + + // Worker settings are config-file-only. + o.WorkerTTLInterval = fc.Charon.Workers.TTLInterval + + if !setFlags["telemetry-exporter-url"] { + o.Telemetry.ExporterURL = fc.Telemetry.ExporterURL + } + o.Telemetry.ServiceName = fc.Telemetry.CharonService + + return nil +} + +// Validate checks CharonOptions for invalid combinations. It performs no I/O. +func (o *CharonOptions) Validate() error { + if o.DataDir == "" { + return fmt.Errorf("charon storage data-dir is empty") + } + return nil +} + +// --------------------------------------------------------------------------- +// private YAML loader +// --------------------------------------------------------------------------- + +type fileConfig struct { + Proxy json.RawMessage `json:"proxy"` // accepted but unused — avoids strict-parse rejection of shared config files + Charon fileCharonConfig `json:"charon"` + Telemetry fileTelemetryConfig `json:"telemetry"` +} + +type fileCharonConfig struct { + Listen string `json:"listen"` + Storage fileStorageConfig `json:"storage"` + Workers fileWorkerConfig `json:"workers"` +} + +type fileStorageConfig struct { + DataDir string `json:"data_dir"` + TTLDays int `json:"ttl_days"` + MaxResponses int64 `json:"max_responses"` + MaxPayload byteSizeType `json:"max_payload"` + MaxChainDepth int `json:"max_chain_depth"` + MaxContextBytes byteSizeType `json:"max_context_bytes"` +} + +type fileWorkerConfig struct { + TTLInterval time.Duration `json:"ttl_interval"` +} + +type fileTelemetryConfig struct { + ExporterURL string `json:"exporter_url"` + CharonService string `json:"charon_service"` + ProxyService string `json:"proxy_service"` // accepted but unused — avoids strict-parse rejection of shared config files +} + +func applyFileDefaults(fc *fileConfig) { + if fc.Charon.Listen == "" { + fc.Charon.Listen = ":8081" + } + if fc.Charon.Storage.DataDir == "" { + fc.Charon.Storage.DataDir = "./data" + } + if fc.Charon.Storage.TTLDays <= 0 { + fc.Charon.Storage.TTLDays = 30 + } + if fc.Charon.Workers.TTLInterval <= 0 { + fc.Charon.Workers.TTLInterval = time.Hour + } + if fc.Telemetry.CharonService == "" { + fc.Telemetry.CharonService = "charon" + } +} + +func loadFileConfig(path string) (fileConfig, error) { + var fc fileConfig + applyFileDefaults(&fc) + + data, err := os.ReadFile(path) + if err != nil { + return fc, fmt.Errorf("read config: %w", err) + } + if err := yaml.UnmarshalStrict(data, &fc); err != nil { + return fc, fmt.Errorf("parse config: %w", err) + } + applyFileDefaults(&fc) + return fc, nil +} + +func visitedFlags(fs *flag.FlagSet) map[string]bool { + m := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { m[f.Name] = true }) + return m +} diff --git a/internal/charonconfig/charonconfig_test.go b/internal/charonconfig/charonconfig_test.go new file mode 100644 index 0000000..d1e71bd --- /dev/null +++ b/internal/charonconfig/charonconfig_test.go @@ -0,0 +1,102 @@ +package charonconfig_test + +import ( + "flag" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/elevran/charon/internal/charonconfig" +) + +func TestCharonOptionsDefaultsWithNoConfig(t *testing.T) { + opts := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse(nil)) + require.NoError(t, opts.Complete(fs)) + + assert.Equal(t, ":8081", opts.Listen) + assert.Equal(t, "./data", opts.DataDir) + assert.Equal(t, 30, opts.TTLDays) + assert.Equal(t, time.Hour, opts.WorkerTTLInterval) + assert.Equal(t, "charon", opts.Telemetry.ServiceName) +} + +func TestCharonOptionsCompleteLoadsFile(t *testing.T) { + opts := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, opts.Complete(fs)) + + assert.Equal(t, ":9090", opts.Listen) + assert.Equal(t, "./data", opts.DataDir) +} + +func TestCharonOptionsCLIOverridesFile(t *testing.T) { + opts := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":7777"})) + require.NoError(t, opts.Complete(fs)) + + assert.Equal(t, ":7777", opts.Listen, "CLI flag must take precedence over config file") +} + +func TestCharonOptionsValidateOK(t *testing.T) { + opts := charonconfig.NewCharonOptions() + require.NoError(t, opts.Validate()) +} + +func TestCharonOptionsDataDirCLIOverridesFile(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "cfg.yaml") + err := os.WriteFile(cfgPath, []byte("charon:\n storage:\n data_dir: /tmp/test-data\n"), 0600) + require.NoError(t, err) + + opts := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--storage-data-dir", "./my-data"})) + require.NoError(t, opts.Complete(fs)) + + assert.Equal(t, "./my-data", opts.DataDir) +} + +func TestLoadDefaults(t *testing.T) { + charon := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + charon.AddFlags(fs) + require.NoError(t, fs.Parse(nil)) + require.NoError(t, charon.Complete(fs)) + + assert.Equal(t, ":8081", charon.Listen) + assert.Equal(t, "./data", charon.DataDir) + assert.Equal(t, 30, charon.TTLDays) + assert.Equal(t, time.Hour, charon.WorkerTTLInterval) + assert.Equal(t, "charon", charon.Telemetry.ServiceName) +} + +func TestLoadFromFile(t *testing.T) { + charon := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + charon.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, charon.Complete(fs)) + assert.Equal(t, 30, charon.TTLDays) +} + +func TestLoadStrictRejectsUnknownFields(t *testing.T) { + charon := charonconfig.NewCharonOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + charon.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", "testdata/invalid.yaml"})) + err := charon.Complete(fs) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse config") +} diff --git a/internal/config/testdata/bytesize.yaml b/internal/charonconfig/testdata/bytesize.yaml similarity index 100% rename from internal/config/testdata/bytesize.yaml rename to internal/charonconfig/testdata/bytesize.yaml diff --git a/internal/config/testdata/config.yaml b/internal/charonconfig/testdata/config.yaml similarity index 100% rename from internal/config/testdata/config.yaml rename to internal/charonconfig/testdata/config.yaml diff --git a/internal/config/testdata/invalid.yaml b/internal/charonconfig/testdata/invalid.yaml similarity index 100% rename from internal/config/testdata/invalid.yaml rename to internal/charonconfig/testdata/invalid.yaml diff --git a/internal/config/config.go b/internal/config/config.go deleted file mode 100644 index fb7e428..0000000 --- a/internal/config/config.go +++ /dev/null @@ -1,17 +0,0 @@ -package config - -import "net" - -// deriveCharonURL returns an HTTP URL for the Charon internal API from its -// listen address. Wildcard hosts (empty, "0.0.0.0", "::") are replaced with -// "127.0.0.1" so the proxy connects to localhost. -func deriveCharonURL(charonListen string) string { - host, port, err := net.SplitHostPort(charonListen) - if err != nil { - return "http://127.0.0.1:8081" - } - if host == "" || host == "0.0.0.0" || host == "::" { - host = "127.0.0.1" - } - return "http://" + net.JoinHostPort(host, port) -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go deleted file mode 100644 index 8e192ca..0000000 --- a/internal/config/config_test.go +++ /dev/null @@ -1,75 +0,0 @@ -package config_test - -import ( - "flag" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/elevran/charon/internal/config" -) - -func TestLoadDefaults(t *testing.T) { - charon := config.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - charon.AddFlags(fs) - require.NoError(t, fs.Parse(nil)) - require.NoError(t, charon.Complete(fs)) - - assert.Equal(t, ":8081", charon.Listen) - assert.Equal(t, "./data", charon.DataDir) - assert.Equal(t, 30, charon.TTLDays) - assert.Equal(t, time.Hour, charon.WorkerTTLInterval) - assert.Equal(t, "charon", charon.Telemetry.ServiceName) - - proxy := config.NewProxyOptions() - fs2 := flag.NewFlagSet("test", flag.ContinueOnError) - proxy.AddFlags(fs2) - require.NoError(t, fs2.Parse(nil)) - require.NoError(t, proxy.Complete(fs2)) - - assert.Equal(t, ":8080", proxy.Listen) - assert.Equal(t, "http://localhost:11434", proxy.Backend) - assert.Equal(t, "http://127.0.0.1:8081", proxy.CharonURL) - assert.Equal(t, 120, proxy.TimeoutSeconds) - assert.Equal(t, "charon-proxy", proxy.Telemetry.ServiceName) -} - -func TestCharonURLDerivedFromListen(t *testing.T) { - // When charon.listen is set in the file and proxy.charon_url is not, - // CharonURL is auto-derived from the file's charon.listen. - proxy := config.NewProxyOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - proxy.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) - require.NoError(t, proxy.Complete(fs)) - assert.Equal(t, "http://127.0.0.1:9090", proxy.CharonURL) -} - -func TestLoadFromFile(t *testing.T) { - proxy := config.NewProxyOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - proxy.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) - require.NoError(t, proxy.Complete(fs)) - assert.Equal(t, ":0", proxy.Listen) - - charon := config.NewCharonOptions() - fs2 := flag.NewFlagSet("test", flag.ContinueOnError) - charon.AddFlags(fs2) - require.NoError(t, fs2.Parse([]string{"--config", "testdata/config.yaml"})) - require.NoError(t, charon.Complete(fs2)) - assert.Equal(t, 30, charon.TTLDays) -} - -func TestLoadStrictRejectsUnknownFields(t *testing.T) { - charon := config.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - charon.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/invalid.yaml"})) - err := charon.Complete(fs) - require.Error(t, err) - assert.Contains(t, err.Error(), "parse config") -} diff --git a/internal/config/options.go b/internal/config/options.go deleted file mode 100644 index 591147f..0000000 --- a/internal/config/options.go +++ /dev/null @@ -1,314 +0,0 @@ -package config - -import ( - "flag" - "fmt" - "os" - "time" - - "sigs.k8s.io/yaml" -) - -// fileConfig is a private type used only to parse YAML config files. -// It mirrors the YAML structure so existing config files continue to work. -type fileConfig struct { - Proxy fileProxyConfig `json:"proxy"` - Charon fileCharonConfig `json:"charon"` - Telemetry fileTelemetryConfig `json:"telemetry"` -} - -type fileProxyConfig struct { - Listen string `json:"listen"` - CharonURL string `json:"charon_url"` - Inference fileInferenceConfig `json:"inference"` -} - -type fileInferenceConfig struct { - BaseURL string `json:"base_url"` - APIKey string `json:"api_key"` - TimeoutSeconds int `json:"timeout_seconds"` -} - -type fileCharonConfig struct { - Listen string `json:"listen"` - Storage fileStorageConfig `json:"storage"` - Workers fileWorkerConfig `json:"workers"` -} - -type fileStorageConfig struct { - DataDir string `json:"data_dir"` - TTLDays int `json:"ttl_days"` - MaxResponses int64 `json:"max_responses"` - MaxPayload ByteSize `json:"max_payload"` - MaxChainDepth int `json:"max_chain_depth"` - MaxContextBytes ByteSize `json:"max_context_bytes"` -} - -type fileWorkerConfig struct { - // TTLInterval is how often the background reaper runs, not the TTL itself. - // Maps to chainstore.Config.TTLInterval. - TTLInterval time.Duration `json:"ttl_interval"` -} - -type fileTelemetryConfig struct { - ExporterURL string `json:"exporter_url"` - CharonService string `json:"charon_service"` - ProxyService string `json:"proxy_service"` -} - -// applyFileDefaults fills in zero-valued fields in fc with built-in defaults. -// Called twice: before and after YAML unmarshalling so that missing file fields -// stay at their defaults. -func applyFileDefaults(fc *fileConfig) { - if fc.Proxy.Listen == "" { - fc.Proxy.Listen = ":8080" - } - if fc.Proxy.Inference.BaseURL == "" { - fc.Proxy.Inference.BaseURL = "http://localhost:11434" - } - if fc.Proxy.Inference.TimeoutSeconds <= 0 { - fc.Proxy.Inference.TimeoutSeconds = 120 - } - if fc.Charon.Listen == "" { - fc.Charon.Listen = ":8081" - } - if fc.Charon.Storage.DataDir == "" { - fc.Charon.Storage.DataDir = "./data" - } - if fc.Charon.Storage.TTLDays <= 0 { - fc.Charon.Storage.TTLDays = 30 - } - if fc.Charon.Workers.TTLInterval <= 0 { - fc.Charon.Workers.TTLInterval = time.Hour - } - if fc.Telemetry.CharonService == "" { - fc.Telemetry.CharonService = "charon" - } - if fc.Telemetry.ProxyService == "" { - fc.Telemetry.ProxyService = "charon-proxy" - } -} - -// loadFileConfig reads and parses a YAML config file, applying defaults. -func loadFileConfig(path string) (fileConfig, error) { - var fc fileConfig - applyFileDefaults(&fc) - - data, err := os.ReadFile(path) - if err != nil { - return fc, fmt.Errorf("read config: %w", err) - } - if err := yaml.UnmarshalStrict(data, &fc); err != nil { - return fc, fmt.Errorf("parse config: %w", err) - } - // Second pass of defaults to fill any gaps after file parse. - applyFileDefaults(&fc) - return fc, nil -} - -// --------------------------------------------------------------------------- -// TelemetryOptions (shared) -// --------------------------------------------------------------------------- - -// TelemetryOptions holds OpenTelemetry settings. -type TelemetryOptions struct { - ExporterURL string // OTLP HTTP endpoint; empty = disabled - ServiceName string // identifies this binary in traces -} - -// AddFlags registers the --telemetry-exporter-url flag on fs. -func (o *TelemetryOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&o.ExporterURL, "telemetry-exporter-url", o.ExporterURL, "OTLP HTTP exporter URL (e.g. http://localhost:4318); empty disables tracing") -} - -// visitedFlags returns a set of flag names that were explicitly set on fs. -func visitedFlags(fs *flag.FlagSet) map[string]bool { - m := make(map[string]bool) - fs.Visit(func(f *flag.Flag) { m[f.Name] = true }) - return m -} - -// --------------------------------------------------------------------------- -// CharonOptions -// --------------------------------------------------------------------------- - -// CharonOptions holds configuration for the Charon response-storage server. -type CharonOptions struct { - // Config file path — set by --config flag. - ConfigFile string - - // Listen address for the Charon internal API. - Listen string - - // Storage settings. - DataDir string - // TTLDays is the maximum age of a stored response. Maps to chainstore.Config.TTL. - TTLDays int - MaxResponses int64 - MaxPayload ByteSize - MaxChainDepth int - MaxContextBytes ByteSize - - // WorkerTTLInterval is how often the background TTL reaper runs (not the TTL itself). - WorkerTTLInterval time.Duration - - Telemetry TelemetryOptions -} - -// NewCharonOptions returns a CharonOptions pre-filled with built-in defaults. -func NewCharonOptions() *CharonOptions { - return &CharonOptions{ - Listen: ":8081", - DataDir: "./data", - TTLDays: 30, - WorkerTTLInterval: time.Hour, - Telemetry: TelemetryOptions{ServiceName: "charon"}, - } -} - -// AddFlags registers CLI flags on fs for the Charon server. -func (o *CharonOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&o.ConfigFile, "config", "", "path to config file") - fs.StringVar(&o.Listen, "listen", o.Listen, "charon internal API listen address") - fs.StringVar(&o.DataDir, "storage-data-dir", o.DataDir, "data directory for Pebble storage") - o.Telemetry.AddFlags(fs) -} - -// Complete loads the config file (if --config was set) and merges file values -// into the options struct. CLI flags take precedence over config-file values. -func (o *CharonOptions) Complete(fs *flag.FlagSet) error { - if o.ConfigFile == "" { - return nil - } - - fc, err := loadFileConfig(o.ConfigFile) - if err != nil { - return err - } - - setFlags := visitedFlags(fs) - - if !setFlags["listen"] { - o.Listen = fc.Charon.Listen - } - if !setFlags["storage-data-dir"] { - o.DataDir = fc.Charon.Storage.DataDir - } - - // Storage fields are config-file-only. - o.TTLDays = fc.Charon.Storage.TTLDays - o.MaxResponses = fc.Charon.Storage.MaxResponses - o.MaxPayload = fc.Charon.Storage.MaxPayload - o.MaxChainDepth = fc.Charon.Storage.MaxChainDepth - o.MaxContextBytes = fc.Charon.Storage.MaxContextBytes - - // Worker settings are config-file-only. - o.WorkerTTLInterval = fc.Charon.Workers.TTLInterval - - if !setFlags["telemetry-exporter-url"] { - o.Telemetry.ExporterURL = fc.Telemetry.ExporterURL - } - o.Telemetry.ServiceName = fc.Telemetry.CharonService - - return nil -} - -// Validate checks CharonOptions for invalid combinations. It performs no I/O. -func (o *CharonOptions) Validate() error { - if o.DataDir == "" { - return fmt.Errorf("charon storage data-dir is empty") - } - return nil -} - -// --------------------------------------------------------------------------- -// ProxyOptions -// --------------------------------------------------------------------------- - -// ProxyOptions holds configuration for the proxy server. -type ProxyOptions struct { - // Config file path — set by --config flag. - ConfigFile string - - // Listen address for the proxy HTTP server. - Listen string - - // Inference backend settings. - Backend string // base URL - APIKey string - TimeoutSeconds int - - // CharonURL is the Charon internal API endpoint the proxy calls. - // Auto-derived from config file proxy.charon_url or Charon's listen address. - CharonURL string - - Telemetry TelemetryOptions -} - -// NewProxyOptions returns a ProxyOptions pre-filled with built-in defaults. -func NewProxyOptions() *ProxyOptions { - return &ProxyOptions{ - Listen: ":8080", - Backend: "http://localhost:11434", - TimeoutSeconds: 120, - CharonURL: "http://127.0.0.1:8081", - Telemetry: TelemetryOptions{ServiceName: "charon-proxy"}, - } -} - -// AddFlags registers CLI flags on fs for the proxy server. -func (o *ProxyOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&o.ConfigFile, "config", "", "path to config file") - fs.StringVar(&o.Listen, "listen", o.Listen, "proxy server listen address") - fs.StringVar(&o.Backend, "backend", o.Backend, "inference backend base URL") - fs.StringVar(&o.CharonURL, "charon-url", o.CharonURL, "charon internal API base URL") - o.Telemetry.AddFlags(fs) -} - -// Complete loads the config file (if --config was set) and merges file values -// into the options struct. CLI flags take precedence over config-file values. -func (o *ProxyOptions) Complete(fs *flag.FlagSet) error { - if o.ConfigFile == "" { - return nil - } - - fc, err := loadFileConfig(o.ConfigFile) - if err != nil { - return err - } - - setFlags := visitedFlags(fs) - - if !setFlags["listen"] { - o.Listen = fc.Proxy.Listen - } - if !setFlags["backend"] { - o.Backend = fc.Proxy.Inference.BaseURL - } - // API key and timeout are config-file-only. - o.APIKey = fc.Proxy.Inference.APIKey - o.TimeoutSeconds = fc.Proxy.Inference.TimeoutSeconds - - if !setFlags["charon-url"] { - if fc.Proxy.CharonURL != "" { - o.CharonURL = fc.Proxy.CharonURL - } else { - o.CharonURL = deriveCharonURL(fc.Charon.Listen) - } - } - - if !setFlags["telemetry-exporter-url"] { - o.Telemetry.ExporterURL = fc.Telemetry.ExporterURL - } - o.Telemetry.ServiceName = fc.Telemetry.ProxyService - - return nil -} - -// Validate checks ProxyOptions for invalid combinations. It performs no I/O. -func (o *ProxyOptions) Validate() error { - if o.Backend == "" { - return fmt.Errorf("proxy backend (inference base URL) is empty") - } - return nil -} diff --git a/internal/httputil/http.go b/internal/httputil/http.go deleted file mode 100644 index 966db90..0000000 --- a/internal/httputil/http.go +++ /dev/null @@ -1,18 +0,0 @@ -package httputil - -import ( - "encoding/json" - "net/http" -) - -// WriteJSON writes a JSON response with the given status code. -func WriteJSON(w http.ResponseWriter, status int, v any) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(v) -} - -// WriteError writes a JSON error response. -func WriteError(w http.ResponseWriter, status int, msg string) { - WriteJSON(w, status, map[string]string{"error": msg}) -} diff --git a/internal/proxyconfig/proxyconfig.go b/internal/proxyconfig/proxyconfig.go new file mode 100644 index 0000000..f3d4db2 --- /dev/null +++ b/internal/proxyconfig/proxyconfig.go @@ -0,0 +1,209 @@ +package proxyconfig + +import ( + "encoding/json" + "flag" + "fmt" + "net" + "os" + + "sigs.k8s.io/yaml" +) + +// TelemetryOptions holds OpenTelemetry settings. +type TelemetryOptions struct { + ExporterURL string // OTLP HTTP endpoint; empty = disabled + ServiceName string // identifies this binary in traces +} + +// AddFlags registers the --telemetry-exporter-url flag on fs. +func (o *TelemetryOptions) AddFlags(fs *flag.FlagSet) { + fs.StringVar(&o.ExporterURL, "telemetry-exporter-url", o.ExporterURL, "OTLP HTTP exporter URL (e.g. http://localhost:4318); empty disables tracing") +} + +// ProxyOptions holds configuration for the proxy server. +type ProxyOptions struct { + // Config file path — set by --config flag. + ConfigFile string + + // Listen address for the proxy HTTP server. + Listen string + + // Inference backend settings. + Backend string // base URL + APIKey string + TimeoutSeconds int + + // CharonURL is the Charon internal API endpoint the proxy calls. + // Auto-derived from config file proxy.charon_url or Charon's listen address. + CharonURL string + + Telemetry TelemetryOptions +} + +// NewProxyOptions returns a ProxyOptions pre-filled with built-in defaults. +func NewProxyOptions() *ProxyOptions { + return &ProxyOptions{ + Listen: ":8080", + Backend: "http://localhost:11434", + TimeoutSeconds: 120, + CharonURL: "http://127.0.0.1:8081", + Telemetry: TelemetryOptions{ServiceName: "charon-proxy"}, + } +} + +// AddFlags registers CLI flags on fs for the proxy server. +func (o *ProxyOptions) AddFlags(fs *flag.FlagSet) { + fs.StringVar(&o.ConfigFile, "config", "", "path to config file") + fs.StringVar(&o.Listen, "listen", o.Listen, "proxy server listen address") + fs.StringVar(&o.Backend, "backend", o.Backend, "inference backend base URL") + fs.StringVar(&o.CharonURL, "charon-url", o.CharonURL, "charon internal API base URL") + o.Telemetry.AddFlags(fs) +} + +// Complete loads the config file (if --config was set) and merges file values +// into the options struct. CLI flags take precedence over config-file values. +func (o *ProxyOptions) Complete(fs *flag.FlagSet) error { + if o.ConfigFile == "" { + return nil + } + + fc, err := loadFileConfig(o.ConfigFile) + if err != nil { + return err + } + + setFlags := visitedFlags(fs) + + if !setFlags["listen"] { + o.Listen = fc.Proxy.Listen + } + if !setFlags["backend"] { + o.Backend = fc.Proxy.Inference.BaseURL + } + // API key and timeout are config-file-only. + o.APIKey = fc.Proxy.Inference.APIKey + o.TimeoutSeconds = fc.Proxy.Inference.TimeoutSeconds + + if !setFlags["charon-url"] { + if fc.Proxy.CharonURL != "" { + o.CharonURL = fc.Proxy.CharonURL + } else { + o.CharonURL = deriveCharonURL(fc.CharonListen) + } + } + + if !setFlags["telemetry-exporter-url"] { + o.Telemetry.ExporterURL = fc.Telemetry.ExporterURL + } + o.Telemetry.ServiceName = fc.Telemetry.ProxyService + + return nil +} + +// Validate checks ProxyOptions for invalid combinations. It performs no I/O. +func (o *ProxyOptions) Validate() error { + if o.Backend == "" { + return fmt.Errorf("proxy backend (inference base URL) is empty") + } + return nil +} + +// deriveCharonURL returns an HTTP URL for the Charon internal API from its +// listen address. Wildcard hosts (empty, "0.0.0.0", "::") are replaced with +// "127.0.0.1" so the proxy connects to localhost. +func deriveCharonURL(charonListen string) string { + host, port, err := net.SplitHostPort(charonListen) + if err != nil { + return "http://127.0.0.1:8081" + } + if host == "" || host == "0.0.0.0" || host == "::" { + host = "127.0.0.1" + } + return "http://" + net.JoinHostPort(host, port) +} + +// --------------------------------------------------------------------------- +// private YAML loader +// --------------------------------------------------------------------------- + +type fileConfig struct { + Proxy fileProxyConfig `json:"proxy"` + CharonListen string `json:"_charon_listen"` // populated from charon.listen after parse + Telemetry fileTelemetryConfig `json:"telemetry"` +} + +// fileConfigRaw is the raw YAML shape; we flatten charon.listen into fileConfig. +type fileConfigRaw struct { + Proxy fileProxyConfig `json:"proxy"` + Charon fileCharonSection `json:"charon"` + Telemetry fileTelemetryConfig `json:"telemetry"` +} + +type fileCharonSection struct { + Listen string `json:"listen"` + Storage json.RawMessage `json:"storage"` + Workers json.RawMessage `json:"workers"` +} + +type fileProxyConfig struct { + Listen string `json:"listen"` + CharonURL string `json:"charon_url"` + Inference fileInferenceConfig `json:"inference"` +} + +type fileInferenceConfig struct { + BaseURL string `json:"base_url"` + APIKey string `json:"api_key"` + TimeoutSeconds int `json:"timeout_seconds"` +} + +type fileTelemetryConfig struct { + ExporterURL string `json:"exporter_url"` + ProxyService string `json:"proxy_service"` + CharonService string `json:"charon_service"` // accepted but unused — avoids strict-parse rejection of shared config files +} + +func applyFileDefaults(raw *fileConfigRaw) { + if raw.Proxy.Listen == "" { + raw.Proxy.Listen = ":8080" + } + if raw.Proxy.Inference.BaseURL == "" { + raw.Proxy.Inference.BaseURL = "http://localhost:11434" + } + if raw.Proxy.Inference.TimeoutSeconds <= 0 { + raw.Proxy.Inference.TimeoutSeconds = 120 + } + if raw.Charon.Listen == "" { + raw.Charon.Listen = ":8081" + } + if raw.Telemetry.ProxyService == "" { + raw.Telemetry.ProxyService = "charon-proxy" + } +} + +func loadFileConfig(path string) (fileConfig, error) { + var raw fileConfigRaw + applyFileDefaults(&raw) + + data, err := os.ReadFile(path) + if err != nil { + return fileConfig{}, fmt.Errorf("read config: %w", err) + } + if err := yaml.UnmarshalStrict(data, &raw); err != nil { + return fileConfig{}, fmt.Errorf("parse config: %w", err) + } + applyFileDefaults(&raw) + + return fileConfig{ + Proxy: raw.Proxy, + CharonListen: raw.Charon.Listen, + Telemetry: raw.Telemetry, + }, nil +} + +func visitedFlags(fs *flag.FlagSet) map[string]bool { + m := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { m[f.Name] = true }) + return m +} diff --git a/internal/config/options_test.go b/internal/proxyconfig/proxyconfig_test.go similarity index 58% rename from internal/config/options_test.go rename to internal/proxyconfig/proxyconfig_test.go index 53a4775..144359b 100644 --- a/internal/config/options_test.go +++ b/internal/proxyconfig/proxyconfig_test.go @@ -1,83 +1,19 @@ -package config_test +package proxyconfig_test import ( "flag" "os" "path/filepath" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/elevran/charon/internal/config" + "github.com/elevran/charon/internal/proxyconfig" ) -// --------------------------------------------------------------------------- -// CharonOptions tests -// --------------------------------------------------------------------------- - -func TestCharonOptionsDefaultsWithNoConfig(t *testing.T) { - opts := config.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - opts.AddFlags(fs) - require.NoError(t, fs.Parse(nil)) - require.NoError(t, opts.Complete(fs)) - - assert.Equal(t, ":8081", opts.Listen) - assert.Equal(t, "./data", opts.DataDir) - assert.Equal(t, 30, opts.TTLDays) - assert.Equal(t, time.Hour, opts.WorkerTTLInterval) - assert.Equal(t, "charon", opts.Telemetry.ServiceName) -} - -func TestCharonOptionsCompleteLoadsFile(t *testing.T) { - opts := config.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) - require.NoError(t, opts.Complete(fs)) - - assert.Equal(t, ":9090", opts.Listen) - assert.Equal(t, "./data", opts.DataDir) -} - -func TestCharonOptionsCLIOverridesFile(t *testing.T) { - opts := config.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":7777"})) - require.NoError(t, opts.Complete(fs)) - - assert.Equal(t, ":7777", opts.Listen, "CLI flag must take precedence over config file") -} - -func TestCharonOptionsValidateOK(t *testing.T) { - opts := config.NewCharonOptions() - require.NoError(t, opts.Validate()) -} - -func TestCharonOptionsDataDirCLIOverridesFile(t *testing.T) { - dir := t.TempDir() - cfgPath := filepath.Join(dir, "cfg.yaml") - err := os.WriteFile(cfgPath, []byte("charon:\n storage:\n data_dir: /tmp/test-data\n"), 0600) - require.NoError(t, err) - - opts := config.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--storage-data-dir", "./my-data"})) - require.NoError(t, opts.Complete(fs)) - - assert.Equal(t, "./my-data", opts.DataDir) -} - -// --------------------------------------------------------------------------- -// ProxyOptions tests -// --------------------------------------------------------------------------- - func TestProxyOptionsDefaultsWithNoConfig(t *testing.T) { - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse(nil)) @@ -91,7 +27,7 @@ func TestProxyOptionsDefaultsWithNoConfig(t *testing.T) { } func TestProxyOptionsCompleteLoadsFile(t *testing.T) { - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) @@ -103,7 +39,7 @@ func TestProxyOptionsCompleteLoadsFile(t *testing.T) { } func TestProxyOptionsCLIOverridesFile(t *testing.T) { - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":5555"})) @@ -118,7 +54,7 @@ func TestProxyOptionsBackendCLIOverridesFile(t *testing.T) { err := os.WriteFile(cfgPath, []byte("proxy:\n inference:\n base_url: http://file-backend:11434\n"), 0600) require.NoError(t, err) - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--backend", "http://cli-backend:11434"})) @@ -133,7 +69,7 @@ func TestProxyOptionsCharonURLCLIOverridesFile(t *testing.T) { err := os.WriteFile(cfgPath, []byte("proxy:\n charon_url: http://file-charon:8081\n"), 0600) require.NoError(t, err) - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--charon-url", "http://cli-charon:9999"})) @@ -143,14 +79,32 @@ func TestProxyOptionsCharonURLCLIOverridesFile(t *testing.T) { } func TestProxyOptionsValidateOK(t *testing.T) { - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() require.NoError(t, opts.Validate()) } func TestProxyOptionsValidateEmptyBackendFails(t *testing.T) { - opts := config.NewProxyOptions() + opts := proxyconfig.NewProxyOptions() opts.Backend = "" err := opts.Validate() require.Error(t, err) assert.Contains(t, err.Error(), "backend") } + +func TestCharonURLDerivedFromListen(t *testing.T) { + opts := proxyconfig.NewProxyOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, opts.Complete(fs)) + assert.Equal(t, "http://127.0.0.1:9090", opts.CharonURL) +} + +func TestLoadFromFile(t *testing.T) { + proxy := proxyconfig.NewProxyOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + proxy.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, proxy.Complete(fs)) + assert.Equal(t, ":0", proxy.Listen) +} diff --git a/internal/proxyconfig/testdata/bytesize.yaml b/internal/proxyconfig/testdata/bytesize.yaml new file mode 100644 index 0000000..ae0709a --- /dev/null +++ b/internal/proxyconfig/testdata/bytesize.yaml @@ -0,0 +1,3 @@ +charon: + storage: + max_payload: "10MB" diff --git a/internal/proxyconfig/testdata/config.yaml b/internal/proxyconfig/testdata/config.yaml new file mode 100644 index 0000000..1b36aa8 --- /dev/null +++ b/internal/proxyconfig/testdata/config.yaml @@ -0,0 +1,6 @@ +proxy: + listen: ":0" +charon: + listen: ":9090" + storage: + data_dir: ./data diff --git a/internal/proxyconfig/testdata/invalid.yaml b/internal/proxyconfig/testdata/invalid.yaml new file mode 100644 index 0000000..da10a6c --- /dev/null +++ b/internal/proxyconfig/testdata/invalid.yaml @@ -0,0 +1,3 @@ +server: + listen: ":8080" + unknown_field: true diff --git a/internal/server/handlers.go b/internal/server/handlers.go index f960244..0c00d96 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -11,7 +11,6 @@ import ( "github.com/google/uuid" "github.com/elevran/charon/internal/chainstore" - "github.com/elevran/charon/internal/httputil" ) // maxChunkBytes is the hard upper bound on a streaming-chunk PUT body. @@ -52,7 +51,7 @@ func (h *Handler) writeErr(w http.ResponseWriter, op, id string, err error) { if status == http.StatusInternalServerError { h.log.Error(op, "id", id, "err", err) } - httputil.WriteError(w, status, msg) + writeError(w, status, msg) } // blobToRaw converts a raw-bytes blob to json.RawMessage for the wire format. @@ -128,7 +127,7 @@ func (h *Handler) HandleBufferedStore(w http.ResponseWriter, r *http.Request) { } var req bufferedStoreRequest if err := json.Unmarshal(b, &req); err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid JSON body") + writeError(w, http.StatusBadRequest, "invalid JSON body") return } @@ -157,7 +156,7 @@ func (h *Handler) HandleBufferedStore(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Location", "/responses/"+responseID) - httputil.WriteJSON(w, http.StatusCreated, bufferedStoreResponse{ + writeJSON(w, http.StatusCreated, bufferedStoreResponse{ ResponseID: responseID, Turns: respTurns, }) @@ -178,7 +177,7 @@ func (h *Handler) HandleOpenStaging(w http.ResponseWriter, r *http.Request) { } requestBlob, err := io.ReadAll(r.Body) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "failed to read request body") + writeError(w, http.StatusBadRequest, "failed to read request body") return } _ = r.Body.Close() @@ -199,7 +198,7 @@ func (h *Handler) HandleOpenStaging(w http.ResponseWriter, r *http.Request) { w.Header().Set("Location", "/staging/"+stagingID) w.Header().Set("X-Staging-ID", stagingID) - httputil.WriteJSON(w, http.StatusCreated, resolveResponse{ + writeJSON(w, http.StatusCreated, resolveResponse{ StagingID: stagingID, Turns: respTurns, }) @@ -214,7 +213,7 @@ func (h *Handler) HandleOpenStaging(w http.ResponseWriter, r *http.Request) { func (h *Handler) HandleStagingStatus(w http.ResponseWriter, r *http.Request) { stagingID := r.PathValue("id") if len(stagingID) > 64 { - httputil.WriteError(w, http.StatusBadRequest, "invalid staging id") + writeError(w, http.StatusBadRequest, "invalid staging id") return } ctx := r.Context() @@ -265,7 +264,7 @@ func (h *Handler) readBodyOr400(w http.ResponseWriter, r *http.Request) (b []byt b, err = io.ReadAll(r.Body) _ = r.Body.Close() if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "failed to read request body") + writeError(w, http.StatusBadRequest, "failed to read request body") return nil, false } return b, true @@ -297,16 +296,16 @@ func (h *Handler) HandleAppendChunk(w http.ResponseWriter, r *http.Request) { responseID := r.URL.Query().Get("response_id") if len(stagingID) > 64 { - httputil.WriteError(w, http.StatusBadRequest, "invalid staging id") + writeError(w, http.StatusBadRequest, "invalid staging id") return } if responseID != "" && len(responseID) > 255 { - httputil.WriteError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") + writeError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") return } k64, err := strconv.ParseUint(kStr, 10, 32) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid chunk number") + writeError(w, http.StatusBadRequest, "invalid chunk number") return } k := uint32(k64) @@ -316,12 +315,12 @@ func (h *Handler) HandleAppendChunk(w http.ResponseWriter, r *http.Request) { r.Body = http.MaxBytesReader(w, r.Body, defaultMaxChunkBytes) chunkBody, err := io.ReadAll(r.Body) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "failed to read chunk body") + writeError(w, http.StatusBadRequest, "failed to read chunk body") return } _ = r.Body.Close() if len(chunkBody) == 0 { - httputil.WriteError(w, http.StatusBadRequest, "empty chunk body") + writeError(w, http.StatusBadRequest, "empty chunk body") return } @@ -347,7 +346,7 @@ func (h *Handler) HandleAppendChunk(w http.ResponseWriter, r *http.Request) { if k < nextExpected { status = http.StatusOK } - httputil.WriteJSON(w, status, map[string]uint32{ + writeJSON(w, status, map[string]uint32{ "received": k, "expected_next": nextExpected, }) @@ -365,20 +364,20 @@ func (h *Handler) HandleComplete(w http.ResponseWriter, r *http.Request) { tenantKey := r.Header.Get("X-Tenant-Key") if len(stagingID) > 64 { - httputil.WriteError(w, http.StatusBadRequest, "invalid staging id") + writeError(w, http.StatusBadRequest, "invalid staging id") return } if totalStr == "" { - httputil.WriteError(w, http.StatusBadRequest, "missing total") + writeError(w, http.StatusBadRequest, "missing total") return } total, err := strconv.ParseUint(totalStr, 10, 32) if err != nil { - httputil.WriteError(w, http.StatusBadRequest, "invalid total") + writeError(w, http.StatusBadRequest, "invalid total") return } if responseID != "" && len(responseID) > 255 { - httputil.WriteError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") + writeError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") return } @@ -388,7 +387,7 @@ func (h *Handler) HandleComplete(w http.ResponseWriter, r *http.Request) { return } w.Header().Set("Location", "/responses/"+finalID) - httputil.WriteJSON(w, http.StatusCreated, map[string]string{"response_id": finalID}) + writeJSON(w, http.StatusCreated, map[string]string{"response_id": finalID}) } // HandleAbort handles PUT /staging//abort. @@ -397,7 +396,7 @@ func (h *Handler) HandleComplete(w http.ResponseWriter, r *http.Request) { func (h *Handler) HandleAbort(w http.ResponseWriter, r *http.Request) { stagingID := r.PathValue("id") if len(stagingID) > 64 { - httputil.WriteError(w, http.StatusBadRequest, "invalid staging id") + writeError(w, http.StatusBadRequest, "invalid staging id") return } if err := h.svc.AbortStaging(r.Context(), stagingID); err != nil { @@ -445,7 +444,7 @@ func (h *Handler) HandleDelete(w http.ResponseWriter, r *http.Request) { // HandleHealthz handles GET /healthz (liveness probe). func (h *Handler) HandleHealthz(w http.ResponseWriter, _ *http.Request) { - httputil.WriteJSON(w, http.StatusOK, map[string]string{"status": "ok"}) + writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } // HandleReadyz handles GET /readyz (readiness probe). @@ -453,8 +452,8 @@ func (h *Handler) HandleHealthz(w http.ResponseWriter, _ *http.Request) { func (h *Handler) HandleReadyz(w http.ResponseWriter, r *http.Request) { if err := h.svc.Ping(r.Context()); err != nil { h.log.Error("readyz: storage ping failed", "err", err) - httputil.WriteJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "storage unavailable"}) + writeJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "storage unavailable"}) return } - httputil.WriteJSON(w, http.StatusOK, map[string]string{"status": "ok"}) + writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } diff --git a/internal/server/httputil.go b/internal/server/httputil.go new file mode 100644 index 0000000..67dd88a --- /dev/null +++ b/internal/server/httputil.go @@ -0,0 +1,16 @@ +package server + +import ( + "encoding/json" + "net/http" +) + +func writeJSON(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(v) +} + +func writeError(w http.ResponseWriter, status int, msg string) { + writeJSON(w, status, map[string]string{"error": msg}) +} diff --git a/internal/metrics/metrics.go b/internal/server/metrics.go similarity index 72% rename from internal/metrics/metrics.go rename to internal/server/metrics.go index 70c8f73..260cb51 100644 --- a/internal/metrics/metrics.go +++ b/internal/server/metrics.go @@ -1,4 +1,4 @@ -package metrics +package server import ( "errors" @@ -7,20 +7,20 @@ import ( ) var ( - HTTPRequestsTotal = prometheus.NewCounterVec( + httpRequestsTotal = prometheus.NewCounterVec( prometheus.CounterOpts{Name: "http_requests_total"}, []string{"endpoint", "status"}, ) - HTTPRequestDuration = prometheus.NewHistogramVec( + httpRequestDuration = prometheus.NewHistogramVec( prometheus.HistogramOpts{Name: "http_request_duration_seconds"}, []string{"endpoint"}, ) ) -// Register registers all metrics into reg under the given namespace prefix. +// RegisterMetrics registers HTTP metrics into reg under the given namespace prefix. // reg nil uses prometheus.DefaultRegisterer; empty namespace defaults to "responses". // Returns an error if registration fails for any reason other than already-registered. -func Register(reg prometheus.Registerer, namespace string) error { +func RegisterMetrics(reg prometheus.Registerer, namespace string) error { if reg == nil { reg = prometheus.DefaultRegisterer } @@ -29,8 +29,8 @@ func Register(reg prometheus.Registerer, namespace string) error { } wrapped := prometheus.WrapRegistererWithPrefix(namespace+"_", reg) for _, c := range []prometheus.Collector{ - HTTPRequestsTotal, - HTTPRequestDuration, + httpRequestsTotal, + httpRequestDuration, } { if err := wrapped.Register(c); err != nil { var are prometheus.AlreadyRegisteredError diff --git a/internal/server/middleware.go b/internal/server/middleware.go index f997c96..828272a 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -9,8 +9,6 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/trace" - - "github.com/elevran/charon/internal/metrics" ) // Middleware is a function that wraps an http.Handler. @@ -76,8 +74,8 @@ func RequestLogger(log *slog.Logger) Middleware { "duration_ms", dur.Milliseconds(), ) endpoint := r.Method + " " + r.Pattern - metrics.HTTPRequestsTotal.WithLabelValues(endpoint, strconv.Itoa(status)).Inc() - metrics.HTTPRequestDuration.WithLabelValues(endpoint).Observe(dur.Seconds()) + httpRequestsTotal.WithLabelValues(endpoint, strconv.Itoa(status)).Inc() + httpRequestDuration.WithLabelValues(endpoint).Observe(dur.Seconds()) }) } } diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index ddd7f8a..e2c9cc8 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -23,7 +23,6 @@ import ( "github.com/elevran/charon/internal/chainstore" pebblebe "github.com/elevran/charon/internal/chainstore/pebble" - "github.com/elevran/charon/internal/metrics" api "github.com/elevran/charon/internal/server" ) @@ -39,7 +38,7 @@ func newFixture(t *testing.T) *fixture { log := slog.New(slog.NewTextHandler(io.Discard, nil)) reg := prometheus.NewRegistry() - require.NoError(t, metrics.Register(reg, "")) + require.NoError(t, api.RegisterMetrics(reg, "")) opts := &crdbpebble.Options{FS: vfs.NewMem()} // Pass reg so chainstore metrics (chainstore_entries_total, etc.) are exported. From d67afc78df61ed337e63e3c37d25a55466588b88 Mon Sep 17 00:00:00 2001 From: Etai Lev Ran Date: Wed, 8 Jul 2026 18:09:21 +0300 Subject: [PATCH 2/4] chore: scope binary ignores to repo root Replace bare 'charon' (which also matched cmd/charon/) with rooted /charon and /proxy so only the top-level binaries are excluded and cmd/* source directories are tracked normally. Signed-off-by: Etai Lev Ran --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index dcbf9b2..bc3458f 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,5 @@ # Local development .claude/ agent/ -charon +/charon +/proxy From a52c2930701b29c4ec935cfcc61e1914a72b9b69 Mon Sep 17 00:00:00 2001 From: Etai Lev Ran Date: Wed, 8 Jul 2026 20:13:00 +0300 Subject: [PATCH 3/4] refactor: eliminate small-package smell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delete internal/httputil: inline writeJSON/writeError as unexported helpers in cmd/proxy and internal/server (WriteJSON/WriteError exported from internal/server so cmd/proxy can use them without a separate pkg) - Delete internal/metrics: fold HTTPRequestsTotal, HTTPRequestDuration, and Register() into internal/server as unexported vars + RegisterMetrics() - Move TelemetryOptions to internal/telemetry as Options; both charonconfig and proxyconfig import it instead of defining their own copy - Split internal/config into internal/charonconfig (CharonOptions + ByteSize) and internal/proxyconfig (ProxyOptions); each has its own private YAML loader with strict unmarshalling; no shared config file between the two processes — all json.RawMessage absorbers removed - Simplify proxyconfig: collapse fileConfigRaw+fileConfig into a single fileConfig struct; fileCharonSection holds only Listen (no storage/workers absorbers); applyFileDefaults called post-unmarshal only - Fix .gitignore: build outputs go under ./bin/; remove stale /charon and /proxy entries that blocked staging source files - Update Makefile: build targets write to ./bin/; add clean target Signed-off-by: Etai Lev Ran --- .gitignore | 4 +- Makefile | 16 ++-- cmd/charon/main.go | 2 +- cmd/charon/main_test.go | 2 +- cmd/proxy/handler.go | 31 ++++---- cmd/proxy/httputil.go | 16 ---- cmd/proxy/main.go | 2 +- cmd/proxy/sse.go | 5 +- cmd/proxy/ws.go | 3 +- internal/charonconfig/bytesize.go | 11 +-- internal/charonconfig/bytesize_test.go | 2 +- internal/charonconfig/charonconfig.go | 52 ++++--------- internal/charonconfig/charonconfig_test.go | 35 ++------- internal/charonconfig/testdata/config.yaml | 2 - internal/proxyconfig/proxyconfig.go | 86 ++++++++------------- internal/proxyconfig/proxyconfig_test.go | 34 ++++---- internal/proxyconfig/testdata/bytesize.yaml | 3 - internal/proxyconfig/testdata/config.yaml | 2 - internal/proxyconfig/testdata/invalid.yaml | 3 - internal/server/handlers.go | 44 +++++------ internal/server/httputil.go | 8 +- internal/server/metrics.go | 8 +- internal/server/middleware.go | 4 +- internal/telemetry/options.go | 14 ++++ 24 files changed, 158 insertions(+), 231 deletions(-) delete mode 100644 cmd/proxy/httputil.go delete mode 100644 internal/proxyconfig/testdata/bytesize.yaml delete mode 100644 internal/proxyconfig/testdata/invalid.yaml create mode 100644 internal/telemetry/options.go diff --git a/.gitignore b/.gitignore index bc3458f..cd9867c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,6 @@ # Build output -/build/ +/bin/ # Local development .claude/ agent/ -/charon -/proxy diff --git a/Makefile b/Makefile index f02c2a7..834afba 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,7 @@ -BINARY := charon -CMD := ./cmd/charon -BUILD_DIR := ./build +BIN_DIR := ./bin IMAGE_NAME ?= charon IMAGE_TAG ?= latest -GOOS ?= $(shell go env GOOS) -GOARCH ?= $(shell go env GOARCH) - -OUT := $(BUILD_DIR)/$(GOOS)/$(GOARCH)/$(BINARY) - OPENRESPONSES_DIR ?= ../openresponses .PHONY: all fmt fmt-check vet lint presubmit test test-unit test-integration test-compliance test-disruptive test-system test-compliance-bun tidy update build image clean @@ -86,11 +79,12 @@ update: # ── Build & package ─────────────────────────────────────────────────────────── build: - mkdir -p $(BUILD_DIR)/$(GOOS)/$(GOARCH) - go build -o $(OUT) $(CMD) + mkdir -p $(BIN_DIR) + go build -o $(BIN_DIR)/charon ./cmd/charon + go build -o $(BIN_DIR)/proxy ./cmd/proxy image: docker build -t $(IMAGE_NAME):$(IMAGE_TAG) . clean: - rm -rf $(BUILD_DIR) + rm -rf $(BIN_DIR) diff --git a/cmd/charon/main.go b/cmd/charon/main.go index bd4ccd1..13b0632 100644 --- a/cmd/charon/main.go +++ b/cmd/charon/main.go @@ -28,7 +28,7 @@ func main() { func run() error { log := slog.New(slog.NewJSONHandler(os.Stdout, nil)) - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() fs := flag.NewFlagSet("charon", flag.ExitOnError) opts.AddFlags(fs) _ = fs.Parse(os.Args[1:]) diff --git a/cmd/charon/main_test.go b/cmd/charon/main_test.go index 31a9ed9..ab5847e 100644 --- a/cmd/charon/main_test.go +++ b/cmd/charon/main_test.go @@ -10,7 +10,7 @@ import ( ) func TestConfigDefaults(t *testing.T) { - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() require.Equal(t, ":8081", opts.Listen) require.Equal(t, 30, opts.TTLDays) } diff --git a/cmd/proxy/handler.go b/cmd/proxy/handler.go index 70e67b0..cbc752d 100644 --- a/cmd/proxy/handler.go +++ b/cmd/proxy/handler.go @@ -9,6 +9,7 @@ import ( "time" "github.com/elevran/charon/cmd/proxy/inference" + "github.com/elevran/charon/internal/server" "github.com/elevran/charon/pkg/charon" ) @@ -45,23 +46,23 @@ func RegisterHandlers(mux *http.ServeMux, h *Handler) { func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { bodyBytes, err := io.ReadAll(r.Body) if err != nil { - writeError(w, http.StatusBadRequest, "failed to read request body") + server.WriteError(w, http.StatusBadRequest, "failed to read request body") return } var req CreateRequest if err := json.Unmarshal(bodyBytes, &req); err != nil { - writeError(w, http.StatusBadRequest, "invalid request body") + server.WriteError(w, http.StatusBadRequest, "invalid request body") return } if req.Model == "" { - writeError(w, http.StatusBadRequest, "model is required") + server.WriteError(w, http.StatusBadRequest, "model is required") return } var rawReq map[string]json.RawMessage if err := json.Unmarshal(bodyBytes, &rawReq); err != nil { - writeError(w, http.StatusBadRequest, "invalid request body") + server.WriteError(w, http.StatusBadRequest, "invalid request body") return } @@ -75,7 +76,7 @@ func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { inputItems, err := inputToItems(req.Input) if err != nil { - writeError(w, http.StatusBadRequest, "invalid input") + server.WriteError(w, http.StatusBadRequest, "invalid input") return } @@ -100,7 +101,7 @@ func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { infResp, err := h.inf.Complete(ctx, infMap) if err != nil { h.log.Error("inference complete", "err", err) - writeError(w, http.StatusBadGateway, "inference error") + server.WriteError(w, http.StatusBadGateway, "inference error") return } completedAt := time.Now() @@ -109,13 +110,13 @@ func (h *Handler) HandleCreate(w http.ResponseWriter, r *http.Request) { responseBlob := marshalStoredResponse(infResp, req.PreviousResponseID, req.Instructions, req.Background) if err := h.charon.Store(ctx, infResp.ID, stagingID, tenantKey, responseBlob); err != nil { h.log.Error("charon store", "id", infResp.ID, "err", err) - writeError(w, http.StatusInternalServerError, "storage error") + server.WriteError(w, http.StatusInternalServerError, "storage error") return } } resource := buildResponseResource(infResp, req.PreviousResponseID, req.ShouldStore(), req.Background, createdAt, &completedAt) - writeJSON(w, http.StatusOK, resource) + server.WriteJSON(w, http.StatusOK, resource) } // HandleRetrieve handles GET /responses/{id}. @@ -132,7 +133,7 @@ func (h *Handler) HandleRetrieve(w http.ResponseWriter, r *http.Request) { var stored storedResponse if err := json.Unmarshal(retrieved.ResponseBlob, &stored); err != nil { h.log.Error("unmarshal stored response", "id", id, "err", err) - writeError(w, http.StatusInternalServerError, "internal server error") + server.WriteError(w, http.StatusInternalServerError, "internal server error") return } @@ -159,7 +160,7 @@ func (h *Handler) HandleRetrieve(w http.ResponseWriter, r *http.Request) { Metadata: map[string]string{}, ServiceTier: "default", } - writeJSON(w, http.StatusOK, resource) + server.WriteJSON(w, http.StatusOK, resource) } // HandleDelete handles DELETE /responses/{id}. @@ -180,10 +181,10 @@ func (h *Handler) HandleCompact(w http.ResponseWriter, r *http.Request) { } _ = json.NewDecoder(r.Body).Decode(&body) if body.Model == "" { - writeError(w, http.StatusBadRequest, "model is required") + server.WriteError(w, http.StatusBadRequest, "model is required") return } - writeError(w, http.StatusNotImplemented, "compact not implemented") + server.WriteError(w, http.StatusNotImplemented, "compact not implemented") } // HandleListOrWS is implemented in ws.go. @@ -195,17 +196,17 @@ func (h *Handler) HandleCompact(w http.ResponseWriter, r *http.Request) { func (h *Handler) mapCharonError(w http.ResponseWriter, err error, notFoundCode string) { switch { case errors.Is(err, charon.ErrNotFound): - writeJSON(w, http.StatusNotFound, map[string]string{ + server.WriteJSON(w, http.StatusNotFound, map[string]string{ "code": notFoundCode, "message": err.Error(), }) case errors.Is(err, charon.ErrChainCorrupted): - writeJSON(w, http.StatusConflict, map[string]string{ + server.WriteJSON(w, http.StatusConflict, map[string]string{ "code": "chain_corrupted", "message": err.Error(), }) default: h.log.Error("charon error", "err", err) - writeError(w, http.StatusInternalServerError, "internal server error") + server.WriteError(w, http.StatusInternalServerError, "internal server error") } } diff --git a/cmd/proxy/httputil.go b/cmd/proxy/httputil.go deleted file mode 100644 index 85ff3b7..0000000 --- a/cmd/proxy/httputil.go +++ /dev/null @@ -1,16 +0,0 @@ -package main - -import ( - "encoding/json" - "net/http" -) - -func writeJSON(w http.ResponseWriter, status int, v any) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(v) -} - -func writeError(w http.ResponseWriter, status int, msg string) { - writeJSON(w, status, map[string]string{"error": msg}) -} diff --git a/cmd/proxy/main.go b/cmd/proxy/main.go index 9763a2d..2071646 100644 --- a/cmd/proxy/main.go +++ b/cmd/proxy/main.go @@ -26,7 +26,7 @@ func main() { func run() error { log := slog.New(slog.NewJSONHandler(os.Stdout, nil)) - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("proxy", flag.ExitOnError) opts.AddFlags(fs) _ = fs.Parse(os.Args[1:]) diff --git a/cmd/proxy/sse.go b/cmd/proxy/sse.go index c0f8204..396793d 100644 --- a/cmd/proxy/sse.go +++ b/cmd/proxy/sse.go @@ -8,6 +8,7 @@ import ( "time" "github.com/elevran/charon/cmd/proxy/inference" + "github.com/elevran/charon/internal/server" "github.com/elevran/charon/pkg/charon" ) @@ -63,7 +64,7 @@ func (h *Handler) handleStream(w http.ResponseWriter, r *http.Request, req Creat inputItems, err := inputToItems(req.Input) if err != nil { - writeError(w, http.StatusBadRequest, "invalid input") + server.WriteError(w, http.StatusBadRequest, "invalid input") return } @@ -89,7 +90,7 @@ func (h *Handler) handleStream(w http.ResponseWriter, r *http.Request, req Creat ch, err := h.inf.Stream(ctx, infMap) if err != nil { h.log.Error("inference stream", "err", err) - writeError(w, http.StatusBadGateway, "inference error") + server.WriteError(w, http.StatusBadGateway, "inference error") return } diff --git a/cmd/proxy/ws.go b/cmd/proxy/ws.go index b6e3061..dcfe360 100644 --- a/cmd/proxy/ws.go +++ b/cmd/proxy/ws.go @@ -12,6 +12,7 @@ import ( "github.com/gorilla/websocket" "github.com/elevran/charon/cmd/proxy/inference" + "github.com/elevran/charon/internal/server" "github.com/elevran/charon/pkg/charon" ) @@ -76,7 +77,7 @@ func (h *Handler) HandleListOrWS(w http.ResponseWriter, r *http.Request) { h.HandleWebSocket(w, r) return } - writeJSON(w, http.StatusOK, map[string]interface{}{ + server.WriteJSON(w, http.StatusOK, map[string]interface{}{ "object": "list", "data": []interface{}{}, "has_more": false, diff --git a/internal/charonconfig/bytesize.go b/internal/charonconfig/bytesize.go index b4f9c84..265ea2b 100644 --- a/internal/charonconfig/bytesize.go +++ b/internal/charonconfig/bytesize.go @@ -7,8 +7,9 @@ import ( "strings" ) -// byteSizeType is the concrete type; ByteSize aliases it so tests can reference it. -type byteSizeType int64 +// ByteSize is an int64 that unmarshals from either a plain integer (bytes) or +// a string with an optional unit suffix: B, KB, MB, GB. K=1024. +type ByteSize int64 var unitMultipliers = map[string]int64{ "b": 1, @@ -17,10 +18,10 @@ var unitMultipliers = map[string]int64{ "gb": 1024 * 1024 * 1024, } -func (b *byteSizeType) UnmarshalJSON(data []byte) error { +func (b *ByteSize) UnmarshalJSON(data []byte) error { var n int64 if err := json.Unmarshal(data, &n); err == nil { - *b = byteSizeType(n) + *b = ByteSize(n) return nil } @@ -59,6 +60,6 @@ func (b *byteSizeType) UnmarshalJSON(data []byte) error { return fmt.Errorf("bytesize: negative size %q", s) } - *b = byteSizeType(n * mult) + *b = ByteSize(n * mult) return nil } diff --git a/internal/charonconfig/bytesize_test.go b/internal/charonconfig/bytesize_test.go index cb37d99..cb934e4 100644 --- a/internal/charonconfig/bytesize_test.go +++ b/internal/charonconfig/bytesize_test.go @@ -72,7 +72,7 @@ func TestByteSizeUnmarshalErrors(t *testing.T) { } func TestByteSizeInConfig(t *testing.T) { - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/bytesize.yaml"})) diff --git a/internal/charonconfig/charonconfig.go b/internal/charonconfig/charonconfig.go index 72523e0..8d941df 100644 --- a/internal/charonconfig/charonconfig.go +++ b/internal/charonconfig/charonconfig.go @@ -1,29 +1,15 @@ package charonconfig import ( - "encoding/json" "flag" "fmt" "os" "time" "sigs.k8s.io/yaml" -) - -// ByteSize is an int64 that unmarshals from either a plain integer (bytes) or -// a string with an optional unit suffix: B, KB, MB, GB. K=1024. -type ByteSize = byteSizeType - -// TelemetryOptions holds OpenTelemetry settings. -type TelemetryOptions struct { - ExporterURL string // OTLP HTTP endpoint; empty = disabled - ServiceName string // identifies this binary in traces -} -// AddFlags registers the --telemetry-exporter-url flag on fs. -func (o *TelemetryOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&o.ExporterURL, "telemetry-exporter-url", o.ExporterURL, "OTLP HTTP exporter URL (e.g. http://localhost:4318); empty disables tracing") -} + "github.com/elevran/charon/internal/telemetry" +) // CharonOptions holds configuration for the Charon response-storage server. type CharonOptions struct { @@ -45,17 +31,17 @@ type CharonOptions struct { // WorkerTTLInterval is how often the background TTL reaper runs (not the TTL itself). WorkerTTLInterval time.Duration - Telemetry TelemetryOptions + Telemetry telemetry.Options } -// NewCharonOptions returns a CharonOptions pre-filled with built-in defaults. -func NewCharonOptions() *CharonOptions { +// NewOptions returns a CharonOptions pre-filled with built-in defaults. +func NewOptions() *CharonOptions { return &CharonOptions{ Listen: ":8081", DataDir: "./data", TTLDays: 30, WorkerTTLInterval: time.Hour, - Telemetry: TelemetryOptions{ServiceName: "charon"}, + Telemetry: telemetry.Options{ServiceName: "charon"}, } } @@ -101,7 +87,7 @@ func (o *CharonOptions) Complete(fs *flag.FlagSet) error { if !setFlags["telemetry-exporter-url"] { o.Telemetry.ExporterURL = fc.Telemetry.ExporterURL } - o.Telemetry.ServiceName = fc.Telemetry.CharonService + o.Telemetry.ServiceName = fc.Telemetry.ServiceName return nil } @@ -119,7 +105,6 @@ func (o *CharonOptions) Validate() error { // --------------------------------------------------------------------------- type fileConfig struct { - Proxy json.RawMessage `json:"proxy"` // accepted but unused — avoids strict-parse rejection of shared config files Charon fileCharonConfig `json:"charon"` Telemetry fileTelemetryConfig `json:"telemetry"` } @@ -131,12 +116,12 @@ type fileCharonConfig struct { } type fileStorageConfig struct { - DataDir string `json:"data_dir"` - TTLDays int `json:"ttl_days"` - MaxResponses int64 `json:"max_responses"` - MaxPayload byteSizeType `json:"max_payload"` - MaxChainDepth int `json:"max_chain_depth"` - MaxContextBytes byteSizeType `json:"max_context_bytes"` + DataDir string `json:"data_dir"` + TTLDays int `json:"ttl_days"` + MaxResponses int64 `json:"max_responses"` + MaxPayload ByteSize `json:"max_payload"` + MaxChainDepth int `json:"max_chain_depth"` + MaxContextBytes ByteSize `json:"max_context_bytes"` } type fileWorkerConfig struct { @@ -144,9 +129,8 @@ type fileWorkerConfig struct { } type fileTelemetryConfig struct { - ExporterURL string `json:"exporter_url"` - CharonService string `json:"charon_service"` - ProxyService string `json:"proxy_service"` // accepted but unused — avoids strict-parse rejection of shared config files + ExporterURL string `json:"exporter_url"` + ServiceName string `json:"service_name"` } func applyFileDefaults(fc *fileConfig) { @@ -162,15 +146,13 @@ func applyFileDefaults(fc *fileConfig) { if fc.Charon.Workers.TTLInterval <= 0 { fc.Charon.Workers.TTLInterval = time.Hour } - if fc.Telemetry.CharonService == "" { - fc.Telemetry.CharonService = "charon" + if fc.Telemetry.ServiceName == "" { + fc.Telemetry.ServiceName = "charon" } } func loadFileConfig(path string) (fileConfig, error) { var fc fileConfig - applyFileDefaults(&fc) - data, err := os.ReadFile(path) if err != nil { return fc, fmt.Errorf("read config: %w", err) diff --git a/internal/charonconfig/charonconfig_test.go b/internal/charonconfig/charonconfig_test.go index d1e71bd..a924873 100644 --- a/internal/charonconfig/charonconfig_test.go +++ b/internal/charonconfig/charonconfig_test.go @@ -14,7 +14,7 @@ import ( ) func TestCharonOptionsDefaultsWithNoConfig(t *testing.T) { - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse(nil)) @@ -28,7 +28,7 @@ func TestCharonOptionsDefaultsWithNoConfig(t *testing.T) { } func TestCharonOptionsCompleteLoadsFile(t *testing.T) { - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) @@ -39,7 +39,7 @@ func TestCharonOptionsCompleteLoadsFile(t *testing.T) { } func TestCharonOptionsCLIOverridesFile(t *testing.T) { - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":7777"})) @@ -49,7 +49,7 @@ func TestCharonOptionsCLIOverridesFile(t *testing.T) { } func TestCharonOptionsValidateOK(t *testing.T) { - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() require.NoError(t, opts.Validate()) } @@ -59,7 +59,7 @@ func TestCharonOptionsDataDirCLIOverridesFile(t *testing.T) { err := os.WriteFile(cfgPath, []byte("charon:\n storage:\n data_dir: /tmp/test-data\n"), 0600) require.NoError(t, err) - opts := charonconfig.NewCharonOptions() + opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--storage-data-dir", "./my-data"})) @@ -68,31 +68,8 @@ func TestCharonOptionsDataDirCLIOverridesFile(t *testing.T) { assert.Equal(t, "./my-data", opts.DataDir) } -func TestLoadDefaults(t *testing.T) { - charon := charonconfig.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - charon.AddFlags(fs) - require.NoError(t, fs.Parse(nil)) - require.NoError(t, charon.Complete(fs)) - - assert.Equal(t, ":8081", charon.Listen) - assert.Equal(t, "./data", charon.DataDir) - assert.Equal(t, 30, charon.TTLDays) - assert.Equal(t, time.Hour, charon.WorkerTTLInterval) - assert.Equal(t, "charon", charon.Telemetry.ServiceName) -} - -func TestLoadFromFile(t *testing.T) { - charon := charonconfig.NewCharonOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - charon.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) - require.NoError(t, charon.Complete(fs)) - assert.Equal(t, 30, charon.TTLDays) -} - func TestLoadStrictRejectsUnknownFields(t *testing.T) { - charon := charonconfig.NewCharonOptions() + charon := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) charon.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/invalid.yaml"})) diff --git a/internal/charonconfig/testdata/config.yaml b/internal/charonconfig/testdata/config.yaml index 1b36aa8..565e809 100644 --- a/internal/charonconfig/testdata/config.yaml +++ b/internal/charonconfig/testdata/config.yaml @@ -1,5 +1,3 @@ -proxy: - listen: ":0" charon: listen: ":9090" storage: diff --git a/internal/proxyconfig/proxyconfig.go b/internal/proxyconfig/proxyconfig.go index f3d4db2..9e0b1a1 100644 --- a/internal/proxyconfig/proxyconfig.go +++ b/internal/proxyconfig/proxyconfig.go @@ -1,25 +1,15 @@ package proxyconfig import ( - "encoding/json" "flag" "fmt" "net" "os" "sigs.k8s.io/yaml" -) - -// TelemetryOptions holds OpenTelemetry settings. -type TelemetryOptions struct { - ExporterURL string // OTLP HTTP endpoint; empty = disabled - ServiceName string // identifies this binary in traces -} -// AddFlags registers the --telemetry-exporter-url flag on fs. -func (o *TelemetryOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&o.ExporterURL, "telemetry-exporter-url", o.ExporterURL, "OTLP HTTP exporter URL (e.g. http://localhost:4318); empty disables tracing") -} + "github.com/elevran/charon/internal/telemetry" +) // ProxyOptions holds configuration for the proxy server. type ProxyOptions struct { @@ -38,17 +28,17 @@ type ProxyOptions struct { // Auto-derived from config file proxy.charon_url or Charon's listen address. CharonURL string - Telemetry TelemetryOptions + Telemetry telemetry.Options } -// NewProxyOptions returns a ProxyOptions pre-filled with built-in defaults. -func NewProxyOptions() *ProxyOptions { +// NewOptions returns a ProxyOptions pre-filled with built-in defaults. +func NewOptions() *ProxyOptions { return &ProxyOptions{ Listen: ":8080", Backend: "http://localhost:11434", TimeoutSeconds: 120, CharonURL: "http://127.0.0.1:8081", - Telemetry: TelemetryOptions{ServiceName: "charon-proxy"}, + Telemetry: telemetry.Options{ServiceName: "charon-proxy"}, } } @@ -89,14 +79,14 @@ func (o *ProxyOptions) Complete(fs *flag.FlagSet) error { if fc.Proxy.CharonURL != "" { o.CharonURL = fc.Proxy.CharonURL } else { - o.CharonURL = deriveCharonURL(fc.CharonListen) + o.CharonURL = deriveCharonURL(fc.Charon.Listen) } } if !setFlags["telemetry-exporter-url"] { o.Telemetry.ExporterURL = fc.Telemetry.ExporterURL } - o.Telemetry.ServiceName = fc.Telemetry.ProxyService + o.Telemetry.ServiceName = fc.Telemetry.ServiceName return nil } @@ -128,22 +118,16 @@ func deriveCharonURL(charonListen string) string { // --------------------------------------------------------------------------- type fileConfig struct { - Proxy fileProxyConfig `json:"proxy"` - CharonListen string `json:"_charon_listen"` // populated from charon.listen after parse - Telemetry fileTelemetryConfig `json:"telemetry"` -} - -// fileConfigRaw is the raw YAML shape; we flatten charon.listen into fileConfig. -type fileConfigRaw struct { Proxy fileProxyConfig `json:"proxy"` Charon fileCharonSection `json:"charon"` Telemetry fileTelemetryConfig `json:"telemetry"` } +// fileCharonSection holds only the Charon fields the proxy needs: the listen +// address (used to derive CharonURL). Charon-specific storage and worker +// config belongs in the Charon config file, never here. type fileCharonSection struct { - Listen string `json:"listen"` - Storage json.RawMessage `json:"storage"` - Workers json.RawMessage `json:"workers"` + Listen string `json:"listen"` } type fileProxyConfig struct { @@ -159,47 +143,39 @@ type fileInferenceConfig struct { } type fileTelemetryConfig struct { - ExporterURL string `json:"exporter_url"` - ProxyService string `json:"proxy_service"` - CharonService string `json:"charon_service"` // accepted but unused — avoids strict-parse rejection of shared config files + ExporterURL string `json:"exporter_url"` + ServiceName string `json:"service_name"` } -func applyFileDefaults(raw *fileConfigRaw) { - if raw.Proxy.Listen == "" { - raw.Proxy.Listen = ":8080" +func applyFileDefaults(fc *fileConfig) { + if fc.Proxy.Listen == "" { + fc.Proxy.Listen = ":8080" } - if raw.Proxy.Inference.BaseURL == "" { - raw.Proxy.Inference.BaseURL = "http://localhost:11434" + if fc.Proxy.Inference.BaseURL == "" { + fc.Proxy.Inference.BaseURL = "http://localhost:11434" } - if raw.Proxy.Inference.TimeoutSeconds <= 0 { - raw.Proxy.Inference.TimeoutSeconds = 120 + if fc.Proxy.Inference.TimeoutSeconds <= 0 { + fc.Proxy.Inference.TimeoutSeconds = 120 } - if raw.Charon.Listen == "" { - raw.Charon.Listen = ":8081" + if fc.Charon.Listen == "" { + fc.Charon.Listen = ":8081" } - if raw.Telemetry.ProxyService == "" { - raw.Telemetry.ProxyService = "charon-proxy" + if fc.Telemetry.ServiceName == "" { + fc.Telemetry.ServiceName = "charon-proxy" } } func loadFileConfig(path string) (fileConfig, error) { - var raw fileConfigRaw - applyFileDefaults(&raw) - + var fc fileConfig data, err := os.ReadFile(path) if err != nil { - return fileConfig{}, fmt.Errorf("read config: %w", err) + return fc, fmt.Errorf("read config: %w", err) } - if err := yaml.UnmarshalStrict(data, &raw); err != nil { - return fileConfig{}, fmt.Errorf("parse config: %w", err) + if err := yaml.UnmarshalStrict(data, &fc); err != nil { + return fc, fmt.Errorf("parse config: %w", err) } - applyFileDefaults(&raw) - - return fileConfig{ - Proxy: raw.Proxy, - CharonListen: raw.Charon.Listen, - Telemetry: raw.Telemetry, - }, nil + applyFileDefaults(&fc) + return fc, nil } func visitedFlags(fs *flag.FlagSet) map[string]bool { diff --git a/internal/proxyconfig/proxyconfig_test.go b/internal/proxyconfig/proxyconfig_test.go index 144359b..a40a02c 100644 --- a/internal/proxyconfig/proxyconfig_test.go +++ b/internal/proxyconfig/proxyconfig_test.go @@ -13,7 +13,7 @@ import ( ) func TestProxyOptionsDefaultsWithNoConfig(t *testing.T) { - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse(nil)) @@ -27,7 +27,7 @@ func TestProxyOptionsDefaultsWithNoConfig(t *testing.T) { } func TestProxyOptionsCompleteLoadsFile(t *testing.T) { - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) @@ -39,7 +39,7 @@ func TestProxyOptionsCompleteLoadsFile(t *testing.T) { } func TestProxyOptionsCLIOverridesFile(t *testing.T) { - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":5555"})) @@ -54,7 +54,7 @@ func TestProxyOptionsBackendCLIOverridesFile(t *testing.T) { err := os.WriteFile(cfgPath, []byte("proxy:\n inference:\n base_url: http://file-backend:11434\n"), 0600) require.NoError(t, err) - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--backend", "http://cli-backend:11434"})) @@ -69,7 +69,7 @@ func TestProxyOptionsCharonURLCLIOverridesFile(t *testing.T) { err := os.WriteFile(cfgPath, []byte("proxy:\n charon_url: http://file-charon:8081\n"), 0600) require.NoError(t, err) - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--charon-url", "http://cli-charon:9999"})) @@ -79,12 +79,12 @@ func TestProxyOptionsCharonURLCLIOverridesFile(t *testing.T) { } func TestProxyOptionsValidateOK(t *testing.T) { - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() require.NoError(t, opts.Validate()) } func TestProxyOptionsValidateEmptyBackendFails(t *testing.T) { - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() opts.Backend = "" err := opts.Validate() require.Error(t, err) @@ -92,7 +92,7 @@ func TestProxyOptionsValidateEmptyBackendFails(t *testing.T) { } func TestCharonURLDerivedFromListen(t *testing.T) { - opts := proxyconfig.NewProxyOptions() + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) @@ -100,11 +100,17 @@ func TestCharonURLDerivedFromListen(t *testing.T) { assert.Equal(t, "http://127.0.0.1:9090", opts.CharonURL) } -func TestLoadFromFile(t *testing.T) { - proxy := proxyconfig.NewProxyOptions() +func TestLoadStrictRejectsUnknownFields(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "bad.yaml") + err := os.WriteFile(cfgPath, []byte("proxy:\n unknown_key: true\n"), 0600) + require.NoError(t, err) + + opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) - proxy.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) - require.NoError(t, proxy.Complete(fs)) - assert.Equal(t, ":0", proxy.Listen) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", cfgPath})) + err = opts.Complete(fs) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse config") } diff --git a/internal/proxyconfig/testdata/bytesize.yaml b/internal/proxyconfig/testdata/bytesize.yaml deleted file mode 100644 index ae0709a..0000000 --- a/internal/proxyconfig/testdata/bytesize.yaml +++ /dev/null @@ -1,3 +0,0 @@ -charon: - storage: - max_payload: "10MB" diff --git a/internal/proxyconfig/testdata/config.yaml b/internal/proxyconfig/testdata/config.yaml index 1b36aa8..8f72fb8 100644 --- a/internal/proxyconfig/testdata/config.yaml +++ b/internal/proxyconfig/testdata/config.yaml @@ -2,5 +2,3 @@ proxy: listen: ":0" charon: listen: ":9090" - storage: - data_dir: ./data diff --git a/internal/proxyconfig/testdata/invalid.yaml b/internal/proxyconfig/testdata/invalid.yaml deleted file mode 100644 index da10a6c..0000000 --- a/internal/proxyconfig/testdata/invalid.yaml +++ /dev/null @@ -1,3 +0,0 @@ -server: - listen: ":8080" - unknown_field: true diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 0c00d96..05754ef 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -51,7 +51,7 @@ func (h *Handler) writeErr(w http.ResponseWriter, op, id string, err error) { if status == http.StatusInternalServerError { h.log.Error(op, "id", id, "err", err) } - writeError(w, status, msg) + WriteError(w, status, msg) } // blobToRaw converts a raw-bytes blob to json.RawMessage for the wire format. @@ -127,7 +127,7 @@ func (h *Handler) HandleBufferedStore(w http.ResponseWriter, r *http.Request) { } var req bufferedStoreRequest if err := json.Unmarshal(b, &req); err != nil { - writeError(w, http.StatusBadRequest, "invalid JSON body") + WriteError(w, http.StatusBadRequest, "invalid JSON body") return } @@ -156,7 +156,7 @@ func (h *Handler) HandleBufferedStore(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Location", "/responses/"+responseID) - writeJSON(w, http.StatusCreated, bufferedStoreResponse{ + WriteJSON(w, http.StatusCreated, bufferedStoreResponse{ ResponseID: responseID, Turns: respTurns, }) @@ -177,7 +177,7 @@ func (h *Handler) HandleOpenStaging(w http.ResponseWriter, r *http.Request) { } requestBlob, err := io.ReadAll(r.Body) if err != nil { - writeError(w, http.StatusBadRequest, "failed to read request body") + WriteError(w, http.StatusBadRequest, "failed to read request body") return } _ = r.Body.Close() @@ -198,7 +198,7 @@ func (h *Handler) HandleOpenStaging(w http.ResponseWriter, r *http.Request) { w.Header().Set("Location", "/staging/"+stagingID) w.Header().Set("X-Staging-ID", stagingID) - writeJSON(w, http.StatusCreated, resolveResponse{ + WriteJSON(w, http.StatusCreated, resolveResponse{ StagingID: stagingID, Turns: respTurns, }) @@ -213,7 +213,7 @@ func (h *Handler) HandleOpenStaging(w http.ResponseWriter, r *http.Request) { func (h *Handler) HandleStagingStatus(w http.ResponseWriter, r *http.Request) { stagingID := r.PathValue("id") if len(stagingID) > 64 { - writeError(w, http.StatusBadRequest, "invalid staging id") + WriteError(w, http.StatusBadRequest, "invalid staging id") return } ctx := r.Context() @@ -264,7 +264,7 @@ func (h *Handler) readBodyOr400(w http.ResponseWriter, r *http.Request) (b []byt b, err = io.ReadAll(r.Body) _ = r.Body.Close() if err != nil { - writeError(w, http.StatusBadRequest, "failed to read request body") + WriteError(w, http.StatusBadRequest, "failed to read request body") return nil, false } return b, true @@ -296,16 +296,16 @@ func (h *Handler) HandleAppendChunk(w http.ResponseWriter, r *http.Request) { responseID := r.URL.Query().Get("response_id") if len(stagingID) > 64 { - writeError(w, http.StatusBadRequest, "invalid staging id") + WriteError(w, http.StatusBadRequest, "invalid staging id") return } if responseID != "" && len(responseID) > 255 { - writeError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") + WriteError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") return } k64, err := strconv.ParseUint(kStr, 10, 32) if err != nil { - writeError(w, http.StatusBadRequest, "invalid chunk number") + WriteError(w, http.StatusBadRequest, "invalid chunk number") return } k := uint32(k64) @@ -315,12 +315,12 @@ func (h *Handler) HandleAppendChunk(w http.ResponseWriter, r *http.Request) { r.Body = http.MaxBytesReader(w, r.Body, defaultMaxChunkBytes) chunkBody, err := io.ReadAll(r.Body) if err != nil { - writeError(w, http.StatusBadRequest, "failed to read chunk body") + WriteError(w, http.StatusBadRequest, "failed to read chunk body") return } _ = r.Body.Close() if len(chunkBody) == 0 { - writeError(w, http.StatusBadRequest, "empty chunk body") + WriteError(w, http.StatusBadRequest, "empty chunk body") return } @@ -346,7 +346,7 @@ func (h *Handler) HandleAppendChunk(w http.ResponseWriter, r *http.Request) { if k < nextExpected { status = http.StatusOK } - writeJSON(w, status, map[string]uint32{ + WriteJSON(w, status, map[string]uint32{ "received": k, "expected_next": nextExpected, }) @@ -364,20 +364,20 @@ func (h *Handler) HandleComplete(w http.ResponseWriter, r *http.Request) { tenantKey := r.Header.Get("X-Tenant-Key") if len(stagingID) > 64 { - writeError(w, http.StatusBadRequest, "invalid staging id") + WriteError(w, http.StatusBadRequest, "invalid staging id") return } if totalStr == "" { - writeError(w, http.StatusBadRequest, "missing total") + WriteError(w, http.StatusBadRequest, "missing total") return } total, err := strconv.ParseUint(totalStr, 10, 32) if err != nil { - writeError(w, http.StatusBadRequest, "invalid total") + WriteError(w, http.StatusBadRequest, "invalid total") return } if responseID != "" && len(responseID) > 255 { - writeError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") + WriteError(w, http.StatusBadRequest, "response_id exceeds 255 bytes") return } @@ -387,7 +387,7 @@ func (h *Handler) HandleComplete(w http.ResponseWriter, r *http.Request) { return } w.Header().Set("Location", "/responses/"+finalID) - writeJSON(w, http.StatusCreated, map[string]string{"response_id": finalID}) + WriteJSON(w, http.StatusCreated, map[string]string{"response_id": finalID}) } // HandleAbort handles PUT /staging//abort. @@ -396,7 +396,7 @@ func (h *Handler) HandleComplete(w http.ResponseWriter, r *http.Request) { func (h *Handler) HandleAbort(w http.ResponseWriter, r *http.Request) { stagingID := r.PathValue("id") if len(stagingID) > 64 { - writeError(w, http.StatusBadRequest, "invalid staging id") + WriteError(w, http.StatusBadRequest, "invalid staging id") return } if err := h.svc.AbortStaging(r.Context(), stagingID); err != nil { @@ -444,7 +444,7 @@ func (h *Handler) HandleDelete(w http.ResponseWriter, r *http.Request) { // HandleHealthz handles GET /healthz (liveness probe). func (h *Handler) HandleHealthz(w http.ResponseWriter, _ *http.Request) { - writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) + WriteJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } // HandleReadyz handles GET /readyz (readiness probe). @@ -452,8 +452,8 @@ func (h *Handler) HandleHealthz(w http.ResponseWriter, _ *http.Request) { func (h *Handler) HandleReadyz(w http.ResponseWriter, r *http.Request) { if err := h.svc.Ping(r.Context()); err != nil { h.log.Error("readyz: storage ping failed", "err", err) - writeJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "storage unavailable"}) + WriteJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "storage unavailable"}) return } - writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) + WriteJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } diff --git a/internal/server/httputil.go b/internal/server/httputil.go index 67dd88a..f06e75e 100644 --- a/internal/server/httputil.go +++ b/internal/server/httputil.go @@ -5,12 +5,14 @@ import ( "net/http" ) -func writeJSON(w http.ResponseWriter, status int, v any) { +// WriteJSON writes a JSON response with the given status code. +func WriteJSON(w http.ResponseWriter, status int, v any) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) _ = json.NewEncoder(w).Encode(v) } -func writeError(w http.ResponseWriter, status int, msg string) { - writeJSON(w, status, map[string]string{"error": msg}) +// WriteError writes a JSON error response. +func WriteError(w http.ResponseWriter, status int, msg string) { + WriteJSON(w, status, map[string]string{"error": msg}) } diff --git a/internal/server/metrics.go b/internal/server/metrics.go index 260cb51..1664b82 100644 --- a/internal/server/metrics.go +++ b/internal/server/metrics.go @@ -7,11 +7,11 @@ import ( ) var ( - httpRequestsTotal = prometheus.NewCounterVec( + requestsTotal = prometheus.NewCounterVec( prometheus.CounterOpts{Name: "http_requests_total"}, []string{"endpoint", "status"}, ) - httpRequestDuration = prometheus.NewHistogramVec( + requestDuration = prometheus.NewHistogramVec( prometheus.HistogramOpts{Name: "http_request_duration_seconds"}, []string{"endpoint"}, ) @@ -29,8 +29,8 @@ func RegisterMetrics(reg prometheus.Registerer, namespace string) error { } wrapped := prometheus.WrapRegistererWithPrefix(namespace+"_", reg) for _, c := range []prometheus.Collector{ - httpRequestsTotal, - httpRequestDuration, + requestsTotal, + requestDuration, } { if err := wrapped.Register(c); err != nil { var are prometheus.AlreadyRegisteredError diff --git a/internal/server/middleware.go b/internal/server/middleware.go index 828272a..9374f53 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -74,8 +74,8 @@ func RequestLogger(log *slog.Logger) Middleware { "duration_ms", dur.Milliseconds(), ) endpoint := r.Method + " " + r.Pattern - httpRequestsTotal.WithLabelValues(endpoint, strconv.Itoa(status)).Inc() - httpRequestDuration.WithLabelValues(endpoint).Observe(dur.Seconds()) + requestsTotal.WithLabelValues(endpoint, strconv.Itoa(status)).Inc() + requestDuration.WithLabelValues(endpoint).Observe(dur.Seconds()) }) } } diff --git a/internal/telemetry/options.go b/internal/telemetry/options.go new file mode 100644 index 0000000..4b0b627 --- /dev/null +++ b/internal/telemetry/options.go @@ -0,0 +1,14 @@ +package telemetry + +import "flag" + +// Options holds OpenTelemetry configuration for a single service. +type Options struct { + ExporterURL string // OTLP HTTP endpoint; empty = disabled + ServiceName string // identifies this binary in traces +} + +// AddFlags registers the --telemetry-exporter-url flag on fs. +func (o *Options) AddFlags(fs *flag.FlagSet) { + fs.StringVar(&o.ExporterURL, "telemetry-exporter-url", o.ExporterURL, "OTLP HTTP exporter URL (e.g. http://localhost:4318); empty disables tracing") +} From a81c8797f18e62d1ea2df53ff832b382824fabf4 Mon Sep 17 00:00:00 2001 From: Etai Lev Ran Date: Wed, 8 Jul 2026 20:15:20 +0300 Subject: [PATCH 4/4] test: inline testdata YAML as []byte; use table-driven tests Remove all testdata/*.yaml files from charonconfig and proxyconfig. Replace file references with inline []byte literals written to temp files via a configFromBytes helper. Expand strict-parse rejection tests into table-driven subtests covering unknown keys at each nesting level. Signed-off-by: Etai Lev Ran --- internal/charonconfig/bytesize_test.go | 16 ++--- internal/charonconfig/charonconfig_test.go | 58 ++++++++++++---- internal/charonconfig/testdata/bytesize.yaml | 3 - internal/charonconfig/testdata/config.yaml | 4 -- internal/charonconfig/testdata/invalid.yaml | 3 - internal/proxyconfig/proxyconfig_test.go | 73 ++++++++++++-------- internal/proxyconfig/testdata/config.yaml | 4 -- 7 files changed, 94 insertions(+), 67 deletions(-) delete mode 100644 internal/charonconfig/testdata/bytesize.yaml delete mode 100644 internal/charonconfig/testdata/config.yaml delete mode 100644 internal/charonconfig/testdata/invalid.yaml delete mode 100644 internal/proxyconfig/testdata/config.yaml diff --git a/internal/charonconfig/bytesize_test.go b/internal/charonconfig/bytesize_test.go index cb934e4..ff31622 100644 --- a/internal/charonconfig/bytesize_test.go +++ b/internal/charonconfig/bytesize_test.go @@ -11,13 +11,6 @@ import ( "github.com/elevran/charon/internal/charonconfig" ) -func unmarshalByteSize(t *testing.T, input string) charonconfig.ByteSize { - t.Helper() - var b charonconfig.ByteSize - require.NoError(t, json.Unmarshal([]byte(input), &b)) - return b -} - func TestByteSizeUnmarshal(t *testing.T) { tests := []struct { input string @@ -43,8 +36,9 @@ func TestByteSizeUnmarshal(t *testing.T) { for _, tc := range tests { t.Run(tc.input, func(t *testing.T) { - got := unmarshalByteSize(t, tc.input) - assert.Equal(t, charonconfig.ByteSize(tc.want), got) + var b charonconfig.ByteSize + require.NoError(t, json.Unmarshal([]byte(tc.input), &b)) + assert.Equal(t, charonconfig.ByteSize(tc.want), b) }) } } @@ -72,10 +66,12 @@ func TestByteSizeUnmarshalErrors(t *testing.T) { } func TestByteSizeInConfig(t *testing.T) { + yaml := []byte("charon:\n storage:\n max_payload: \"10MB\"\n") + cfg := configFromBytes(t, yaml) opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/bytesize.yaml"})) + require.NoError(t, fs.Parse([]string{"--config", cfg})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, charonconfig.ByteSize(10*1024*1024), opts.MaxPayload) } diff --git a/internal/charonconfig/charonconfig_test.go b/internal/charonconfig/charonconfig_test.go index a924873..a787531 100644 --- a/internal/charonconfig/charonconfig_test.go +++ b/internal/charonconfig/charonconfig_test.go @@ -13,6 +13,15 @@ import ( "github.com/elevran/charon/internal/charonconfig" ) +// configFromBytes writes yaml to a temp file and returns the path. +func configFromBytes(t *testing.T, yaml []byte) string { + t.Helper() + dir := t.TempDir() + p := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(p, yaml, 0600)) + return p +} + func TestCharonOptionsDefaultsWithNoConfig(t *testing.T) { opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) @@ -28,10 +37,17 @@ func TestCharonOptionsDefaultsWithNoConfig(t *testing.T) { } func TestCharonOptionsCompleteLoadsFile(t *testing.T) { + yaml := []byte(` +charon: + listen: ":9090" + storage: + data_dir: ./data +`) + cfg := configFromBytes(t, yaml) opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, fs.Parse([]string{"--config", cfg})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, ":9090", opts.Listen) @@ -39,10 +55,12 @@ func TestCharonOptionsCompleteLoadsFile(t *testing.T) { } func TestCharonOptionsCLIOverridesFile(t *testing.T) { + yaml := []byte("charon:\n listen: \":9090\"\n") + cfg := configFromBytes(t, yaml) opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":7777"})) + require.NoError(t, fs.Parse([]string{"--config", cfg, "--listen", ":7777"})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, ":7777", opts.Listen, "CLI flag must take precedence over config file") @@ -54,26 +72,36 @@ func TestCharonOptionsValidateOK(t *testing.T) { } func TestCharonOptionsDataDirCLIOverridesFile(t *testing.T) { - dir := t.TempDir() - cfgPath := filepath.Join(dir, "cfg.yaml") - err := os.WriteFile(cfgPath, []byte("charon:\n storage:\n data_dir: /tmp/test-data\n"), 0600) - require.NoError(t, err) - + yaml := []byte("charon:\n storage:\n data_dir: /tmp/test-data\n") + cfg := configFromBytes(t, yaml) opts := charonconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--storage-data-dir", "./my-data"})) + require.NoError(t, fs.Parse([]string{"--config", cfg, "--storage-data-dir", "./my-data"})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, "./my-data", opts.DataDir) } func TestLoadStrictRejectsUnknownFields(t *testing.T) { - charon := charonconfig.NewOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - charon.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/invalid.yaml"})) - err := charon.Complete(fs) - require.Error(t, err) - assert.Contains(t, err.Error(), "parse config") + tests := []struct { + name string + yaml []byte + }{ + {"top-level unknown key", []byte("unknown_field: true\n")}, + {"unknown key under charon", []byte("charon:\n not_a_field: true\n")}, + {"unknown key under storage", []byte("charon:\n storage:\n bad_key: 1\n")}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := configFromBytes(t, tc.yaml) + opts := charonconfig.NewOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", cfg})) + err := opts.Complete(fs) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse config") + }) + } } diff --git a/internal/charonconfig/testdata/bytesize.yaml b/internal/charonconfig/testdata/bytesize.yaml deleted file mode 100644 index ae0709a..0000000 --- a/internal/charonconfig/testdata/bytesize.yaml +++ /dev/null @@ -1,3 +0,0 @@ -charon: - storage: - max_payload: "10MB" diff --git a/internal/charonconfig/testdata/config.yaml b/internal/charonconfig/testdata/config.yaml deleted file mode 100644 index 565e809..0000000 --- a/internal/charonconfig/testdata/config.yaml +++ /dev/null @@ -1,4 +0,0 @@ -charon: - listen: ":9090" - storage: - data_dir: ./data diff --git a/internal/charonconfig/testdata/invalid.yaml b/internal/charonconfig/testdata/invalid.yaml deleted file mode 100644 index da10a6c..0000000 --- a/internal/charonconfig/testdata/invalid.yaml +++ /dev/null @@ -1,3 +0,0 @@ -server: - listen: ":8080" - unknown_field: true diff --git a/internal/proxyconfig/proxyconfig_test.go b/internal/proxyconfig/proxyconfig_test.go index a40a02c..e90fe0f 100644 --- a/internal/proxyconfig/proxyconfig_test.go +++ b/internal/proxyconfig/proxyconfig_test.go @@ -12,6 +12,15 @@ import ( "github.com/elevran/charon/internal/proxyconfig" ) +// configFromBytes writes yaml to a temp file and returns the path. +func configFromBytes(t *testing.T, yaml []byte) string { + t.Helper() + dir := t.TempDir() + p := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(p, yaml, 0600)) + return p +} + func TestProxyOptionsDefaultsWithNoConfig(t *testing.T) { opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) @@ -27,52 +36,50 @@ func TestProxyOptionsDefaultsWithNoConfig(t *testing.T) { } func TestProxyOptionsCompleteLoadsFile(t *testing.T) { + yaml := []byte("proxy:\n listen: \":0\"\ncharon:\n listen: \":9090\"\n") + cfg := configFromBytes(t, yaml) opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, fs.Parse([]string{"--config", cfg})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, ":0", opts.Listen) - // charon_url not set in testdata/config.yaml → auto-derived from charon.listen ":9090" + // charon_url not set → auto-derived from charon.listen ":9090" assert.Equal(t, "http://127.0.0.1:9090", opts.CharonURL) } func TestProxyOptionsCLIOverridesFile(t *testing.T) { + yaml := []byte("proxy:\n listen: \":0\"\n") + cfg := configFromBytes(t, yaml) opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml", "--listen", ":5555"})) + require.NoError(t, fs.Parse([]string{"--config", cfg, "--listen", ":5555"})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, ":5555", opts.Listen, "CLI flag must take precedence over config file") } func TestProxyOptionsBackendCLIOverridesFile(t *testing.T) { - dir := t.TempDir() - cfgPath := filepath.Join(dir, "cfg.yaml") - err := os.WriteFile(cfgPath, []byte("proxy:\n inference:\n base_url: http://file-backend:11434\n"), 0600) - require.NoError(t, err) - + yaml := []byte("proxy:\n inference:\n base_url: http://file-backend:11434\n") + cfg := configFromBytes(t, yaml) opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--backend", "http://cli-backend:11434"})) + require.NoError(t, fs.Parse([]string{"--config", cfg, "--backend", "http://cli-backend:11434"})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, "http://cli-backend:11434", opts.Backend, "CLI --backend must take precedence over config file") } func TestProxyOptionsCharonURLCLIOverridesFile(t *testing.T) { - dir := t.TempDir() - cfgPath := filepath.Join(dir, "cfg.yaml") - err := os.WriteFile(cfgPath, []byte("proxy:\n charon_url: http://file-charon:8081\n"), 0600) - require.NoError(t, err) - + yaml := []byte("proxy:\n charon_url: http://file-charon:8081\n") + cfg := configFromBytes(t, yaml) opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", cfgPath, "--charon-url", "http://cli-charon:9999"})) + require.NoError(t, fs.Parse([]string{"--config", cfg, "--charon-url", "http://cli-charon:9999"})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, "http://cli-charon:9999", opts.CharonURL, "CLI --charon-url must take precedence over config file") @@ -92,25 +99,35 @@ func TestProxyOptionsValidateEmptyBackendFails(t *testing.T) { } func TestCharonURLDerivedFromListen(t *testing.T) { + yaml := []byte("proxy:\n listen: \":0\"\ncharon:\n listen: \":9090\"\n") + cfg := configFromBytes(t, yaml) opts := proxyconfig.NewOptions() fs := flag.NewFlagSet("test", flag.ContinueOnError) opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", "testdata/config.yaml"})) + require.NoError(t, fs.Parse([]string{"--config", cfg})) require.NoError(t, opts.Complete(fs)) assert.Equal(t, "http://127.0.0.1:9090", opts.CharonURL) } func TestLoadStrictRejectsUnknownFields(t *testing.T) { - dir := t.TempDir() - cfgPath := filepath.Join(dir, "bad.yaml") - err := os.WriteFile(cfgPath, []byte("proxy:\n unknown_key: true\n"), 0600) - require.NoError(t, err) - - opts := proxyconfig.NewOptions() - fs := flag.NewFlagSet("test", flag.ContinueOnError) - opts.AddFlags(fs) - require.NoError(t, fs.Parse([]string{"--config", cfgPath})) - err = opts.Complete(fs) - require.Error(t, err) - assert.Contains(t, err.Error(), "parse config") + tests := []struct { + name string + yaml []byte + }{ + {"top-level unknown key", []byte("unknown_field: true\n")}, + {"unknown key under proxy", []byte("proxy:\n bad_key: 1\n")}, + {"unknown key under inference", []byte("proxy:\n inference:\n not_a_field: true\n")}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := configFromBytes(t, tc.yaml) + opts := proxyconfig.NewOptions() + fs := flag.NewFlagSet("test", flag.ContinueOnError) + opts.AddFlags(fs) + require.NoError(t, fs.Parse([]string{"--config", cfg})) + err := opts.Complete(fs) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse config") + }) + } } diff --git a/internal/proxyconfig/testdata/config.yaml b/internal/proxyconfig/testdata/config.yaml deleted file mode 100644 index 8f72fb8..0000000 --- a/internal/proxyconfig/testdata/config.yaml +++ /dev/null @@ -1,4 +0,0 @@ -proxy: - listen: ":0" -charon: - listen: ":9090"