From 81623bc90b844e8edc8d86352df517d7f3baf307 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:31:22 -0700 Subject: [PATCH 01/28] CaptureResponseWriter Signed-off-by: James Ranson --- pkg/proxy/response/capture/capture.go | 68 +++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 pkg/proxy/response/capture/capture.go diff --git a/pkg/proxy/response/capture/capture.go b/pkg/proxy/response/capture/capture.go new file mode 100644 index 000000000..b5ff0cb05 --- /dev/null +++ b/pkg/proxy/response/capture/capture.go @@ -0,0 +1,68 @@ +/* + * Copyright 2018 The Trickster Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package capture + +import ( + "bytes" + "net/http" +) + +// CaptureResponseWriter captures the response body to a byte slice +type CaptureResponseWriter struct { + http.ResponseWriter + header http.Header + statusCode int + body bytes.Buffer + len int +} + +// NewCaptureResponseWriter returns a new CaptureResponseWriter +func NewCaptureResponseWriter() *CaptureResponseWriter { + return &CaptureResponseWriter{ + header: make(http.Header), + } +} + +// Header returns the response header map +func (sw *CaptureResponseWriter) Header() http.Header { + return sw.header +} + +// WriteHeader sets the status code +func (sw *CaptureResponseWriter) WriteHeader(code int) { + if code == 0 { + code = 200 + } + sw.statusCode = code +} + +// Write appends data to the response body +func (sw *CaptureResponseWriter) Write(b []byte) (int, error) { + sw.body.Write(b) + sw.len += len(b) + return sw.len, nil +} + +// Body returns the captured response body +func (sw *CaptureResponseWriter) Body() []byte { + return sw.body.Bytes() +} + +// StatusCode returns the captured status code +func (sw *CaptureResponseWriter) StatusCode() int { + return sw.statusCode +} From 1fecafe537658cb3032d09d85b01ec61e8410e8e Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:32:18 -0700 Subject: [PATCH 02/28] move Target to own file, add HealthyTargets() Signed-off-by: James Ranson --- pkg/backends/alb/pool/pool.go | 79 +++++++++++++-------------------- pkg/backends/alb/pool/target.go | 75 +++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 49 deletions(-) create mode 100644 pkg/backends/alb/pool/target.go diff --git a/pkg/backends/alb/pool/pool.go b/pkg/backends/alb/pool/pool.go index 893736d18..266a594be 100644 --- a/pkg/backends/alb/pool/pool.go +++ b/pkg/backends/alb/pool/pool.go @@ -22,14 +22,14 @@ import ( "net/http" "sync" "sync/atomic" - - "github.com/trickstercache/trickster/v2/pkg/backends/healthcheck" ) // Pool defines the interface for a load balancer pool type Pool interface { - // Healthy returns the full list of Healthy Targets + // Healthy returns the full list of Healthy Targets as http.Handlers Healthy() []http.Handler + // HealthyTargets returns the full list of Healthy Targets as *Targets + HealthyTargets() Targets // SetHealthy sets the Healthy Targets List SetHealthy([]http.Handler) // Stop stops the pool and its health checker goroutines @@ -38,67 +38,48 @@ type Pool interface { RefreshHealthy() } -// Target defines an alb pool target -type Target struct { - hcStatus *healthcheck.Status - handler http.Handler -} - -// New returns a new Pool -func New(targets []*Target, healthyFloor int) Pool { - ctx, cancel := context.WithCancel(context.Background()) - p := &pool{ - targets: targets, - ctx: ctx, - stopper: cancel, - ch: make(chan bool, 16), - healthyFloor: healthyFloor, - } - p.ch <- true - - for _, t := range targets { - t.hcStatus.RegisterSubscriber(p.ch) - } - go p.checkHealth() - return p -} - -// NewTarget returns a new Target using the provided inputs -func NewTarget(handler http.Handler, hcStatus *healthcheck.Status) *Target { - return &Target{ - hcStatus: hcStatus, - handler: handler, - } -} - // pool implements Pool type pool struct { - targets []*Target - healthy atomic.Pointer[[]http.Handler] - healthyFloor int - ctx context.Context - stopper context.CancelFunc - ch chan bool - mtx sync.Mutex + targets Targets + healthyTargets atomic.Pointer[Targets] + healthyHandlers atomic.Pointer[[]http.Handler] + healthyFloor int + ctx context.Context + stopper context.CancelFunc + ch chan bool + mtx sync.Mutex } func (p *pool) RefreshHealthy() { p.mtx.Lock() defer p.mtx.Unlock() - h := make([]http.Handler, len(p.targets)) + hh := make([]http.Handler, len(p.targets)) + ht := make(Targets, len(p.targets)) + var k int for _, t := range p.targets { if int(t.hcStatus.Get()) >= p.healthyFloor { - h[k] = t.handler + hh[k] = t.handler + ht[k] = t k++ } } - h = h[:k] - p.healthy.Store(&h) + hh = hh[:k] + ht = ht[:k] + p.healthyHandlers.Store(&hh) + p.healthyTargets.Store(&ht) +} + +func (p *pool) HealthyTargets() Targets { + t := p.healthyTargets.Load() + if t != nil { + return *t + } + return nil } func (p *pool) Healthy() []http.Handler { - t := p.healthy.Load() + t := p.healthyHandlers.Load() if t != nil { return *t } @@ -106,7 +87,7 @@ func (p *pool) Healthy() []http.Handler { } func (p *pool) SetHealthy(h []http.Handler) { - p.healthy.Store(&h) + p.healthyHandlers.Store(&h) } func (p *pool) Stop() { diff --git a/pkg/backends/alb/pool/target.go b/pkg/backends/alb/pool/target.go new file mode 100644 index 000000000..11c8977f4 --- /dev/null +++ b/pkg/backends/alb/pool/target.go @@ -0,0 +1,75 @@ +/* + * Copyright 2018 The Trickster Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pool + +import ( + "context" + "net/http" + + "github.com/trickstercache/trickster/v2/pkg/backends" + "github.com/trickstercache/trickster/v2/pkg/backends/healthcheck" +) + +// Target defines an alb pool target +type Target struct { + hcStatus *healthcheck.Status + handler http.Handler + backend backends.Backend +} + +type Targets []*Target + +// New returns a new Pool +func New(targets Targets, healthyFloor int) Pool { + ctx, cancel := context.WithCancel(context.Background()) + p := &pool{ + targets: targets, + ctx: ctx, + stopper: cancel, + ch: make(chan bool, 16), + healthyFloor: healthyFloor, + } + p.ch <- true + + for _, t := range targets { + t.hcStatus.RegisterSubscriber(p.ch) + } + go p.checkHealth() + return p +} + +// NewTarget returns a new Target using the provided inputs +func NewTarget(handler http.Handler, hcStatus *healthcheck.Status, + backend backends.Backend) *Target { + return &Target{ + hcStatus: hcStatus, + handler: handler, + backend: backend, + } +} + +func (t *Target) HealthStatus() *healthcheck.Status { + return t.hcStatus +} + +func (t *Target) Handler() http.Handler { + return t.handler +} + +func (t *Target) Backend() backends.Backend { + return t.backend +} From 917bf95ff8225e1fc3303d691c801eccd59eeefb Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:34:33 -0700 Subject: [PATCH 03/28] refactor tsm Signed-off-by: James Ranson --- .../alb/mech/tsm/time_series_merge.go | 125 ++++++++------ .../alb/mech/tsm/time_series_merge_test.go | 2 +- pkg/backends/prometheus/handler_alerts.go | 3 +- .../prometheus/handler_alerts_test.go | 2 +- pkg/backends/prometheus/handler_labels.go | 5 +- .../prometheus/handler_labels_test.go | 2 +- pkg/backends/prometheus/handler_query.go | 48 +++++- .../prometheus/handler_query_range.go | 6 +- pkg/backends/prometheus/handler_series.go | 5 +- pkg/backends/prometheus/model/alerts.go | 156 +++--------------- pkg/backends/prometheus/model/labels.go | 30 ++-- pkg/backends/prometheus/model/merge.go | 98 +++++++++++ pkg/backends/prometheus/model/series.go | 40 +++-- pkg/backends/prometheus/model/vector.go | 64 ++----- pkg/backends/prometheus/transformations.go | 23 +-- pkg/proxy/response/merge/merge.go | 130 +++++++++------ pkg/proxy/response/merge/timeseries.go | 98 ++++++----- 17 files changed, 443 insertions(+), 394 deletions(-) create mode 100644 pkg/backends/prometheus/model/merge.go diff --git a/pkg/backends/alb/mech/tsm/time_series_merge.go b/pkg/backends/alb/mech/tsm/time_series_merge.go index 0bd91ca5f..f2a77470c 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge.go @@ -30,9 +30,11 @@ import ( "github.com/trickstercache/trickster/v2/pkg/backends/alb/pool" "github.com/trickstercache/trickster/v2/pkg/backends/providers" rt "github.com/trickstercache/trickster/v2/pkg/backends/providers/registry/types" + "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" "github.com/trickstercache/trickster/v2/pkg/proxy/handlers/trickster/failures" "github.com/trickstercache/trickster/v2/pkg/proxy/headers" "github.com/trickstercache/trickster/v2/pkg/proxy/request" + "github.com/trickstercache/trickster/v2/pkg/proxy/response/capture" "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" ) @@ -46,6 +48,7 @@ type handler struct { pool pool.Pool mergePaths []string // paths handled by the alb client that are enabled for tsmerge nonmergeHandler types.Mechanism // when methodology is tsmerge, this handler is for non-mergeable paths + outputFormat string // the provider output format (e.g., "prometheus") } func RegistryEntry() types.RegistryEntry { @@ -60,6 +63,7 @@ func New(o *options.Options, factories rt.Lookup) (types.Mechanism, error) { if !providers.IsSupportedTimeSeriesMergeProvider(o.OutputFormat) { return nil, errors.ErrInvalidTimeSeriesMergeProvider } + // next, get the factory function required to create a backend handler for the supplied format f, ok := factories[o.OutputFormat] if !ok { @@ -77,6 +81,7 @@ func New(o *options.Options, factories rt.Lookup) (types.Mechanism, error) { } // set the merge paths in the ALB client out.mergePaths = mc2.MergeablePaths() + out.outputFormat = o.OutputFormat return out, nil } @@ -99,19 +104,18 @@ func (h *handler) StopPool() { } } +func (h *handler) Pool() pool.Pool { + return h.pool +} + func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - hl := h.pool.Healthy() // should return a fanout list + hl := h.pool.HealthyTargets() // should return a fanout list l := len(hl) if l == 0 { failures.HandleBadGateway(w, r) return } - // just proxy 1:1 if no folds in the fan - if l == 1 { - hl[0].ServeHTTP(w, r) - return - } - + defaultHandler := hl[0].Handler() var isMergeablePath bool for _, v := range h.mergePaths { if strings.HasPrefix(r.URL.Path, v) { @@ -119,68 +123,81 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { break } } - if !isMergeablePath { - hl[0].ServeHTTP(w, r) + defaultHandler.ServeHTTP(w, r) return } - - mgs := GetResponseGates(w, r, hl) - if len(mgs) == 0 { - failures.HandleBadGateway(w, r) + // just proxy 1:1 if no folds in the fan + // there are now merge functions attached to the request + rsc := request.GetResources(r) + if rsc == nil || l == 1 { + defaultHandler.ServeHTTP(w, r) return } - SetStatusHeader(w, mgs) - rsc := request.GetResources(mgs[0].Request) - if rsc != nil && rsc.ResponseMergeFunc != nil { - if f, ok := rsc.ResponseMergeFunc.(func(http.ResponseWriter, - *http.Request, merge.ResponseGates)); ok { - f(w, r, mgs) - } - } -} + var mrf merge.RespondFunc -// GetResponseGates makes the handler request to each fanout backend and -// returns a collection of responses -func GetResponseGates(w http.ResponseWriter, r *http.Request, - hl []http.Handler, -) merge.ResponseGates { + // Scatter/Gather section + + // Perform streaming merge: each goroutine merges as soon as response arrives + accumulator := merge.NewAccumulator() var wg sync.WaitGroup - l := len(hl) - mgs := make(merge.ResponseGates, l) + var statusCode int + var statusHeader string + var mu sync.Mutex // protects statusCode and statusHeader + for i := range l { + if hl[i] == nil { + continue + } wg.Go(func() { - if hl[i] == nil { - return - } r2, _ := request.Clone(r) - rsc := request.GetResources(r2) - rsc.IsMergeMember = true - mgs[i] = merge.NewResponseGate(w, r2, rsc) - hl[i].ServeHTTP(mgs[i], r2) + rsc2 := &request.Resources{IsMergeMember: true} + r2 = request.SetResources(r2, rsc2) + + crw := capture.NewCaptureResponseWriter() + hl[i].Handler().ServeHTTP(crw, r2) + // Ensure merge functions are set on cloned request + if rsc2.MergeFunc == nil || rsc2.MergeRespondFunc == nil { + logger.Warn("tsm gather failed due to nil func", nil) + } + // As soon as response is complete, unmarshal and merge + // This happens in parallel for each response as it arrives + if rsc2.MergeFunc != nil { + rsc2.MergeFunc(accumulator, rsc2.TS, i) + } + // Update status code and headers (take best status code) + mu.Lock() + if mrf == nil { + mrf = rsc2.MergeRespondFunc + } + if crw.StatusCode() > 0 { + if statusCode == 0 || crw.StatusCode() < statusCode { + statusCode = crw.StatusCode() + } + } + if crw.Header() != nil { + headers.StripMergeHeaders(crw.Header()) + statusHeader = headers.MergeResultHeaderVals(statusHeader, + crw.Header().Get(headers.NameTricksterResult)) + } + mu.Unlock() }) } + + // Wait for all fanout requests to complete wg.Wait() - return mgs.Compress() -} -// SetStatusHeader inspects the X-Trickster-Result header value crafted for each mergeable response -// and aggregates into a single header value for the primary merged response -func SetStatusHeader(w http.ResponseWriter, mgs merge.ResponseGates) { - var statusHeader string - for _, mg := range mgs { - if mg == nil { - continue - } - if h := mg.Header(); h != nil { - headers.StripMergeHeaders(h) - statusHeader = headers.MergeResultHeaderVals(statusHeader, - h.Get(headers.NameTricksterResult)) - } - } + // Set aggregated status header if statusHeader != "" { - h := w.Header() - h.Set(headers.NameTricksterResult, statusHeader) + w.Header().Set(headers.NameTricksterResult, statusHeader) + } + + // Marshal and write the merged series to the client + if statusCode == 0 { + statusCode = http.StatusOK + } + if mrf != nil { + mrf(w, r, accumulator, statusCode) } } diff --git a/pkg/backends/alb/mech/tsm/time_series_merge_test.go b/pkg/backends/alb/mech/tsm/time_series_merge_test.go index 240c67815..e2a69277c 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge_test.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge_test.go @@ -40,7 +40,7 @@ func TestHandleResponseMerge(t *testing.T) { logger.SetLogger(testLogger) r, _ := http.NewRequest("GET", "http://trickstercache.org/", nil) rsc := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc.ResponseMergeFunc = testMergeFunc + rsc.MergeFunc = testMergeFunc rsc.IsMergeMember = true r = request.SetResources(r, rsc) diff --git a/pkg/backends/prometheus/handler_alerts.go b/pkg/backends/prometheus/handler_alerts.go index 333e31ce6..7b0740e56 100644 --- a/pkg/backends/prometheus/handler_alerts.go +++ b/pkg/backends/prometheus/handler_alerts.go @@ -31,7 +31,8 @@ func (c *Client) AlertsHandler(w http.ResponseWriter, r *http.Request) { r.URL = urls.BuildUpstreamURL(r, c.BaseUpstreamURL()) resp := engines.DoProxy(w, r, true) if rsc != nil && rsc.IsMergeMember { - rsc.ResponseMergeFunc = model.MergeAndWriteAlerts + rsc.MergeFunc = model.MergeAndWriteAlertsMergeFunc() + rsc.MergeRespondFunc = model.MergeAndWriteAlertsRespondFunc() rsc.Response = resp } } diff --git a/pkg/backends/prometheus/handler_alerts_test.go b/pkg/backends/prometheus/handler_alerts_test.go index 5df21a5ca..4962c3cbe 100644 --- a/pkg/backends/prometheus/handler_alerts_test.go +++ b/pkg/backends/prometheus/handler_alerts_test.go @@ -48,7 +48,7 @@ func TestAlertsHandler(t *testing.T) { client.AlertsHandler(w, r) - if rsc.ResponseMergeFunc == nil { + if rsc.MergeFunc == nil { t.Error("expected non-nil func value") } } diff --git a/pkg/backends/prometheus/handler_labels.go b/pkg/backends/prometheus/handler_labels.go index 6c521a1a4..ca1384c67 100644 --- a/pkg/backends/prometheus/handler_labels.go +++ b/pkg/backends/prometheus/handler_labels.go @@ -31,8 +31,9 @@ func (c *Client) LabelsHandler(w http.ResponseWriter, r *http.Request) { u := urls.BuildUpstreamURL(r, c.BaseUpstreamURL()) rsc := request.GetResources(r) - if rsc.IsMergeMember { - rsc.ResponseMergeFunc = model.MergeAndWriteLabelData + if rsc != nil && rsc.IsMergeMember { + rsc.MergeFunc = model.MergeAndWriteLabelDataMergeFunc() + rsc.MergeRespondFunc = model.MergeAndWriteLabelDataRespondFunc() } qp, _, _ := params.GetRequestValues(r) diff --git a/pkg/backends/prometheus/handler_labels_test.go b/pkg/backends/prometheus/handler_labels_test.go index 3f8f65990..db9b343b8 100644 --- a/pkg/backends/prometheus/handler_labels_test.go +++ b/pkg/backends/prometheus/handler_labels_test.go @@ -50,7 +50,7 @@ func TestLabelsHandler(t *testing.T) { client.LabelsHandler(w, r) - if rsc.ResponseMergeFunc == nil { + if rsc.MergeFunc == nil { t.Error("expected non-nil func value") } } diff --git a/pkg/backends/prometheus/handler_query.go b/pkg/backends/prometheus/handler_query.go index d59c25099..085594cc6 100644 --- a/pkg/backends/prometheus/handler_query.go +++ b/pkg/backends/prometheus/handler_query.go @@ -25,7 +25,6 @@ import ( "github.com/trickstercache/trickster/v2/pkg/proxy/engines" "github.com/trickstercache/trickster/v2/pkg/proxy/params" "github.com/trickstercache/trickster/v2/pkg/proxy/request" - "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" "github.com/trickstercache/trickster/v2/pkg/proxy/urls" "github.com/trickstercache/trickster/v2/pkg/timeseries" ) @@ -48,7 +47,11 @@ func (c *Client) QueryHandler(w http.ResponseWriter, r *http.Request) { rsc.TimeRangeQuery = trq } if rsc.IsMergeMember { - rsc.ResponseMergeFunc = model.MergeAndWriteVector + m := c.Modeler() + if m != nil { + rsc.MergeFunc = model.MergeAndWriteVectorMergeFunc(m.WireUnmarshaler) + rsc.MergeRespondFunc = model.MergeAndWriteVectorRespondFunc(m.WireMarshalWriter) + } } } if c.hasTransformations { @@ -69,13 +72,44 @@ func (c *Client) QueryHandler(w http.ResponseWriter, r *http.Request) { // if there are labels to append to the dataset, if c.hasTransformations { - // using a merge response gate allows the capturing of the response body for transformation - mg := merge.NewResponseGate(w, r, rsc) - engines.ObjectProxyCacheRequest(mg, r) - mg.Response = rsc.Response - c.processVectorTransformations(w, mg) + // use a streaming response writer to capture the response body for transformation + sw := &transformationResponseWriter{ + ResponseWriter: w, + header: make(http.Header), + body: make([]byte, 0), + } + engines.ObjectProxyCacheRequest(sw, r) + statusCode := sw.statusCode + if statusCode == 0 { + statusCode = http.StatusOK + } + if rsc != nil && rsc.Response != nil { + statusCode = rsc.Response.StatusCode + } + c.processVectorTransformations(w, sw.body, statusCode, rsc) return } engines.ObjectProxyCacheRequest(w, r) } + +// transformationResponseWriter captures the response for transformations +type transformationResponseWriter struct { + http.ResponseWriter + header http.Header + statusCode int + body []byte +} + +func (tw *transformationResponseWriter) Header() http.Header { + return tw.header +} + +func (tw *transformationResponseWriter) WriteHeader(code int) { + tw.statusCode = code +} + +func (tw *transformationResponseWriter) Write(b []byte) (int, error) { + tw.body = append(tw.body, b...) + return len(b), nil +} diff --git a/pkg/backends/prometheus/handler_query_range.go b/pkg/backends/prometheus/handler_query_range.go index 0a13301ba..810546f4a 100644 --- a/pkg/backends/prometheus/handler_query_range.go +++ b/pkg/backends/prometheus/handler_query_range.go @@ -35,7 +35,11 @@ func (c *Client) QueryRangeHandler(w http.ResponseWriter, r *http.Request) { rsc.TSTransformer = c.ProcessTransformations } if rsc.IsMergeMember { - rsc.ResponseMergeFunc = merge.Timeseries + m := c.Modeler() + if m != nil { + rsc.MergeFunc = merge.TimeseriesMergeFunc(m.WireUnmarshaler) + rsc.MergeRespondFunc = merge.TimeseriesRespondFunc(m.WireMarshalWriter, rsc.TSReqestOptions) + } } } r.URL = urls.BuildUpstreamURL(r, c.BaseUpstreamURL()) diff --git a/pkg/backends/prometheus/handler_series.go b/pkg/backends/prometheus/handler_series.go index 0e18ff3de..4bb8f8d26 100644 --- a/pkg/backends/prometheus/handler_series.go +++ b/pkg/backends/prometheus/handler_series.go @@ -30,8 +30,9 @@ import ( func (c *Client) SeriesHandler(w http.ResponseWriter, r *http.Request) { // if this request is part of a scatter/gather, provide a reconstitution function rsc := request.GetResources(r) - if rsc.IsMergeMember { - rsc.ResponseMergeFunc = model.MergeAndWriteSeries + if rsc != nil && rsc.IsMergeMember { + rsc.MergeFunc = model.MergeAndWriteSeriesMergeFunc() + rsc.MergeRespondFunc = model.MergeAndWriteSeriesRespondFunc() } u := urls.BuildUpstreamURL(r, c.BaseUpstreamURL()) diff --git a/pkg/backends/prometheus/model/alerts.go b/pkg/backends/prometheus/model/alerts.go index 967f74c4d..4964495e9 100644 --- a/pkg/backends/prometheus/model/alerts.go +++ b/pkg/backends/prometheus/model/alerts.go @@ -17,21 +17,12 @@ package model import ( - "bytes" - "encoding/json" "fmt" - "io" "maps" "net/http" "slices" - "sort" - "github.com/trickstercache/trickster/v2/pkg/backends/providers" "github.com/trickstercache/trickster/v2/pkg/checksum/fnv" - "github.com/trickstercache/trickster/v2/pkg/observability/logging" - "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" - "github.com/trickstercache/trickster/v2/pkg/proxy/handlers/trickster/failures" - "github.com/trickstercache/trickster/v2/pkg/proxy/headers" "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" "github.com/trickstercache/trickster/v2/pkg/timeseries/dataset" ) @@ -98,132 +89,39 @@ func (a *WFAlerts) Merge(results ...*WFAlerts) { a.Data.Alerts = alerts } -// Mergeable represents types that can merge with other instances of the same type -type Mergeable[T any] interface { - *T - Merge(...*T) -} - -// MarshallerPtr represents pointer types that can start marshaling with an envelope -type MarshallerPtr[T any] interface { - *T - StartMarshal(w io.Writer, httpStatus int) -} - -// unmarshalAndMerge is a generic function that handles the common pattern of -// unmarshaling JSON responses and merging them using a Merge method -func unmarshalAndMerge[T any, PT Mergeable[T]]( - r *http.Request, - rgs merge.ResponseGates, - errorType string, - newInstance func() PT, -) (PT, []int, *http.Response) { - var result PT - responses, bestResp := gatherResponses(r, rgs, func(rg *merge.ResponseGate) bool { - instance := newInstance() - err := json.Unmarshal(rg.Body(), instance) - if err != nil { - logger.Error(errorType+" unmarshaling error", - logging.Pairs{"provider": providers.Prometheus, "detail": err.Error()}) - return false - } - if result == nil { - result = instance - } else { - result.Merge(instance) - } - return true - }) - return result, responses, bestResp -} - -// handleMergeResult handles the common error pattern and starts marshaling if successful -func handleMergeResult[T any, PT MarshallerPtr[T]]( - w http.ResponseWriter, - r *http.Request, - result PT, - responses []int, - bestResp *http.Response, -) bool { - if result == nil || len(responses) == 0 { - if bestResp != nil { - h := w.Header() - headers.Merge(h, bestResp.Header) - w.WriteHeader(bestResp.StatusCode) - io.Copy(w, bestResp.Body) - } else { - failures.HandleBadGateway(w, r) - } - return false - } - - sort.Ints(responses) - statusCode := responses[0] - result.StartMarshal(w, statusCode) - return true -} - -// MergeAndWriteAlerts merges the provided Responses into a single prometheus Alerts data object, -// and writes it to the provided ResponseWriter -func MergeAndWriteAlerts(w http.ResponseWriter, r *http.Request, rgs merge.ResponseGates) { - a, responses, bestResp := unmarshalAndMerge(r, rgs, "alerts", func() *WFAlerts { +// MergeAndWriteAlertsMergeFunc returns a MergeFunc for WFAlerts +func MergeAndWriteAlertsMergeFunc() merge.MergeFunc { + return MakeMergeFunc("alerts", func() *WFAlerts { return &WFAlerts{} }) - - if !handleMergeResult(w, r, a, responses, bestResp) { - return - } - - var sep string - w.Write([]byte(`,"data":{"alerts":[`)) - if a.Data != nil && len(a.Data.Alerts) > 0 { - for _, alert := range a.Data.Alerts { - fmt.Fprintf(w, - `{"state":"%s","labels":%s,"annotations":%s`, - alert.State, dataset.Tags(alert.Labels).JSON(), - dataset.Tags(alert.Annotations).JSON(), - ) - if alert.Value != "" { - fmt.Fprintf(w, `,"value":"%s"`, alert.Value) - } - if alert.ActiveAt != "" { - fmt.Fprintf(w, `,"activeAt":"%s"`, alert.ActiveAt) - } - w.Write([]byte("}" + sep)) - sep = "," - } - } - w.Write([]byte("]}}")) // complete the alert list and the envelope } -// helper function to gather responses from a ResponseGate -func gatherResponses(_ *http.Request, rgs merge.ResponseGates, handler func(*merge.ResponseGate) bool) ([]int, *http.Response) { - responses := make([]int, len(rgs)) - var bestResp *http.Response - for i, rg := range rgs { - if rg == nil { - continue +// MergeAndWriteAlertsRespondFunc returns a RespondFunc for WFAlerts +func MergeAndWriteAlertsRespondFunc() merge.RespondFunc { + return MakeRespondFunc(func(w http.ResponseWriter, r *http.Request, a *WFAlerts, statusCode int) { + if a == nil { + return } - if rg.Resources != nil && rg.Resources.Response != nil { - resp := rg.Resources.Response - responses[i] = resp.StatusCode - - if resp.Body != nil { - defer resp.Body.Close() - } - - if resp.StatusCode < 400 { - ok := handler(rg) - if !ok { - continue + a.StartMarshal(w, statusCode) + var sep string + w.Write([]byte(`,"data":{"alerts":[`)) + if a.Data != nil && len(a.Data.Alerts) > 0 { + for _, alert := range a.Data.Alerts { + fmt.Fprintf(w, + `{"state":"%s","labels":%s,"annotations":%s`, + alert.State, dataset.Tags(alert.Labels).JSON(), + dataset.Tags(alert.Annotations).JSON(), + ) + if alert.Value != "" { + fmt.Fprintf(w, `,"value":"%s"`, alert.Value) } - } - if bestResp == nil || resp.StatusCode < bestResp.StatusCode { - bestResp = resp - resp.Body = io.NopCloser(bytes.NewReader(rg.Body())) + if alert.ActiveAt != "" { + fmt.Fprintf(w, `,"activeAt":"%s"`, alert.ActiveAt) + } + w.Write([]byte("}" + sep)) + sep = "," } } - } - - return responses, bestResp + w.Write([]byte("]}}")) // complete the alert list and the envelope + }) } diff --git a/pkg/backends/prometheus/model/labels.go b/pkg/backends/prometheus/model/labels.go index 58e412ae2..1a21d6517 100644 --- a/pkg/backends/prometheus/model/labels.go +++ b/pkg/backends/prometheus/model/labels.go @@ -48,20 +48,24 @@ func (ld *WFLabelData) Merge(results ...*WFLabelData) { } } -// MergeAndWriteLabelData merges the provided Responses into a single prometheus basic data object, -// and writes it to the provided ResponseWriter -func MergeAndWriteLabelData(w http.ResponseWriter, r *http.Request, rgs merge.ResponseGates) { - ld, responses, bestResp := unmarshalAndMerge(r, rgs, "labels", func() *WFLabelData { +// MergeAndWriteLabelDataMergeFunc returns a MergeFunc for WFLabelData +func MergeAndWriteLabelDataMergeFunc() merge.MergeFunc { + return MakeMergeFunc("labels", func() *WFLabelData { return &WFLabelData{} }) +} - if !handleMergeResult(w, r, ld, responses, bestResp) { - return - } - - if len(ld.Data) > 0 { - sort.Strings(ld.Data) - fmt.Fprintf(w, `,"data":["%s"]`, strings.Join(ld.Data, `","`)) - } - w.Write([]byte("}")) +// MergeAndWriteLabelDataRespondFunc returns a RespondFunc for WFLabelData +func MergeAndWriteLabelDataRespondFunc() merge.RespondFunc { + return MakeRespondFunc(func(w http.ResponseWriter, r *http.Request, ld *WFLabelData, statusCode int) { + if ld == nil { + return + } + ld.StartMarshal(w, statusCode) + if len(ld.Data) > 0 { + sort.Strings(ld.Data) + fmt.Fprintf(w, `,"data":["%s"]`, strings.Join(ld.Data, `","`)) + } + w.Write([]byte("}")) + }) } diff --git a/pkg/backends/prometheus/model/merge.go b/pkg/backends/prometheus/model/merge.go new file mode 100644 index 000000000..71612f13f --- /dev/null +++ b/pkg/backends/prometheus/model/merge.go @@ -0,0 +1,98 @@ +/* + * Copyright 2018 The Trickster Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package model + +import ( + "encoding/json" + "net/http" + + "github.com/trickstercache/trickster/v2/pkg/backends/providers" + "github.com/trickstercache/trickster/v2/pkg/observability/logging" + "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" + "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" +) + +// MakeMergeFunc creates a MergeFunc for a specific type that implements Merge +// The returned function accepts a type conforming to Mergeable[T] and merges it into the accumulator +func MakeMergeFunc[T any, PT merge.Mergeable[T]](errorType string, + newInstance func() PT) merge.MergeFunc { + return func(accum *merge.Accumulator, data any, idx int) error { + var instance PT + // Try to type assert to PT first + if pt, ok := data.(PT); ok { + instance = pt + } else if body, ok := data.([]byte); ok { + // If data is []byte, unmarshal it first (for backward compatibility during transition) + instance = newInstance() + if err := json.Unmarshal(body, instance); err != nil { + logger.Error(errorType+" unmarshaling error", + logging.Pairs{"provider": providers.Prometheus, "detail": err.Error()}) + return err + } + } else { + // Not the expected type and not []byte + return nil + } + accum.Lock() + defer accum.Unlock() + existing := accum.GetGenericUnsafe() + if existing == nil { + accum.SetGenericUnsafe(instance) + } else { + // Type assert to call Merge method + if existingPT, ok := existing.(PT); ok { + existingPT.Merge(instance) + } else { + // If type assertion fails, replace (shouldn't happen in practice) + accum.SetGenericUnsafe(instance) + } + } + return nil + } +} + +// MakeMergeFuncFromBytes creates a MergeFunc that accepts []byte and unmarshals it +// This is a convenience function for call sites that still have []byte +func MakeMergeFuncFromBytes[T any, PT merge.Mergeable[T]](errorType string, + newInstance func() PT) func(*merge.Accumulator, []byte, int) error { + mergeFunc := MakeMergeFunc(errorType, newInstance) + return func(accum *merge.Accumulator, body []byte, idx int) error { + instance := newInstance() + if err := json.Unmarshal(body, instance); err != nil { + logger.Error(errorType+" unmarshaling error", + logging.Pairs{"provider": providers.Prometheus, "detail": err.Error()}) + return err + } + return mergeFunc(accum, instance, idx) + } +} + +// MakeRespondFunc creates a RespondFunc for a specific type that implements MarshallerPtr +func MakeRespondFunc[T any, PT merge.MarshallerPtr[T]]( + handleResult func(http.ResponseWriter, *http.Request, PT, int), +) merge.RespondFunc { + return func(w http.ResponseWriter, r *http.Request, + accum *merge.Accumulator, statusCode int) { + generic := accum.GetGeneric() + if generic == nil { + return + } + if result, ok := generic.(PT); ok { + handleResult(w, r, result, statusCode) + } + } +} diff --git a/pkg/backends/prometheus/model/series.go b/pkg/backends/prometheus/model/series.go index 47ec03a45..f45ebc8f3 100644 --- a/pkg/backends/prometheus/model/series.go +++ b/pkg/backends/prometheus/model/series.go @@ -52,26 +52,30 @@ func (s *WFSeries) Merge(results ...*WFSeries) { } } -// MergeAndWriteSeries merges the provided Responses into a single prometheus Series data object, -// and writes it to the provided ResponseWriter -func MergeAndWriteSeries(w http.ResponseWriter, r *http.Request, rgs merge.ResponseGates) { - s, responses, bestResp := unmarshalAndMerge(r, rgs, "series", func() *WFSeries { +// MergeAndWriteSeriesMergeFunc returns a MergeFunc for WFSeries +func MergeAndWriteSeriesMergeFunc() merge.MergeFunc { + return MakeMergeFunc("series", func() *WFSeries { return &WFSeries{} }) +} - if !handleMergeResult(w, r, s, responses, bestResp) { - return - } - - var sep string - if len(s.Data) > 0 { - w.Write([]byte(`,"data":[`)) - for _, series := range s.Data { - fmt.Fprintf(w, `%s{"__name__":"%s","instance":"%s","job":"%s"}`, - sep, series.Name, series.Instance, series.Job) - sep = "," +// MergeAndWriteSeriesRespondFunc returns a RespondFunc for WFSeries +func MergeAndWriteSeriesRespondFunc() merge.RespondFunc { + return MakeRespondFunc(func(w http.ResponseWriter, r *http.Request, s *WFSeries, statusCode int) { + if s == nil { + return } - w.Write([]byte("]")) - } - w.Write([]byte("}")) // complete the envelope + s.StartMarshal(w, statusCode) + var sep string + if len(s.Data) > 0 { + w.Write([]byte(`,"data":[`)) + for _, series := range s.Data { + fmt.Fprintf(w, `%s{"__name__":"%s","instance":"%s","job":"%s"}`, + sep, series.Name, series.Instance, series.Job) + sep = "," + } + w.Write([]byte("]")) + } + w.Write([]byte("}")) // complete the envelope + }) } diff --git a/pkg/backends/prometheus/model/vector.go b/pkg/backends/prometheus/model/vector.go index b7eebcbf5..1a3f2fa2c 100644 --- a/pkg/backends/prometheus/model/vector.go +++ b/pkg/backends/prometheus/model/vector.go @@ -17,67 +17,31 @@ package model import ( - "io" "net/http" - "github.com/trickstercache/trickster/v2/pkg/backends/providers" - "github.com/trickstercache/trickster/v2/pkg/observability/logging" - "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" "github.com/trickstercache/trickster/v2/pkg/proxy/handlers/trickster/failures" - "github.com/trickstercache/trickster/v2/pkg/proxy/headers" "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" "github.com/trickstercache/trickster/v2/pkg/timeseries" - "github.com/trickstercache/trickster/v2/pkg/timeseries/dataset" ) -// MergeAndWriteVector merges the provided Responses into a single prometheus Vector data object, -// and writes it to the provided ResponseWriter -func MergeAndWriteVector(w http.ResponseWriter, r *http.Request, rgs merge.ResponseGates) { - var ts *dataset.DataSet - var trq *timeseries.TimeRangeQuery - - responses, bestResp := gatherResponses(r, rgs, func(rg *merge.ResponseGate) bool { - if rg.Resources.TimeRangeQuery != nil { - trq = rg.Resources.TimeRangeQuery - } - - t2, err := UnmarshalTimeseries(rg.Body(), trq) - if err != nil { - logger.Error("vector unmarshaling error", - logging.Pairs{"provider": providers.Prometheus, "detail": err.Error()}) - return false - } +// MergeAndWriteVectorMergeFunc returns a MergeFunc for Vector (timeseries) +func MergeAndWriteVectorMergeFunc(unmarshaler timeseries.UnmarshalerFunc) merge.MergeFunc { + return merge.TimeseriesMergeFunc(unmarshaler) +} +// MergeAndWriteVectorRespondFunc returns a RespondFunc for Vector (timeseries) +func MergeAndWriteVectorRespondFunc(marshaler timeseries.MarshalWriterFunc) merge.RespondFunc { + return func(w http.ResponseWriter, r *http.Request, accum *merge.Accumulator, statusCode int) { + ts := accum.GetTSData() if ts == nil { - ds, ok := t2.(*dataset.DataSet) - if !ok { - logger.Error("vector unmarshaling error", - logging.Pairs{"provider": providers.Prometheus}) - return false - } - ts = ds - } else { - ts.Merge(false, t2) + failures.HandleBadGateway(w, r) + return } - return true - }) - if len(responses) == 0 { - failures.HandleBadGateway(w, r) - return - } - - if ts == nil { - if bestResp != nil { - h := w.Header() - headers.Merge(h, bestResp.Header) - w.WriteHeader(bestResp.StatusCode) - io.Copy(w, bestResp.Body) - } else { - failures.HandleBadGateway(w, r) + if statusCode == 0 { + statusCode = http.StatusOK } - return - } - MarshalTSOrVectorWriter(ts, nil, bestResp.StatusCode, w, true) //revive:disable:unhandled-error + MarshalTSOrVectorWriter(ts, nil, statusCode, w, true) //revive:disable:unhandled-error + } } diff --git a/pkg/backends/prometheus/transformations.go b/pkg/backends/prometheus/transformations.go index c07efb7a1..e3ca9794f 100644 --- a/pkg/backends/prometheus/transformations.go +++ b/pkg/backends/prometheus/transformations.go @@ -23,8 +23,7 @@ import ( "github.com/trickstercache/trickster/v2/pkg/backends/providers" "github.com/trickstercache/trickster/v2/pkg/observability/logging" "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" - "github.com/trickstercache/trickster/v2/pkg/proxy/headers" - "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" + "github.com/trickstercache/trickster/v2/pkg/proxy/request" "github.com/trickstercache/trickster/v2/pkg/timeseries" "github.com/trickstercache/trickster/v2/pkg/timeseries/dataset" ) @@ -40,24 +39,26 @@ func (c *Client) ProcessTransformations(ts timeseries.Timeseries) { ds.InjectTags(c.injectLabels) } -func (c *Client) processVectorTransformations(w http.ResponseWriter, rg *merge.ResponseGate) { +func (c *Client) processVectorTransformations(w http.ResponseWriter, + body []byte, statusCode int, rsc *request.Resources) { var trq *timeseries.TimeRangeQuery - if rg.Resources.TimeRangeQuery != nil { - trq = rg.Resources.TimeRangeQuery + if rsc != nil && rsc.TimeRangeQuery != nil { + trq = rsc.TimeRangeQuery } - bytes := rg.Body() - h := w.Header() - headers.Merge(h, rg.Header()) - t2, err := model.UnmarshalTimeseries(bytes, trq) + t2, err := model.UnmarshalTimeseries(body, trq) if err != nil || t2 == nil { logger.Error("vector unmarshaling error", logging.Pairs{"provider": providers.Prometheus, "detail": err.Error()}) - defaultWrite(rg.Response.StatusCode, w, bytes) + defaultWrite(statusCode, w, body) return } ds := t2.(*dataset.DataSet) // failure of this type assertion should be impossible ds.InjectTags(c.injectLabels) - model.MarshalTSOrVectorWriter(ds, rg.Resources.TSReqestOptions, rg.Response.StatusCode, w, true) + var requestOptions *timeseries.RequestOptions + if rsc != nil { + requestOptions = rsc.TSReqestOptions + } + model.MarshalTSOrVectorWriter(ds, requestOptions, statusCode, w, true) } func defaultWrite(statusCode int, w http.ResponseWriter, b []byte) { diff --git a/pkg/proxy/response/merge/merge.go b/pkg/proxy/response/merge/merge.go index f82a788cc..dd1971caf 100644 --- a/pkg/proxy/response/merge/merge.go +++ b/pkg/proxy/response/merge/merge.go @@ -17,74 +17,98 @@ package merge import ( - stdbytes "bytes" + "io" "net/http" + "sync" - "github.com/trickstercache/trickster/v2/pkg/proxy/request" - "github.com/trickstercache/trickster/v2/pkg/util/bytes" + "github.com/trickstercache/trickster/v2/pkg/timeseries" ) -// ResponseGate is a Request/ResponseWriter Pair that must be handled in its entirety -// before its respective response pool can be merged. -type ResponseGate struct { - http.ResponseWriter - Request *http.Request - Response *http.Response - Resources *request.Resources - body []byte - header http.Header +// MergeFunc is a function type that merges unmarshaled data into an accumulator +// It takes the accumulator, unmarshaled data (either timeseries.Timeseries or a type +// conforming to Mergeable[T]), and index, and returns an error +type MergeFunc func(*Accumulator, any, int) error + +// RespondFunc is a function type that writes the merged result to the response writer +// It takes the response writer, request, accumulator, and status code +type RespondFunc func(http.ResponseWriter, *http.Request, *Accumulator, int) + +type MergeFuncPair struct { + Merge MergeFunc + Respond RespondFunc +} + +type MergeFuncLookup map[string]*MergeFuncPair + +// Mergeable represents types that can merge with other instances of the same type +type Mergeable[T any] interface { + *T + Merge(...*T) +} + +// MarshallerPtr represents pointer types that can start marshaling with an envelope +type MarshallerPtr[T any] interface { + *T + StartMarshal(w io.Writer, httpStatus int) } -// ResponseGates represents a slice of type *ResponseGate -type ResponseGates []*ResponseGate +// Accumulator is a thread-safe accumulator for merging data +// It can hold either timeseries.Timeseries or any other mergeable type +type Accumulator struct { + mu sync.Mutex + tsdata timeseries.Timeseries + generic any // For non-timeseries mergeable types +} + +// NewAccumulator returns a new Accumulator +func NewAccumulator() *Accumulator { + return &Accumulator{} +} + +// GetTSData returns the accumulated timeseries data (thread-safe) +func (a *Accumulator) GetTSData() timeseries.Timeseries { + a.mu.Lock() + defer a.mu.Unlock() + return a.tsdata +} + +// SetTSData sets the accumulated timeseries data (thread-safe) +func (a *Accumulator) SetTSData(data timeseries.Timeseries) { + a.mu.Lock() + defer a.mu.Unlock() + a.tsdata = data +} -// NewResponseGate provides a new ResponseGate object -func NewResponseGate(w http.ResponseWriter, r *http.Request, rsc *request.Resources) *ResponseGate { - rg := &ResponseGate{ResponseWriter: w, Request: r, Resources: rsc} - if w != nil { - rg.header = w.Header().Clone() - } - return rg +// GetGeneric returns the accumulated generic data (thread-safe) +func (a *Accumulator) GetGeneric() any { + a.mu.Lock() + defer a.mu.Unlock() + return a.generic } -// Header returns the ResponseGate's Header map -func (rg *ResponseGate) Header() http.Header { - return rg.header +// SetGeneric sets the accumulated generic data (thread-safe) +func (a *Accumulator) SetGeneric(data any) { + a.mu.Lock() + defer a.mu.Unlock() + a.generic = data } -// WriteHeader is not used with a ResponseGate -func (rg *ResponseGate) WriteHeader(_ int) { +// Lock locks the accumulator's mutex (for use by merge functions) +func (a *Accumulator) Lock() { + a.mu.Lock() } -// Body returns the stored body for merging -func (rg *ResponseGate) Body() []byte { - return rg.body +// Unlock unlocks the accumulator's mutex (for use by merge functions) +func (a *Accumulator) Unlock() { + a.mu.Unlock() } -// Write is not used with a ResponseGate -func (rg *ResponseGate) Write(b []byte) (int, error) { - l := len(b) - if l == 0 { - return 0, nil - } - if len(rg.body) == 0 { - rg.body = stdbytes.Clone(b) - } else { - rg.body = bytes.MergeSlices(rg.body, b) - } - return len(b), nil +// GetGenericUnsafe returns the generic data without locking (caller must hold lock) +func (a *Accumulator) GetGenericUnsafe() any { + return a.generic } -// Compress removes nil ResponseGates from the slice -func (rgs ResponseGates) Compress() ResponseGates { - out := make(ResponseGates, len(rgs)) - var k int - for _, rg := range rgs { - if rg == nil { - continue - } - out[k] = rg - k++ - } - return out[:k] +// SetGenericUnsafe sets the generic data without locking (caller must hold lock) +func (a *Accumulator) SetGenericUnsafe(data any) { + a.generic = data } diff --git a/pkg/proxy/response/merge/timeseries.go b/pkg/proxy/response/merge/timeseries.go index f04e2ff3f..8f50e5799 100644 --- a/pkg/proxy/response/merge/timeseries.go +++ b/pkg/proxy/response/merge/timeseries.go @@ -17,8 +17,6 @@ package merge import ( - "bytes" - "io" "net/http" "github.com/trickstercache/trickster/v2/pkg/proxy/handlers/trickster/failures" @@ -26,61 +24,61 @@ import ( "github.com/trickstercache/trickster/v2/pkg/timeseries" ) -// Timeseries merges the provided Responses into a single Timeseries DataSet -// and writes it to the provided responsewriter -func Timeseries(w http.ResponseWriter, r *http.Request, rgs ResponseGates) { - var f timeseries.MarshalWriterFunc - var rlo *timeseries.RequestOptions - - responses := make([]int, len(rgs)) - var bestResp *http.Response - - h := w.Header() - tsm := make(timeseries.List, len(rgs)) - var k int - for i, rg := range rgs { - if rg == nil || rg.Resources == nil || - rg.Resources.Response == nil { - continue - } - - resp := rg.Resources.Response - responses[i] = resp.StatusCode - - if rg.Resources.TS != nil { - headers.Merge(h, rg.Header()) - if f == nil && rg.Resources.TSMarshaler != nil { - f = rg.Resources.TSMarshaler - } - if rlo == nil { - rlo = rg.Resources.TSReqestOptions +// TimeseriesMergeFunc creates a MergeFunc for timeseries data +// The returned function accepts a timeseries.Timeseries and merges it into the accumulator +func TimeseriesMergeFunc(unmarshaler timeseries.UnmarshalerFunc) MergeFunc { + return func(accum *Accumulator, data any, idx int) error { + ts, ok := data.(timeseries.Timeseries) + if !ok { + // If data is []byte, unmarshal it first (for backward compatibility during transition) + if body, ok := data.([]byte); ok { + var err error + ts, err = unmarshaler(body, nil) + if err != nil { + return err + } + } else { + // Not a timeseries and not []byte + return nil } - tsm[k] = rg.Resources.TS - k++ } - if bestResp == nil || resp.StatusCode < bestResp.StatusCode { - bestResp = resp - resp.Body = io.NopCloser(bytes.NewReader(rg.Body())) + if accum.tsdata == nil { + accum.tsdata = ts + } else { + accum.tsdata.Merge(false, ts) } + return nil } +} - if k == 0 || f == nil { - if bestResp != nil { - h := w.Header() - headers.Merge(h, bestResp.Header) - w.WriteHeader(bestResp.StatusCode) - io.Copy(w, bestResp.Body) - } else { - failures.HandleBadGateway(w, r) +// TimeseriesMergeFuncFromBytes creates a MergeFunc that accepts []byte and unmarshals it +// This is a convenience function for call sites that still have []byte +func TimeseriesMergeFuncFromBytes(unmarshaler timeseries.UnmarshalerFunc) func(*Accumulator, []byte, int) error { + mergeFunc := TimeseriesMergeFunc(unmarshaler) + return func(accum *Accumulator, body []byte, idx int) error { + ts, err := unmarshaler(body, nil) + if err != nil { + return err } - return + return mergeFunc(accum, ts, idx) } +} - statusCode := http.StatusOK - if bestResp != nil { - statusCode = bestResp.StatusCode +// TimeseriesRespondFunc creates a RespondFunc for timeseries data +// It writes the merged timeseries using the marshaler +func TimeseriesRespondFunc(marshaler timeseries.MarshalWriterFunc, requestOptions *timeseries.RequestOptions) RespondFunc { + return func(w http.ResponseWriter, r *http.Request, accum *Accumulator, statusCode int) { + accum.mu.Lock() + ts := accum.tsdata + accum.mu.Unlock() + if ts == nil { + failures.HandleBadGateway(w, r) + return + } + if statusCode == 0 { + statusCode = http.StatusOK + } + headers.StripMergeHeaders(w.Header()) + marshaler(ts, requestOptions, statusCode, w) } - - headers.StripMergeHeaders(h) - f(tsm.Merge(false), rlo, statusCode, w) } From bc04b10dc292efdcadbfd4d2877ab9635fc27c48 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:35:55 -0700 Subject: [PATCH 04/28] bugfix: regression - expand '*' path wildcard Signed-off-by: James Ranson --- pkg/proxy/paths/options/options.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/proxy/paths/options/options.go b/pkg/proxy/paths/options/options.go index 6ad467e05..bbf7e1491 100644 --- a/pkg/proxy/paths/options/options.go +++ b/pkg/proxy/paths/options/options.go @@ -146,6 +146,15 @@ func (o *Options) Initialize(_ string) error { o.Methods = []string{http.MethodGet} } + // Expand "*" to all HTTP methods + // If "*" is present, it replaces all other methods + for _, method := range o.Methods { + if method == "*" { + o.Methods = methods.AllHTTPMethods() + break + } + } + if o.MatchTypeName == "" { o.MatchTypeName = matching.PathMatchNameExact o.MatchType = matching.PathMatchTypeExact From f71076991d4533490a99decf845fa4ef75244dbc Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:36:36 -0700 Subject: [PATCH 05/28] restore "stack" field to all log entries Signed-off-by: James Ranson --- pkg/observability/logging/logging.go | 43 +++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/pkg/observability/logging/logging.go b/pkg/observability/logging/logging.go index 84fc9a522..13ce51c23 100644 --- a/pkg/observability/logging/logging.go +++ b/pkg/observability/logging/logging.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "os" + "runtime" "slices" "strconv" "strings" @@ -319,7 +320,7 @@ func (l *logger) HasLoggedOnce(logLevel level.Level, key string) bool { } func (l *logger) logAsyncronous(logLevel level.Level, event string, detail Pairs) { - go l.log(logLevel, event, detail) + go l.logWithStack(logLevel, event, detail, getCallerStack(1)) } type item struct { @@ -337,7 +338,40 @@ const ( newline = "\n" ) +// getCallerStack returns the first path in the call stack from /pkg not in +func getCallerStack(skip int) string { + for s := skip; s < skip+20; s++ { + pc, file, line, ok := runtime.Caller(s) + if !ok { + break + } + idx := strings.Index(file, "/pkg/") + if idx == -1 || strings.Contains(file, "/pkg/observability/logging/") { + continue + } + fn := runtime.FuncForPC(pc) + if fn == nil { + continue + } + return file[idx+1:] + ":" + strconv.Itoa(line) + } + return "" +} + func (l *logger) log(logLevel level.Level, event string, detail Pairs) { + // For synchronous logging, capture caller here + // Call stack from getCallerStack's perspective: + // runtime.Caller(1) = log + // runtime.Caller(2) = logConditionally or logFuncConditionally + // runtime.Caller(3) = Warn/Info/etc in logging package + // runtime.Caller(4) = Warn/Info/etc in logger package + // runtime.Caller(5) = actual caller + // Start from skip 1 to check log itself, then walk up + stack := getCallerStack(1) + l.logWithStack(logLevel, event, detail, stack) +} + +func (l *logger) logWithStack(logLevel level.Level, event string, detail Pairs, stack string) { if l.writer == nil { return } @@ -346,12 +380,19 @@ func (l *logger) log(logLevel level.Level, event string, detail Pairs) { if strings.HasPrefix(event, space) || strings.HasSuffix(event, space) { event = strings.TrimSpace(event) } + logLine := []byte( "time=" + ts.UTC().Format(time.RFC3339Nano) + space + "app=trickster" + space + "level=" + string(logLevel) + space + "event=" + quoteAsNeeded(event), ) + + // Add stack field if available + if stack != "" { + logLine = append(logLine, []byte(space+"stack="+stack)...) + } + if ld > 0 { logLine = append(logLine, []byte(space)...) keyPairs := make([]item, ld) From 786f74758254efa6fd2347bd57bab3cb7050bd7d Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:38:39 -0700 Subject: [PATCH 06/28] refactor cache name/id consts Signed-off-by: James Ranson --- pkg/cache/bbolt/bbolt_test.go | 14 +++--- pkg/cache/options/defaults/defaults.go | 2 +- pkg/cache/options/options.go | 63 +++++++++++++++++++------- pkg/cache/providers/providers.go | 42 +++++++++++------ pkg/cache/registry/registry.go | 59 ++++++++---------------- pkg/cache/registry/registry_test.go | 12 ++--- pkg/config/config.go | 2 +- 7 files changed, 107 insertions(+), 87 deletions(-) diff --git a/pkg/cache/bbolt/bbolt_test.go b/pkg/cache/bbolt/bbolt_test.go index 1ce176d96..d565a85d2 100644 --- a/pkg/cache/bbolt/bbolt_test.go +++ b/pkg/cache/bbolt/bbolt_test.go @@ -67,7 +67,7 @@ func storeBenchmark(b *testing.B) CacheClient { return *bc } -func TestBboltCache_Connect(t *testing.T) { +func TestBBoltCache_Connect(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) testDbPath := t.TempDir() + "/test.db" cacheConfig := newCacheConfig(testDbPath) @@ -80,7 +80,7 @@ func TestBboltCache_Connect(t *testing.T) { bc.Close() } -func TestBboltCache_ConnectFailed(t *testing.T) { +func TestBBoltCache_ConnectFailed(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) const expected = `open /root/noaccess.bbolt:` cacheConfig := newCacheConfig("/root/noaccess.bbolt") @@ -96,7 +96,7 @@ func TestBboltCache_ConnectFailed(t *testing.T) { } } -func TestBboltCache_ConnectBadBucketName(t *testing.T) { +func TestBBoltCache_ConnectBadBucketName(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) const expected = `create bucket: bucket name required` testDbPath := t.TempDir() + "/test.db" @@ -113,7 +113,7 @@ func TestBboltCache_ConnectBadBucketName(t *testing.T) { } } -func TestBboltCache_Store(t *testing.T) { +func TestBBoltCache_Store(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) testDbPath := t.TempDir() + "/test.db" cacheConfig := newCacheConfig(testDbPath) @@ -137,7 +137,7 @@ func BenchmarkCache_Store(b *testing.B) { defer bc.Close() } -func TestBboltCache_Remove(t *testing.T) { +func TestBBoltCache_Remove(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) testDbPath := t.TempDir() + "/test.db" cacheConfig := newCacheConfig(testDbPath) @@ -215,7 +215,7 @@ func BenchmarkCache_Remove(b *testing.B) { } } -func TestBboltCache_BulkRemove(t *testing.T) { +func TestBBoltCache_BulkRemove(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) testDbPath := t.TempDir() + "/test.db" cacheConfig := newCacheConfig(testDbPath) @@ -279,7 +279,7 @@ func BenchmarkCache_BulkRemove(b *testing.B) { } } -func TestBboltCache_Retrieve(t *testing.T) { +func TestBBoltCache_Retrieve(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) testDbPath := t.TempDir() + "/test.db" diff --git a/pkg/cache/options/defaults/defaults.go b/pkg/cache/options/defaults/defaults.go index 7aa19e602..d7d0bc79e 100644 --- a/pkg/cache/options/defaults/defaults.go +++ b/pkg/cache/options/defaults/defaults.go @@ -23,7 +23,7 @@ const ( DefaultCacheProvider = "memory" // DefaultCacheProviderID is the default cache providers ID for any defined cache // and should align with DefaultCacheProvider - DefaultCacheProviderID = providers.Memory + DefaultCacheProviderID = providers.MemoryID DefaultUseCacheChunking = false DefaultTimeseriesChunkFactor = int64(420) DefaultByterangeChunkSize = int64(4096) diff --git a/pkg/cache/options/options.go b/pkg/cache/options/options.go index 0bb81bae9..0dc6faaf5 100644 --- a/pkg/cache/options/options.go +++ b/pkg/cache/options/options.go @@ -119,13 +119,13 @@ func (o *Options) Equal(o2 *Options) bool { return false } switch o.ProviderID { - case providers.Redis: + case providers.RedisID: return o.Redis.Equal(o2.Redis) - case providers.Filesystem: + case providers.FilesystemID: return o.Filesystem.Equal(o2.Filesystem) - case providers.Bbolt: + case providers.BBoltID: return o.BBolt.Equal(o2.BBolt) - case providers.BadgerDB: + case providers.BadgerDBID: return o.Badger.Equal(o2.Badger) default: return true @@ -136,6 +136,9 @@ func (o *Options) Validate() (bool, error) { if restrictedNames.Contains(o.Name) { return false, ErrInvalidName } + if o.Index == nil { + return true, nil + } if o.Index.MaxSizeBytes > 0 && o.Index.MaxSizeBackoffBytes > o.Index.MaxSizeBytes { return false, errMaxSizeBackoffBytesTooBig } @@ -157,26 +160,46 @@ func (o *Options) Initialize(name string) error { o.Name = name if o.Provider != "" { - o.Provider = strings.ToLower(o.Provider) + o.Provider = strings.TrimSpace(strings.ToLower(o.Provider)) if n, ok := providers.Names[o.Provider]; ok { o.ProviderID = n } } - if o.Index == nil { - o.Index = index.New() + if providers.UsesIndex(o.Provider) { + if o.Index == nil { + o.Index = index.New() + } + } else { + o.Index = nil } - if o.Redis == nil { - o.Redis = redis.New() + if o.ProviderID == providers.RedisID { + if o.Redis == nil { + o.Redis = redis.New() + } + } else { + o.Redis = nil } - if o.Filesystem == nil { - o.Filesystem = filesystem.New() + if o.ProviderID == providers.FilesystemID { + if o.Filesystem == nil { + o.Filesystem = filesystem.New() + } + } else { + o.Filesystem = nil } - if o.BBolt == nil { - o.BBolt = bbolt.New() + if o.ProviderID == providers.BBoltID { + if o.BBolt == nil { + o.BBolt = bbolt.New() + } + } else { + o.BBolt = nil } - if o.Badger == nil { - o.Badger = badger.New() + if o.ProviderID == providers.BadgerDBID { + if o.Badger == nil { + o.Badger = badger.New() + } + } else { + o.Badger = nil } o.UseCacheChunking = defaults.DefaultUseCacheChunking @@ -207,7 +230,7 @@ func (l Lookup) Initialize(activeCaches sets.Set[string]) ([]string, error) { return nil, err } - if v.ProviderID == providers.Redis { + if v.ProviderID == providers.RedisID { var hasEndpoint, hasEndpoints bool if v.Redis.Endpoint != "" { @@ -254,3 +277,11 @@ func (o *Options) UnmarshalYAML(unmarshal func(any) error) error { *o = Options(lo) return nil } + +func (o *Options) ClearProviderOptions() { + o.Index = nil + o.Redis = nil + o.Filesystem = nil + o.BBolt = nil + o.Badger = nil +} diff --git a/pkg/cache/providers/providers.go b/pkg/cache/providers/providers.go index e25682bbf..594c6e2a1 100644 --- a/pkg/cache/providers/providers.go +++ b/pkg/cache/providers/providers.go @@ -22,25 +22,31 @@ import "strconv" type Provider int const ( - // Memory indicates a memory cache - Memory = Provider(iota) - // Filesystem indicates a filesystem cache - Filesystem - // Redis indicates a Redis cache - Redis - // Bbolt indicates a Bbolt cache - Bbolt - // BadgerDB indicates a BadgerDB cache - BadgerDB + // MemoryID indicates a memory cache + MemoryID = Provider(iota) + // FilesystemID indicates a filesystem cache + FilesystemID + // RedisID indicates a Redis cache + RedisID + // BBoltID indicates a BBolt cache + BBoltID + // BadgerDBID indicates a BadgerDB cache + BadgerDBID + + Memory = "memory" + Filesystem = "filesystem" + Redis = "redis" + BBolt = "bbolt" + BadgerDB = "badger" ) // Names is a map of cache providers keyed by name var Names = map[string]Provider{ - "memory": Memory, - "filesystem": Filesystem, - "redis": Redis, - "bbolt": Bbolt, - "badger": BadgerDB, + Memory: MemoryID, + Filesystem: FilesystemID, + Redis: RedisID, + BBolt: BBoltID, + BadgerDB: BadgerDBID, } // Values is a map of cache providers keyed by internal id @@ -58,3 +64,9 @@ func (p Provider) String() string { } return strconv.Itoa(int(p)) } + +// UsesIndex returns true if the providerName uses an index +// providerName is expected to already be lowercase/no-space +func UsesIndex(providerName string) bool { + return providerName != BadgerDB && providerName != Redis +} diff --git a/pkg/cache/registry/registry.go b/pkg/cache/registry/registry.go index de58b9f32..9c162a7cd 100644 --- a/pkg/cache/registry/registry.go +++ b/pkg/cache/registry/registry.go @@ -25,23 +25,14 @@ import ( "github.com/trickstercache/trickster/v2/pkg/cache/badger" "github.com/trickstercache/trickster/v2/pkg/cache/bbolt" "github.com/trickstercache/trickster/v2/pkg/cache/filesystem" - "github.com/trickstercache/trickster/v2/pkg/cache/index" - indexopts "github.com/trickstercache/trickster/v2/pkg/cache/index/options" "github.com/trickstercache/trickster/v2/pkg/cache/manager" "github.com/trickstercache/trickster/v2/pkg/cache/memory" "github.com/trickstercache/trickster/v2/pkg/cache/options" + "github.com/trickstercache/trickster/v2/pkg/cache/providers" "github.com/trickstercache/trickster/v2/pkg/cache/redis" "github.com/trickstercache/trickster/v2/pkg/config" ) -// Cache Interface Types -const ( - ctFilesystem = "filesystem" - ctRedis = "redis" - ctBBolt = "bbolt" - ctBadger = "badger" -) - // LoadCachesFromConfig iterates the Caching Config and Connects/Maps each Cache func LoadCachesFromConfig(conf *config.Config) cache.Lookup { caches := make(cache.Lookup) @@ -64,41 +55,27 @@ func CloseCaches(caches cache.Lookup) error { // NewCache returns a Cache object based on the provided config.CachingConfig func NewCache(cacheName string, cfg *options.Options) cache.Cache { - if cfg.Index == nil { - cfg.Index = indexopts.New() - } - var c cache.Cache - + co := manager.CacheOptions{ + UseIndex: providers.UsesIndex(cfg.Provider), + } switch cfg.Provider { - case ctFilesystem: - c = manager.NewCache(filesystem.NewCache(cacheName, cfg), manager.CacheOptions{ - UseIndex: true, - IndexCliOpts: index.IndexedClientOptions{ - NeedsFlushInterval: true, - NeedsReapInterval: true, - }, - }, cfg) - case ctRedis: - c = manager.NewCache(redis.New(context.Background(), cacheName, cfg), manager.CacheOptions{}, cfg) - case ctBBolt: - c = manager.NewCache(bbolt.New(cacheName, "", "", cfg), manager.CacheOptions{ - UseIndex: true, - IndexCliOpts: index.IndexedClientOptions{ - NeedsFlushInterval: true, - NeedsReapInterval: true, - }, - }, cfg) - case ctBadger: - c = manager.NewCache(badger.New(cacheName, cfg), manager.CacheOptions{}, cfg) + case providers.Filesystem: + co.IndexCliOpts.NeedsFlushInterval = true + co.IndexCliOpts.NeedsReapInterval = true + c = manager.NewCache(filesystem.NewCache(cacheName, cfg), co, cfg) + case providers.Redis: + c = manager.NewCache(redis.New(context.Background(), cacheName, cfg), co, cfg) + case providers.BBolt: + co.IndexCliOpts.NeedsFlushInterval = true + co.IndexCliOpts.NeedsReapInterval = true + c = manager.NewCache(bbolt.New(cacheName, "", "", cfg), co, cfg) + case providers.BadgerDB: + c = manager.NewCache(badger.New(cacheName, cfg), co, cfg) default: // Default to MemoryCache - c = manager.NewCache(memory.New(cacheName, cfg), manager.CacheOptions{ - UseIndex: true, - IndexCliOpts: index.IndexedClientOptions{ - NeedsReapInterval: true, - }, - }, cfg) + co.IndexCliOpts.NeedsReapInterval = true + c = manager.NewCache(memory.New(cacheName, cfg), co, cfg) } c.Connect() return c diff --git a/pkg/cache/registry/registry_test.go b/pkg/cache/registry/registry_test.go index 2ba7188df..d9b43380f 100644 --- a/pkg/cache/registry/registry_test.go +++ b/pkg/cache/registry/registry_test.go @@ -41,11 +41,11 @@ func TestLoadCachesFromConfig(t *testing.T) { cfg := newCacheConfig(t, key) conf.Caches[key] = cfg switch v { - case providers.Bbolt: + case providers.BBoltID: cfg.BBolt.Filename = t.TempDir() + "/" + key + "-testcache.db" - case providers.Filesystem: + case providers.FilesystemID: cfg.BBolt.Filename = t.TempDir() + "/" + key + "-testcache" - case providers.BadgerDB: + case providers.BadgerDBID: cfg.BBolt.Filename = t.TempDir() + "/" + key + "-testcache" } } @@ -76,13 +76,13 @@ func newCacheConfig(t *testing.T, cacheProvider string) *co.Options { ctid, ok := providers.Names[cacheProvider] if !ok { - ctid = providers.Memory + ctid = providers.MemoryID } switch ctid { - case providers.BadgerDB: + case providers.BadgerDBID: bd = t.TempDir() + "/" + cacheProvider - case providers.Filesystem: + case providers.FilesystemID: fd = t.TempDir() + "/" + cacheProvider } diff --git a/pkg/config/config.go b/pkg/config/config.go index 391993a98..345b1259f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -337,7 +337,7 @@ func (c *Config) String() string { // strip Redis password for k, v := range cp.Caches { - if v != nil && cp.Caches[k].Redis.Password != "" { + if v != nil && cp.Caches[k].Redis != nil && cp.Caches[k].Redis.Password != "" { cp.Caches[k].Redis.Password = "*****" } } From 27830880184f61c1756b4f9e20def1fa1cd10dc6 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:42:14 -0700 Subject: [PATCH 07/28] cleanup healthcheck Signed-off-by: James Ranson --- pkg/backends/alb/client.go | 4 ++-- pkg/backends/alb/mech/types/types.go | 1 + pkg/backends/alb/pool/health_test.go | 2 +- pkg/backends/alb/pool/pool_test.go | 8 ++++---- pkg/backends/healthcheck/target.go | 4 +++- pkg/backends/healthcheck/target_test.go | 12 ++++++++---- pkg/backends/options/options.go | 8 +------- pkg/daemon/setup/setup.go | 2 +- pkg/testutil/albpool/albpool.go | 2 +- 9 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/backends/alb/client.go b/pkg/backends/alb/client.go index c565ee04b..02e58987f 100644 --- a/pkg/backends/alb/client.go +++ b/pkg/backends/alb/client.go @@ -156,7 +156,7 @@ func (c *Client) ValidateAndStartPool(clients backends.Backends, hcs healthcheck if o.MechanismName == names.MechanismUR && o.UserRouter != nil { return c.validateAndStartUserRouter(clients) } - targets := make([]*pool.Target, 0, len(o.Pool)) + targets := make(pool.Targets, 0, len(o.Pool)) for _, n := range o.Pool { tc, ok := clients[n] if !ok { @@ -166,7 +166,7 @@ func (c *Client) ValidateAndStartPool(clients backends.Backends, hcs healthcheck if !ok { continue // virtual backends (rule, alb) don't currently have health checks } - targets = append(targets, pool.NewTarget(tc.Router(), hc)) + targets = append(targets, pool.NewTarget(tc.Router(), hc, tc)) } if c.handler != nil { c.handler.SetPool(pool.New(targets, o.HealthyFloor)) diff --git a/pkg/backends/alb/mech/types/types.go b/pkg/backends/alb/mech/types/types.go index 1ed75c152..332de3036 100644 --- a/pkg/backends/alb/mech/types/types.go +++ b/pkg/backends/alb/mech/types/types.go @@ -39,6 +39,7 @@ type Mechanism interface { http.Handler SetPool(pool.Pool) StopPool() + Pool() pool.Pool ID() ID Name() Name } diff --git a/pkg/backends/alb/pool/health_test.go b/pkg/backends/alb/pool/health_test.go index 90871de45..b063af3a3 100644 --- a/pkg/backends/alb/pool/health_test.go +++ b/pkg/backends/alb/pool/health_test.go @@ -43,7 +43,7 @@ func TestCheckHealth(t *testing.T) { cancel() time.Sleep(10 * time.Millisecond) - h := p.healthy.Load() + h := p.healthyHandlers.Load() if h == nil { t.Error("expected non-nil healthy list") return diff --git a/pkg/backends/alb/pool/pool_test.go b/pkg/backends/alb/pool/pool_test.go index 59b8a9299..1e182c4e5 100644 --- a/pkg/backends/alb/pool/pool_test.go +++ b/pkg/backends/alb/pool/pool_test.go @@ -25,7 +25,7 @@ import ( func TestNewTarget(t *testing.T) { s := &healthcheck.Status{} - tgt := NewTarget(http.NotFoundHandler(), s) + tgt := NewTarget(http.NotFoundHandler(), s, nil) if tgt.hcStatus != s { t.Error("unexpected mismatch") } @@ -33,12 +33,12 @@ func TestNewTarget(t *testing.T) { func TestNewPool(t *testing.T) { s := &healthcheck.Status{} - tgt := NewTarget(http.NotFoundHandler(), s) + tgt := NewTarget(http.NotFoundHandler(), s, nil) if tgt.hcStatus != s { t.Error("unexpected mismatch") } - p := New([]*Target{tgt}, 1) + p := New(Targets{tgt}, 1) if p == nil { t.Error("expected non-nil") } @@ -51,7 +51,7 @@ func TestNewPool(t *testing.T) { p2 := p.(*pool) hl := []http.Handler{http.NotFoundHandler()} - p2.healthy.Store(&hl) + p2.healthyHandlers.Store(&hl) if len(p.Healthy()) != 1 { t.Error("expected 1 healthy target", len(p.Healthy())) diff --git a/pkg/backends/healthcheck/target.go b/pkg/backends/healthcheck/target.go index cdd875d37..34d683991 100644 --- a/pkg/backends/healthcheck/target.go +++ b/pkg/backends/healthcheck/target.go @@ -100,7 +100,9 @@ func newTarget(_ context.Context, name, description string, o *ho.Options, } t.status = &Status{name: name, detail: isd, description: description, prober: t.demandProbe} - t.status.Set(StatusInitializing) + if interval > 0 { + t.status.Set(StatusInitializing) + } if len(o.ExpectedHeaders) > 0 { t.eh = headers.Lookup(o.ExpectedHeaders).ToHeader() } diff --git a/pkg/backends/healthcheck/target_test.go b/pkg/backends/healthcheck/target_test.go index 5cdc035f7..cd2840afb 100644 --- a/pkg/backends/healthcheck/target_test.go +++ b/pkg/backends/healthcheck/target_test.go @@ -224,6 +224,7 @@ func TestNewHTTPClient(t *testing.T) { func TestProbe(t *testing.T) { logger.SetLogger(testLogger) ts := newTestServer(200, "OK", map[string]string{}) + defer ts.Close() t.Run("direct probe calls", func(t *testing.T) { r, _ := http.NewRequest("GET", ts.URL+"/", nil) @@ -248,6 +249,9 @@ func TestProbe(t *testing.T) { }) t.Run("probe loop", func(t *testing.T) { + const intervalMS = 5 + const windowMS = 1500 + u, err := url.Parse(ts.URL) require.NoError(t, err) target, err := newTarget(context.Background(), "testprobe", "testprobe", &ho.Options{ @@ -255,21 +259,21 @@ func TestProbe(t *testing.T) { Scheme: u.Scheme, Host: u.Host, Path: "/", - Interval: 1 * time.Second, + Interval: intervalMS * time.Millisecond, ExpectedCodes: []int{200}, }, ts.Client()) require.NoError(t, err) // start probe loop - ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(1500*time.Millisecond)) + ctx, cancel := context.WithCancel(context.Background()) defer cancel() - target.interval = 5 * time.Millisecond target.Start(ctx) - time.Sleep(1500 * time.Millisecond) + time.Sleep((intervalMS + windowMS) * time.Millisecond) // verify results success := target.successConsecutiveCnt.Load() fail := target.failConsecutiveCnt.Load() require.Equal(t, int32(0), fail) require.GreaterOrEqual(t, success, int32(90)) + target.Stop() }) } diff --git a/pkg/backends/options/options.go b/pkg/backends/options/options.go index 83b8c1e4b..1a4f3cc84 100644 --- a/pkg/backends/options/options.go +++ b/pkg/backends/options/options.go @@ -514,6 +514,7 @@ func (l Lookup) Initialize() error { if !ncb.Contains(v.Provider) && v.CacheName == "" { v.CacheName = DefaultBackendCacheName } + v.Initialize(k) if len(v.Paths) > 0 { err := v.Paths.Initialize() if err != nil { @@ -527,15 +528,8 @@ func (l Lookup) Initialize() error { // Initialize sets up the backend Options with default values and overlays // any values that were set during YAML unmarshaling func (o *Options) Initialize(name string) error { - // activeCaches sets.Set[string], o.Name = name - // // If there is only one backend and is_default is not explicitly false, make it true - // if len(backends) == 1 && (!y.IsDefined("backends", name, "is_default")) { - // out.IsDefault = true - // } - // activeCaches.Set(out.CacheName) - if o.OriginURL != "" { parsedURL, err := url.Parse(o.OriginURL) if err != nil { diff --git a/pkg/daemon/setup/setup.go b/pkg/daemon/setup/setup.go index bb427ba04..f5d862078 100644 --- a/pkg/daemon/setup/setup.go +++ b/pkg/daemon/setup/setup.go @@ -297,7 +297,7 @@ func applyCachingConfig(si *instance.ServerInstance, // is the only change. In this case, we'll apply the new index configuration, // then add the old cache with the new index config to the new cache map if ocfg.ProviderID == v.ProviderID && - v.ProviderID == providers.Memory { + v.ProviderID == providers.MemoryID { // Note: this is only necessary for the memory cache as all other providers will be closed and reopened with the newest config if v.Index != nil { mc := w.(*manager.Manager).Client.(*index.IndexedClient) diff --git a/pkg/testutil/albpool/albpool.go b/pkg/testutil/albpool/albpool.go index bee992815..cc98c67f9 100644 --- a/pkg/testutil/albpool/albpool.go +++ b/pkg/testutil/albpool/albpool.go @@ -34,7 +34,7 @@ func New(healthyFloor int, hs []http.Handler) (pool.Pool, for _, h := range hs { hst := &healthcheck.Status{} statuses = append(statuses, hst) - targets = append(targets, pool.NewTarget(h, hst)) + targets = append(targets, pool.NewTarget(h, hst, nil)) } } pool := pool.New(targets, healthyFloor) From 9ac1329799f96d3f8aa388c08260f9b24315fdd5 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:42:35 -0700 Subject: [PATCH 08/28] refactor resources Signed-off-by: James Ranson --- pkg/proxy/context/resources.go | 8 ++++++++ pkg/proxy/engines/httpproxy.go | 8 ++++++-- pkg/proxy/engines/objectproxycache.go | 2 +- pkg/proxy/request/resources.go | 21 ++++++++++++++++++--- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/pkg/proxy/context/resources.go b/pkg/proxy/context/resources.go index d6fc83878..e3548f771 100644 --- a/pkg/proxy/context/resources.go +++ b/pkg/proxy/context/resources.go @@ -33,3 +33,11 @@ func WithResources(ctx context.Context, r any) context.Context { func Resources(ctx context.Context) any { return ctx.Value(resourcesKey) } + +// ClearResources removes Resources from the context +func ClearResources(ctx context.Context) context.Context { + if ctx == nil { + return ctx + } + return context.WithValue(ctx, resourcesKey, nil) +} diff --git a/pkg/proxy/engines/httpproxy.go b/pkg/proxy/engines/httpproxy.go index 9c03608f8..cf25c1c0d 100644 --- a/pkg/proxy/engines/httpproxy.go +++ b/pkg/proxy/engines/httpproxy.go @@ -18,6 +18,8 @@ package engines import ( "bytes" + "context" + "errors" "io" "math" "net/http" @@ -209,8 +211,10 @@ func PrepareFetchReader(r *http.Request) (io.ReadCloser, *http.Response, int64) resp, err := o.HTTPClient.Do(r) if err != nil { - logger.Error("error downloading url", - logging.Pairs{"url": r.URL.String(), "detail": err.Error()}) + if rsc == nil || !rsc.Cancelable || !errors.Is(err, context.Canceled) { + logger.Error("error downloading url", + logging.Pairs{"url": r.URL.String(), "detail": err.Error()}) + } // if there is an err and the response is nil, the server could not be reached // so make a 502 for the downstream response if resp == nil { diff --git a/pkg/proxy/engines/objectproxycache.go b/pkg/proxy/engines/objectproxycache.go index 11d3c0499..8553f0188 100644 --- a/pkg/proxy/engines/objectproxycache.go +++ b/pkg/proxy/engines/objectproxycache.go @@ -490,7 +490,7 @@ func ObjectProxyCacheRequest(w http.ResponseWriter, r *http.Request) { if cacheStatus == status.LookupStatusProxyOnly { DoProxy(w, r, true) } else if rsc := request.GetResources(r); resp != nil && rsc != nil && - (rsc.ResponseMergeFunc != nil || rsc.TSTransformer != nil) { + (rsc.MergeFunc != nil || rsc.TSTransformer != nil) { rsc.Response = resp } } diff --git a/pkg/proxy/request/resources.go b/pkg/proxy/request/resources.go index c29cc5779..12ae7ae8a 100644 --- a/pkg/proxy/request/resources.go +++ b/pkg/proxy/request/resources.go @@ -30,6 +30,7 @@ import ( auth "github.com/trickstercache/trickster/v2/pkg/proxy/authenticator/types" tctx "github.com/trickstercache/trickster/v2/pkg/proxy/context" po "github.com/trickstercache/trickster/v2/pkg/proxy/paths/options" + "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" "github.com/trickstercache/trickster/v2/pkg/timeseries" ) @@ -48,7 +49,8 @@ type Resources struct { Tracer *tracing.Tracer IsMergeMember bool RequestBody []byte - ResponseMergeFunc any + MergeFunc merge.MergeFunc + MergeRespondFunc merge.RespondFunc TSUnmarshaler timeseries.UnmarshalerFunc TSMarshaler timeseries.MarshalWriterFunc TSTransformer func(timeseries.Timeseries) @@ -57,6 +59,7 @@ type Resources struct { Response *http.Response AuthResult *auth.AuthResult AlreadyEncoded bool + Cancelable bool } // Clone returns an exact copy of the subject Resources collection @@ -73,7 +76,8 @@ func (r *Resources) Clone() *Resources { Tracer: r.Tracer, IsMergeMember: r.IsMergeMember, RequestBody: slices.Clone(r.RequestBody), - ResponseMergeFunc: r.ResponseMergeFunc, + MergeFunc: r.MergeFunc, + MergeRespondFunc: r.MergeRespondFunc, TSUnmarshaler: r.TSUnmarshaler, TSMarshaler: r.TSMarshaler, TSTransformer: r.TSTransformer, @@ -81,6 +85,7 @@ func (r *Resources) Clone() *Resources { TSReqestOptions: r.TSReqestOptions, AuthResult: r.AuthResult, // shallow copy of the auth result AlreadyEncoded: r.AlreadyEncoded, + Cancelable: r.Cancelable, } } @@ -119,6 +124,14 @@ func SetResources(r *http.Request, rsc *Resources) *http.Request { return r.WithContext(tctx.WithResources(r.Context(), rsc)) } +// ClearResources removes Resources from the HTTP Request's context +func ClearResources(r *http.Request) *http.Request { + if r == nil { + return r + } + return r.WithContext(tctx.ClearResources(r.Context())) +} + // Merge sets the configuration references in the subject resources to the source's func (r *Resources) Merge(r2 *Resources) { if r == nil || r2 == nil { @@ -138,5 +151,7 @@ func (r *Resources) Merge(r2 *Resources) { r.RequestBody = slices.Clone(r2.RequestBody) r.IsMergeMember = r.IsMergeMember || r2.IsMergeMember r.AlreadyEncoded = r.AlreadyEncoded || r2.AlreadyEncoded - r.ResponseMergeFunc = r2.ResponseMergeFunc + r.MergeFunc = r2.MergeFunc + r.MergeRespondFunc = r2.MergeRespondFunc + r.Cancelable = r.Cancelable || r2.Cancelable } From c08488960b7fadd4111f4244cc3ad1ad4b56a7e9 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:42:52 -0700 Subject: [PATCH 09/28] refactor first response / first good response Signed-off-by: James Ranson --- pkg/backends/alb/mech/fr/first_response.go | 125 ++++++++++++--------- 1 file changed, 72 insertions(+), 53 deletions(-) diff --git a/pkg/backends/alb/mech/fr/first_response.go b/pkg/backends/alb/mech/fr/first_response.go index a2243f081..c860c1e5e 100644 --- a/pkg/backends/alb/mech/fr/first_response.go +++ b/pkg/backends/alb/mech/fr/first_response.go @@ -17,8 +17,10 @@ package fr import ( + "context" "net/http" "sync" + "sync/atomic" "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/types" "github.com/trickstercache/trickster/v2/pkg/backends/alb/names" @@ -28,6 +30,7 @@ import ( "github.com/trickstercache/trickster/v2/pkg/proxy/handlers/trickster/failures" "github.com/trickstercache/trickster/v2/pkg/proxy/headers" "github.com/trickstercache/trickster/v2/pkg/proxy/request" + "github.com/trickstercache/trickster/v2/pkg/proxy/response/capture" "github.com/trickstercache/trickster/v2/pkg/util/sets" ) @@ -68,6 +71,10 @@ func (h *handler) SetPool(p pool.Pool) { h.pool = p } +func (h *handler) Pool() pool.Pool { + return h.pool +} + func (h *handler) ID() types.ID { if h.fgr { return FGRID @@ -101,67 +108,79 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } // otherwise iterate the fanout - wc := newResponderClaim(l) + var claimed int64 = -1 + contexts := make([]context.Context, l) + cancels := make([]context.CancelFunc, l) + for i := range l { + contexts[i], cancels[i] = context.WithCancel(r.Context()) + } + captures := make([]*capture.CaptureResponseWriter, l) var wg sync.WaitGroup + responseWritten := make(chan struct{}, 1) + + serveAndCancelOthers := func(i int, crw *capture.CaptureResponseWriter) { + go func() { + // cancels all other contexts + for j, cancel := range cancels { + if j != i { + cancel() + } + } + }() + headers.Merge(w.Header(), crw.Header()) + w.WriteHeader(crw.StatusCode()) + w.Write(crw.Body()) + responseWritten <- struct{}{} + } + + // fanout to all healthy targets for i := range l { - // only the one of these i fanouts to respond will be mapped back to the - // end user based on the methodology and the rest will have their - // contexts canceled + if hl[i] == nil { + continue + } wg.Go(func() { - if hl[i] == nil { - return - } - wm := newFirstResponseGate(w, wc, i, h.fgr) r2, _ := request.Clone(r) - r2 = r2.WithContext(wc.contexts[i]) - hl[i].ServeHTTP(wm, r2) + r2 = r2.WithContext(contexts[i]) + r2 = request.SetResources(r2, &request.Resources{Cancelable: true}) + crw := capture.NewCaptureResponseWriter() + captures[i] = crw + hl[i].ServeHTTP(crw, r2) + statusCode := crw.StatusCode() + custom := h.fgr && len(h.fgrCodes) > 0 + isGood := custom && h.fgrCodes.Contains(statusCode) + // this checks if the response qualifies as a client response + if (!h.fgr || (!custom && statusCode < 400) || isGood) && + // this checks that the qualifying response is the first response + atomic.CompareAndSwapInt64(&claimed, -1, int64(i)) { + // this serves only the first qualifying response + serveAndCancelOthers(i, crw) + // this signals the response is written + } }) } - wg.Wait() -} -type firstResponseGate struct { - http.ResponseWriter - i int - fh http.Header - c *responderClaim - fgr bool - fgrCodes sets.Set[int] -} - -func newFirstResponseGate(w http.ResponseWriter, c *responderClaim, i int, - fgr bool, -) *firstResponseGate { - return &firstResponseGate{ResponseWriter: w, c: c, fh: http.Header{}, i: i, fgr: fgr} -} - -func (frg *firstResponseGate) Header() http.Header { - return frg.fh -} - -func (frg *firstResponseGate) WriteHeader(i int) { - custom := frg.fgr && len(frg.fgrCodes) > 0 - var isGood bool - if custom { - _, isGood = frg.fgrCodes[i] - } - if (!frg.fgr || !custom && i < 400 || custom && isGood) && frg.c.Claim(int64(frg.i)) { - if len(frg.fh) > 0 { - headers.Merge(frg.ResponseWriter.Header(), frg.fh) - frg.fh = nil + // this is a fallback case for when no qualifying upstream response arrives, + // the first response is used, regardless of qualification + go func() { + wg.Wait() + // if claimed is still -1, the fallback case must be used + if atomic.LoadInt64(&claimed) == -1 && r.Context().Err() == nil { + // this iterates the captures and serves the first non-nil response + for i, crw := range captures { + if crw != nil { + serveAndCancelOthers(i, crw) + break + } + } } - frg.ResponseWriter.WriteHeader(i) - return - } -} + }() -func (frg *firstResponseGate) Write(b []byte) (int, error) { - if frg.c.Claim(int64(frg.i)) { - if len(frg.fh) > 0 { - headers.Merge(frg.ResponseWriter.Header(), frg.fh) - frg.fh = nil - } - return frg.ResponseWriter.Write(b) + // this prevents ServeHTTP from returning until the response is fully + // written or the request context is canceled + select { + case <-responseWritten: + return + case <-r.Context().Done(): + return } - return len(b), nil } From 6c5eede48880e7b22bbd6c6596dd8c704964309e Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:43:23 -0700 Subject: [PATCH 10/28] add Pool() to rr mech Signed-off-by: James Ranson --- pkg/backends/alb/mech/rr/round_robin.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/backends/alb/mech/rr/round_robin.go b/pkg/backends/alb/mech/rr/round_robin.go index 83d689f38..ebb03fdbe 100644 --- a/pkg/backends/alb/mech/rr/round_robin.go +++ b/pkg/backends/alb/mech/rr/round_robin.go @@ -50,6 +50,10 @@ func (h *handler) SetPool(p pool.Pool) { h.pool = p } +func (h *handler) Pool() pool.Pool { + return h.pool +} + func (h *handler) ID() types.ID { return ID } @@ -63,8 +67,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { failures.HandleBadGateway(w, r) return } - t := h.nextTarget() - if t != nil { + if t := h.nextTarget(); t != nil { t.ServeHTTP(w, r) return } From 4276722d607630067085310d631b78b66a1e0cc6 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:43:47 -0700 Subject: [PATCH 11/28] refactor newest last modified Signed-off-by: James Ranson --- .../alb/mech/nlm/newest_last_modified.go | 175 ++++++++---------- .../alb/mech/nlm/newest_last_modified_test.go | 15 -- 2 files changed, 75 insertions(+), 115 deletions(-) diff --git a/pkg/backends/alb/mech/nlm/newest_last_modified.go b/pkg/backends/alb/mech/nlm/newest_last_modified.go index e31c8882c..a7904910c 100644 --- a/pkg/backends/alb/mech/nlm/newest_last_modified.go +++ b/pkg/backends/alb/mech/nlm/newest_last_modified.go @@ -20,7 +20,6 @@ import ( "context" "net/http" "sync" - "sync/atomic" "time" "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/types" @@ -31,7 +30,7 @@ import ( "github.com/trickstercache/trickster/v2/pkg/proxy/handlers/trickster/failures" "github.com/trickstercache/trickster/v2/pkg/proxy/headers" "github.com/trickstercache/trickster/v2/pkg/proxy/request" - "github.com/trickstercache/trickster/v2/pkg/util/atomicx" + "github.com/trickstercache/trickster/v2/pkg/proxy/response/capture" ) const ( @@ -55,6 +54,9 @@ func (h *handler) SetPool(p pool.Pool) { h.pool = p } +func (h *handler) Pool() pool.Pool { + return h.pool +} func (h *handler) ID() types.ID { return ID } @@ -72,7 +74,7 @@ func (h *handler) StopPool() { func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { hl := h.pool.Healthy() // should return a fanout list l := len(hl) - if len(hl) == 0 { + if l == 0 { failures.HandleBadGateway(w, r) return } @@ -81,111 +83,84 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { hl[0].ServeHTTP(w, r) return } - // otherwise iterate the fanout - nrm := newNewestResponseMux(l) - var wg sync.WaitGroup - for i := range l { - // only one of these i fanouts to respond will be mapped back to - // the end user based on the methodology and the rest will have their - // contexts canceled - wg.Go(func() { - if hl[i] == nil { - return - } - nrg := newNewestResponseGate(w, i, nrm) - r2, _ := request.Clone(r) - r2 = r2.WithContext(nrm.contexts[i]) - hl[i].ServeHTTP(nrg, r2) - }) - } - wg.Wait() -} -// newestResponseGate is a ResponseWriter that only writes when the muxer -// selects it based on the newness of the response's LastModified header when -// compared to other responses in the Mux -type newestResponseGate struct { - http.ResponseWriter - i, s int64 - ca bool - h, wh http.Header - nrm *newestResponseMux -} - -// newestResponseMux is used by the ResponseGate while collecting the fanout -// Responses to track the response slice index representing the Response -// with the newest LastModified header. -type newestResponseMux struct { - i int64 - t atomicx.Time - wg sync.WaitGroup - contexts []context.Context -} - -func newNewestResponseMux(sz int) *newestResponseMux { - contexts := make([]context.Context, sz) - for i := range sz { - contexts[i] = context.Background() + // Create contexts for cancellation + contexts := make([]context.Context, l) + cancels := make([]context.CancelFunc, l) + for i := range l { + contexts[i], cancels[i] = context.WithCancel(r.Context()) } - nrm := &newestResponseMux{i: -1, contexts: contexts} - nrm.wg.Add(sz) - return nrm -} -func (nrm *newestResponseMux) registerLM(i int, t time.Time) bool { - var ok bool - if t.IsZero() { - return false - } - if nrm.t.Load().IsZero() || t.After(nrm.t.Load()) { - atomic.StoreInt64(&nrm.i, int64(i)) - nrm.t.Store(t) - ok = true - } - return ok -} + // Track the newest Last-Modified response + var newestIdx = -1 + var newestTime time.Time + var mu sync.Mutex -func (nrm *newestResponseMux) getNewest() int64 { - return atomic.LoadInt64(&nrm.i) -} + // Capture all responses + captures := make([]*capture.CaptureResponseWriter, l) + var wg sync.WaitGroup -func newNewestResponseGate(w http.ResponseWriter, i int, - nrm *newestResponseMux, -) *newestResponseGate { - return &newestResponseGate{ - ResponseWriter: w, h: http.Header{}, - i: int64(i), nrm: nrm, + // Fanout to all healthy targets + for i := range l { + if hl[i] == nil { + continue + } + idx := i + wg.Go(func() { + r2, _ := request.Clone(r) + r2 = request.ClearResources(r2.WithContext(contexts[idx])) + crw := capture.NewCaptureResponseWriter() + captures[idx] = crw + hl[idx].ServeHTTP(crw, r2) + + if lmStr := crw.Header().Get(headers.NameLastModified); lmStr != "" { + lm, err := time.Parse(time.RFC1123, lmStr) + if err == nil && !lm.IsZero() { + mu.Lock() + if newestIdx == -1 || lm.After(newestTime) { + newestIdx = idx + newestTime = lm + } + mu.Unlock() + } + } + }) } -} -func (nrg *newestResponseGate) Header() http.Header { - return nrg.h -} - -func (nrg *newestResponseGate) WriteHeader(i int) { - nrg.s = int64(i) - nrg.wh = nrg.h - nrg.h = nil - lm, err := time.Parse(time.RFC1123, nrg.wh.Get(headers.NameLastModified)) - if err == nil { - nrg.ca = !nrg.nrm.registerLM(int(nrg.i), lm) - } - nrg.nrm.wg.Done() -} + // Wait for all responses to complete + wg.Wait() -func (nrg *newestResponseGate) Write(b []byte) (int, error) { - if nrg.ca { // can abort without waiting, since this gate is already proven - // not to be newest - return len(b), nil - } - nrg.nrm.wg.Wait() - if nrg.nrm.getNewest() == nrg.i { - if len(nrg.wh) > 0 { - headers.Merge(nrg.ResponseWriter.Header(), nrg.wh) - nrg.wh = nil + // Write the response with the newest Last-Modified + if newestIdx >= 0 && newestIdx < len(captures) && captures[newestIdx] != nil { + crw := captures[newestIdx] + headers.Merge(w.Header(), crw.Header()) + statusCode := crw.StatusCode() + w.WriteHeader(statusCode) + w.Write(crw.Body()) + + // Cancel all other contexts + for j, cancel := range cancels { + if j != newestIdx { + cancel() + } + } + } else { + // No valid response found, use the first available + for i, crw := range captures { + if crw != nil { + headers.Merge(w.Header(), crw.Header()) + statusCode := crw.StatusCode() + w.WriteHeader(statusCode) + w.Write(crw.Body()) + + // Cancel all other contexts + for j, cancel := range cancels { + if j != i { + cancel() + } + } + break + } } - nrg.ResponseWriter.WriteHeader(int(nrg.s)) - nrg.ResponseWriter.Write(b) } - return len(b), nil } diff --git a/pkg/backends/alb/mech/nlm/newest_last_modified_test.go b/pkg/backends/alb/mech/nlm/newest_last_modified_test.go index fc4a42ad0..2cb2d11ad 100644 --- a/pkg/backends/alb/mech/nlm/newest_last_modified_test.go +++ b/pkg/backends/alb/mech/nlm/newest_last_modified_test.go @@ -66,18 +66,3 @@ func TestHandleNewestResponse(t *testing.T) { } } -func TestWriteHeader(t *testing.T) { - w := httptest.NewRecorder() - nrm := &newestResponseMux{} - nrm.wg.Add(1) - nrg := newNewestResponseGate(w, 0, nrm) - nrg.WriteHeader(200) -} - -func TestRegisterLM(t *testing.T) { - nrm := &newestResponseMux{} - b := nrm.registerLM(0, time.Time{}) - if b { - t.Error("expected false") - } -} From c5d6ec2eacbd7d13f23327557729dee6be4c9993 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:43:58 -0700 Subject: [PATCH 12/28] add Pool() to ur mech Signed-off-by: James Ranson --- pkg/backends/alb/mech/ur/user_router.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/backends/alb/mech/ur/user_router.go b/pkg/backends/alb/mech/ur/user_router.go index 799a64173..2aaa96ecd 100644 --- a/pkg/backends/alb/mech/ur/user_router.go +++ b/pkg/backends/alb/mech/ur/user_router.go @@ -134,3 +134,4 @@ func (h *Handler) handleDefault(w http.ResponseWriter, r *http.Request) { // stubs for unused interface functions func (h *Handler) SetPool(_ pool.Pool) {} func (h *Handler) StopPool() {} +func (h *Handler) Pool() pool.Pool { return nil } From 681d87ddbf010fbe758b33e0a989ebff588469fc Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:44:31 -0700 Subject: [PATCH 13/28] remove unused struct Signed-off-by: James Ranson --- pkg/backends/alb/mech/fr/responder_claim.go | 53 ------------------- .../alb/mech/fr/responder_claim_test.go | 48 ----------------- 2 files changed, 101 deletions(-) delete mode 100644 pkg/backends/alb/mech/fr/responder_claim.go delete mode 100644 pkg/backends/alb/mech/fr/responder_claim_test.go diff --git a/pkg/backends/alb/mech/fr/responder_claim.go b/pkg/backends/alb/mech/fr/responder_claim.go deleted file mode 100644 index 881a8c3d2..000000000 --- a/pkg/backends/alb/mech/fr/responder_claim.go +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright 2018 The Trickster Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package fr - -import ( - "context" - "sync/atomic" -) - -// responderClaim is a construct that allows only the first claimant -// to the response to act as the downstream responder -type responderClaim struct { - lockVal int64 - contexts []context.Context -} - -func newResponderClaim(sz int) *responderClaim { - contexts := make([]context.Context, sz) - for i := range sz { - contexts[i] = context.Background() - } - return &responderClaim{lockVal: -1, contexts: contexts} -} - -func (rc *responderClaim) Claim(i int64) bool { - if atomic.LoadInt64(&rc.lockVal) == i { - return true - } - if atomic.CompareAndSwapInt64(&rc.lockVal, -1, i) { - for j, ctx := range rc.contexts { - if int64(j) != i { - go ctx.Done() - } - } - return true - } - rc.contexts[i].Done() - return false -} diff --git a/pkg/backends/alb/mech/fr/responder_claim_test.go b/pkg/backends/alb/mech/fr/responder_claim_test.go deleted file mode 100644 index 692f1e4e2..000000000 --- a/pkg/backends/alb/mech/fr/responder_claim_test.go +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright 2018 The Trickster Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package fr - -import "testing" - -func TestNewResponderClaim(t *testing.T) { - rc := newResponderClaim(1) - if len(rc.contexts) != 1 { - t.Error("expected 1 got ", len(rc.contexts)) - } - if rc.lockVal != -1 { - t.Error("expected -1 got ", rc.lockVal) - } -} - -func TestClaim(t *testing.T) { - rc := newResponderClaim(2) - - b := rc.Claim(1) - if !b { - t.Error("expected true") - } - - b = rc.Claim(1) - if !b { - t.Error("expected true") - } - - b = rc.Claim(0) - if b { - t.Error("expected false") - } -} From fe77a150e7b48a2e85ad9eee736e5a63039df75f Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 12:49:59 -0700 Subject: [PATCH 14/28] linter Signed-off-by: James Ranson --- pkg/backends/alb/mech/nlm/newest_last_modified.go | 3 ++- pkg/backends/alb/pool/target.go | 3 ++- pkg/backends/options/options.go | 4 +++- pkg/backends/prometheus/model/merge.go | 9 ++++++--- pkg/backends/prometheus/transformations.go | 3 ++- pkg/proxy/paths/options/options.go | 7 ++----- pkg/proxy/response/merge/timeseries.go | 14 +++++++------- 7 files changed, 24 insertions(+), 19 deletions(-) diff --git a/pkg/backends/alb/mech/nlm/newest_last_modified.go b/pkg/backends/alb/mech/nlm/newest_last_modified.go index a7904910c..f0443f280 100644 --- a/pkg/backends/alb/mech/nlm/newest_last_modified.go +++ b/pkg/backends/alb/mech/nlm/newest_last_modified.go @@ -57,6 +57,7 @@ func (h *handler) SetPool(p pool.Pool) { func (h *handler) Pool() pool.Pool { return h.pool } + func (h *handler) ID() types.ID { return ID } @@ -92,7 +93,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Track the newest Last-Modified response - var newestIdx = -1 + newestIdx := -1 var newestTime time.Time var mu sync.Mutex diff --git a/pkg/backends/alb/pool/target.go b/pkg/backends/alb/pool/target.go index 11c8977f4..60e53dafc 100644 --- a/pkg/backends/alb/pool/target.go +++ b/pkg/backends/alb/pool/target.go @@ -54,7 +54,8 @@ func New(targets Targets, healthyFloor int) Pool { // NewTarget returns a new Target using the provided inputs func NewTarget(handler http.Handler, hcStatus *healthcheck.Status, - backend backends.Backend) *Target { + backend backends.Backend, +) *Target { return &Target{ hcStatus: hcStatus, handler: handler, diff --git a/pkg/backends/options/options.go b/pkg/backends/options/options.go index 1a4f3cc84..5d9eb9a36 100644 --- a/pkg/backends/options/options.go +++ b/pkg/backends/options/options.go @@ -514,7 +514,9 @@ func (l Lookup) Initialize() error { if !ncb.Contains(v.Provider) && v.CacheName == "" { v.CacheName = DefaultBackendCacheName } - v.Initialize(k) + if err := v.Initialize(k); err != nil { + return err + } if len(v.Paths) > 0 { err := v.Paths.Initialize() if err != nil { diff --git a/pkg/backends/prometheus/model/merge.go b/pkg/backends/prometheus/model/merge.go index 71612f13f..e94775780 100644 --- a/pkg/backends/prometheus/model/merge.go +++ b/pkg/backends/prometheus/model/merge.go @@ -29,7 +29,8 @@ import ( // MakeMergeFunc creates a MergeFunc for a specific type that implements Merge // The returned function accepts a type conforming to Mergeable[T] and merges it into the accumulator func MakeMergeFunc[T any, PT merge.Mergeable[T]](errorType string, - newInstance func() PT) merge.MergeFunc { + newInstance func() PT, +) merge.MergeFunc { return func(accum *merge.Accumulator, data any, idx int) error { var instance PT // Try to type assert to PT first @@ -68,7 +69,8 @@ func MakeMergeFunc[T any, PT merge.Mergeable[T]](errorType string, // MakeMergeFuncFromBytes creates a MergeFunc that accepts []byte and unmarshals it // This is a convenience function for call sites that still have []byte func MakeMergeFuncFromBytes[T any, PT merge.Mergeable[T]](errorType string, - newInstance func() PT) func(*merge.Accumulator, []byte, int) error { + newInstance func() PT, +) func(*merge.Accumulator, []byte, int) error { mergeFunc := MakeMergeFunc(errorType, newInstance) return func(accum *merge.Accumulator, body []byte, idx int) error { instance := newInstance() @@ -86,7 +88,8 @@ func MakeRespondFunc[T any, PT merge.MarshallerPtr[T]]( handleResult func(http.ResponseWriter, *http.Request, PT, int), ) merge.RespondFunc { return func(w http.ResponseWriter, r *http.Request, - accum *merge.Accumulator, statusCode int) { + accum *merge.Accumulator, statusCode int, + ) { generic := accum.GetGeneric() if generic == nil { return diff --git a/pkg/backends/prometheus/transformations.go b/pkg/backends/prometheus/transformations.go index e3ca9794f..72bb4987e 100644 --- a/pkg/backends/prometheus/transformations.go +++ b/pkg/backends/prometheus/transformations.go @@ -40,7 +40,8 @@ func (c *Client) ProcessTransformations(ts timeseries.Timeseries) { } func (c *Client) processVectorTransformations(w http.ResponseWriter, - body []byte, statusCode int, rsc *request.Resources) { + body []byte, statusCode int, rsc *request.Resources, +) { var trq *timeseries.TimeRangeQuery if rsc != nil && rsc.TimeRangeQuery != nil { trq = rsc.TimeRangeQuery diff --git a/pkg/proxy/paths/options/options.go b/pkg/proxy/paths/options/options.go index bbf7e1491..a4998c2cd 100644 --- a/pkg/proxy/paths/options/options.go +++ b/pkg/proxy/paths/options/options.go @@ -148,11 +148,8 @@ func (o *Options) Initialize(_ string) error { // Expand "*" to all HTTP methods // If "*" is present, it replaces all other methods - for _, method := range o.Methods { - if method == "*" { - o.Methods = methods.AllHTTPMethods() - break - } + if slices.Contains(o.Methods, "*") { + o.Methods = methods.AllHTTPMethods() } if o.MatchTypeName == "" { diff --git a/pkg/proxy/response/merge/timeseries.go b/pkg/proxy/response/merge/timeseries.go index 8f50e5799..1509a3645 100644 --- a/pkg/proxy/response/merge/timeseries.go +++ b/pkg/proxy/response/merge/timeseries.go @@ -31,16 +31,16 @@ func TimeseriesMergeFunc(unmarshaler timeseries.UnmarshalerFunc) MergeFunc { ts, ok := data.(timeseries.Timeseries) if !ok { // If data is []byte, unmarshal it first (for backward compatibility during transition) - if body, ok := data.([]byte); ok { - var err error - ts, err = unmarshaler(body, nil) - if err != nil { - return err - } - } else { + body, ok := data.([]byte) + if !ok { // Not a timeseries and not []byte return nil } + var err error + ts, err = unmarshaler(body, nil) + if err != nil { + return err + } } if accum.tsdata == nil { accum.tsdata = ts From fc3351edce09d1ae1dd75451e98a2575cde57883 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 13:15:00 -0700 Subject: [PATCH 15/28] fix tests Signed-off-by: James Ranson --- .../alb/mech/tsm/time_series_merge_test.go | 5 - pkg/backends/prometheus/model/alerts_test.go | 143 +++------ pkg/backends/prometheus/model/labels_test.go | 144 +++------ pkg/backends/prometheus/model/series_test.go | 146 +++------- pkg/backends/prometheus/model/vector_test.go | 122 ++------ .../prometheus/transformations_test.go | 11 +- pkg/cache/options/options_test.go | 2 +- pkg/cache/providers/providers_test.go | 4 +- pkg/config/loader_test.go | 273 +++++++++++------- pkg/config/testdata/example.alb.yaml | 5 + pkg/config/testdata/example.auth.yaml | 3 + pkg/config/testdata/example.full.yaml | 1 + pkg/config/testdata/exmple.sharding.yaml | 2 + pkg/config/testdata/simple.prometheus.yaml | 1 + .../testdata/simple.reverseproxycache.yaml | 1 + 15 files changed, 336 insertions(+), 527 deletions(-) diff --git a/pkg/backends/alb/mech/tsm/time_series_merge_test.go b/pkg/backends/alb/mech/tsm/time_series_merge_test.go index e2a69277c..f4a7f66d5 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge_test.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge_test.go @@ -26,21 +26,16 @@ import ( "github.com/trickstercache/trickster/v2/pkg/observability/logging" "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" "github.com/trickstercache/trickster/v2/pkg/proxy/request" - "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" tu "github.com/trickstercache/trickster/v2/pkg/testutil" "github.com/trickstercache/trickster/v2/pkg/testutil/albpool" ) var testLogger = logging.NoopLogger() -func testMergeFunc(w http.ResponseWriter, r *http.Request, rgs merge.ResponseGates) { -} - func TestHandleResponseMerge(t *testing.T) { logger.SetLogger(testLogger) r, _ := http.NewRequest("GET", "http://trickstercache.org/", nil) rsc := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc.MergeFunc = testMergeFunc rsc.IsMergeMember = true r = request.SetResources(r, rsc) diff --git a/pkg/backends/prometheus/model/alerts_test.go b/pkg/backends/prometheus/model/alerts_test.go index 56336a0e2..e058e683e 100644 --- a/pkg/backends/prometheus/model/alerts_test.go +++ b/pkg/backends/prometheus/model/alerts_test.go @@ -17,11 +17,8 @@ package model import ( - "bytes" - "io" "net/http" "net/http/httptest" - "strconv" "testing" "github.com/trickstercache/trickster/v2/pkg/observability/logging" @@ -32,7 +29,6 @@ import ( ) var ( - testLogger = logging.NoopLogger() testResources = request.NewResources(nil, nil, nil, nil, nil, nil) ) @@ -93,123 +89,62 @@ func newTestReq() *http.Request { func TestMergeAndWriteAlerts(t *testing.T) { logger.SetLogger(logging.ConsoleLogger(level.Error)) - var nilRG *merge.ResponseGate tests := []struct { - r *http.Request - rgs merge.ResponseGates - expCode int + name string + bodies [][]byte + expCode int + hasError bool }{ { - newTestReq(), - nil, - http.StatusBadGateway, + name: "nil bodies", + bodies: nil, + expCode: http.StatusOK, + hasError: false, }, { - newTestReq(), - merge.ResponseGates{nilRG}, - http.StatusBadGateway, + name: "empty bodies", + bodies: [][]byte{}, + expCode: http.StatusOK, + hasError: false, }, { - newTestReq(), - testResponseGates1(), - http.StatusOK, + name: "valid merge", + bodies: [][]byte{ + []byte(`{"status":"success","data":{"alerts":[{"state":"test","labels":{},"annotations":{},"value":"x","activeAt":"y"}]}}`), + []byte(`{"stat`), + []byte(`{"status":"success","data":{"alerts":[]}}`), + }, + expCode: http.StatusOK, + hasError: false, }, { - newTestReq(), - testResponseGates2(), - http.StatusBadRequest, + name: "error status", + bodies: [][]byte{ + []byte(`{"status":"error","data":{"alerts":[]}}`), + []byte(`{"status":"error","data":{"alerts":[]}}`), + }, + expCode: http.StatusBadRequest, + hasError: false, }, } - for i, test := range tests { - t.Run(strconv.Itoa(i), func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { w := httptest.NewRecorder() - MergeAndWriteAlerts(w, request.SetResources(test.r, testResources), test.rgs) + r := request.SetResources(newTestReq(), testResources) + + accum := merge.NewAccumulator() + mergeFunc := MergeAndWriteAlertsMergeFunc() + respondFunc := MergeAndWriteAlertsRespondFunc() + for i, body := range test.bodies { + _ = mergeFunc(accum, body, i) + } + respondFunc(w, r, accum, test.expCode) + if w.Code != test.expCode { t.Errorf("expected %d got %d", test.expCode, w.Code) } }) } } - -func testResponseGates1() merge.ResponseGates { - logger.SetLogger(logging.ConsoleLogger(level.Error)) - b1 := []byte(`{"status":"success","data":{"alerts":[ - {"state":"test","labels":{},"annotations":{},"value":"x","activeAt":"y"} - ]}}`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 200, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2bad := []byte(`{"stat`) - closer2 := io.NopCloser(bytes.NewReader(b2bad)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 200, - } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc2, - ) - rg2.Write(b2bad) - - b3 := []byte(`{"status":"success","data":{"alerts":[]}}`) - closer3 := io.NopCloser(bytes.NewReader(b3)) - rsc3 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc3.Response = &http.Response{ - Body: closer3, - StatusCode: 200, - } - rg3 := merge.NewResponseGate( - nil, // w - nil, // r - rsc3, - ) - rg3.Write(b3) - - return merge.ResponseGates{rg1, rg2, rg3} -} - -func testResponseGates2() merge.ResponseGates { - b1 := []byte(`{"status":"error","data":{"alerts":[]}}`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 400, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2 := []byte(`{"status":"error","data":{"alerts":[]}}`) - closer2 := io.NopCloser(bytes.NewReader(b1)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 400, - } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg2.Write(b2) - - return merge.ResponseGates{rg1, rg2} -} diff --git a/pkg/backends/prometheus/model/labels_test.go b/pkg/backends/prometheus/model/labels_test.go index 5d5ae997a..2f9716ee6 100644 --- a/pkg/backends/prometheus/model/labels_test.go +++ b/pkg/backends/prometheus/model/labels_test.go @@ -17,15 +17,10 @@ package model import ( - "bytes" - "io" "net/http" "net/http/httptest" - "strconv" "testing" - "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" - "github.com/trickstercache/trickster/v2/pkg/proxy/request" "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" ) @@ -74,121 +69,56 @@ func TestMergeLabelData(t *testing.T) { } func TestMergeAndWriteLabelData(t *testing.T) { - var nilRG *merge.ResponseGate - tests := []struct { - r *http.Request - rgs merge.ResponseGates + name string + bodies [][]byte expCode int }{ - { // 0 - nil, - nil, - http.StatusBadGateway, + { + name: "nil bodies", + bodies: nil, + expCode: http.StatusOK, }, - { // 1 - nil, - merge.ResponseGates{nilRG}, - http.StatusBadGateway, + { + name: "empty bodies", + bodies: [][]byte{}, + expCode: http.StatusOK, }, - { // 2 - nil, - testResponseGates3(), - http.StatusOK, + { + name: "valid merge", + bodies: [][]byte{ + []byte(`{"status":"success","data":["test", "trickster"]}`), + []byte(`{"stat`), + []byte(`{"status":"success","data":["test2", "trickster2"]}`), + }, + expCode: http.StatusOK, }, - { // 3 - nil, - testResponseGates4(), - http.StatusBadRequest, + { + name: "error status", + bodies: [][]byte{ + []byte(`{"status":"error"`), + []byte(`{"status":"error","data":["should", "not", "append"]`), + }, + expCode: http.StatusOK, }, } - for i, test := range tests { - t.Run(strconv.Itoa(i), func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { w := httptest.NewRecorder() - MergeAndWriteLabelData(w, test.r, test.rgs) + r, _ := http.NewRequest("GET", "/", nil) + + accum := merge.NewAccumulator() + mergeFunc := MergeAndWriteLabelDataMergeFunc() + respondFunc := MergeAndWriteLabelDataRespondFunc() + for i, body := range test.bodies { + _ = mergeFunc(accum, body, i) + } + respondFunc(w, r, accum, test.expCode) + if w.Code != test.expCode { t.Errorf("expected %d got %d", test.expCode, w.Code) } }) } } - -func testResponseGates3() merge.ResponseGates { - logger.SetLogger(testLogger) - b1 := []byte(`{"status":"success","data":["test", "trickster"]}`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 200, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2bad := []byte(`{"stat`) - closer2 := io.NopCloser(bytes.NewReader(b2bad)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 200, - } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc2, - ) - rg2.Write(b2bad) - - b3 := []byte(`{"status":"success","data":["test2", "trickster2"]}`) - closer3 := io.NopCloser(bytes.NewReader(b3)) - rsc3 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc3.Response = &http.Response{ - Body: closer3, - StatusCode: 200, - } - rg3 := merge.NewResponseGate( - nil, // w - nil, // r - rsc3, - ) - rg3.Write(b3) - - return merge.ResponseGates{rg1, rg2, rg3} -} - -func testResponseGates4() merge.ResponseGates { - b1 := []byte(`{"status":"error"`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 400, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2 := []byte(`{"status":"error","data":["should", "not", "append"]`) - closer2 := io.NopCloser(bytes.NewReader(b1)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 400, - } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg2.Write(b2) - - return merge.ResponseGates{rg1, rg2} -} diff --git a/pkg/backends/prometheus/model/series_test.go b/pkg/backends/prometheus/model/series_test.go index c97c4eb5f..2107bcd36 100644 --- a/pkg/backends/prometheus/model/series_test.go +++ b/pkg/backends/prometheus/model/series_test.go @@ -17,16 +17,11 @@ package model import ( - "bytes" "encoding/json" - "io" "net/http" "net/http/httptest" - "strconv" "testing" - "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" - "github.com/trickstercache/trickster/v2/pkg/proxy/request" "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" ) @@ -130,122 +125,57 @@ func TestSeries(t *testing.T) { } func TestMergeAndWriteSeries(t *testing.T) { - var nilRG *merge.ResponseGate - tests := []struct { - r *http.Request - rgs merge.ResponseGates + name string + bodies [][]byte expCode int }{ - { // 0 - nil, - nil, - http.StatusBadGateway, + { + name: "nil bodies", + bodies: nil, + expCode: http.StatusOK, }, - { // 1 - nil, - merge.ResponseGates{nilRG}, - http.StatusBadGateway, + { + name: "empty bodies", + bodies: [][]byte{}, + expCode: http.StatusOK, }, - { // 2 - nil, - testResponseGates5(), - http.StatusOK, + { + name: "valid merge", + bodies: [][]byte{ + []byte(`{"status":"success","data":[{"__name__":"test1","instance":"i1","job":"trickster"}]}`), + []byte(`{"stat`), + []byte(`{"status":"success","data":[{"__name__":"test1","instance":"i2","job":"trickster"}]}`), + }, + expCode: http.StatusOK, }, - { // 3 - nil, - testResponseGates6(), - http.StatusBadRequest, + { + name: "error status", + bodies: [][]byte{ + []byte(`{"status":"error"`), + []byte(`{"status":"error","data":[{"__name__":"should","instance":"not","job":"append"}]}`), + }, + expCode: http.StatusBadRequest, }, } - for i, test := range tests { - t.Run(strconv.Itoa(i), func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { w := httptest.NewRecorder() - MergeAndWriteSeries(w, test.r, test.rgs) + r, _ := http.NewRequest("GET", "/", nil) + accum := merge.NewAccumulator() + mergeFunc := MergeAndWriteSeriesMergeFunc() + respondFunc := MergeAndWriteSeriesRespondFunc() + + for i, body := range test.bodies { + _ = mergeFunc(accum, body, i) + } + + respondFunc(w, r, accum, test.expCode) + if w.Code != test.expCode { t.Errorf("expected %d got %d", test.expCode, w.Code) } }) } } - -func testResponseGates5() merge.ResponseGates { - logger.SetLogger(testLogger) - b1 := []byte(`{"status":"success","data":[{"__name__":"test1","instance":"i1","job":"trickster"}]}`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 200, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2bad := []byte(`{"stat`) - closer2 := io.NopCloser(bytes.NewReader(b2bad)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 200, - } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc2, - ) - rg2.Write(b2bad) - - b3 := []byte(`{"status":"success","data":[{"__name__":"test1","instance":"i2","job":"trickster"}]}`) - closer3 := io.NopCloser(bytes.NewReader(b3)) - rsc3 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc3.Response = &http.Response{ - Body: closer3, - StatusCode: 200, - } - rg3 := merge.NewResponseGate( - nil, // w - nil, // r - rsc3, - ) - rg3.Write(b3) - - return merge.ResponseGates{rg1, rg2, rg3} -} - -func testResponseGates6() merge.ResponseGates { - logger.SetLogger(testLogger) - b1 := []byte(`{"status":"error"`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 400, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2 := []byte(`{"status":"error","data":[{"__name__":"should","instance":"not","job":"append"}]}`) - closer2 := io.NopCloser(bytes.NewReader(b1)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 400, - } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg2.Write(b2) - - return merge.ResponseGates{rg1, rg2} -} diff --git a/pkg/backends/prometheus/model/vector_test.go b/pkg/backends/prometheus/model/vector_test.go index eff74db5f..718f6889b 100644 --- a/pkg/backends/prometheus/model/vector_test.go +++ b/pkg/backends/prometheus/model/vector_test.go @@ -17,14 +17,10 @@ package model import ( - "bytes" - "io" "net/http" "net/http/httptest" "testing" - "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" - "github.com/trickstercache/trickster/v2/pkg/proxy/request" "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" "github.com/trickstercache/trickster/v2/pkg/timeseries" ) @@ -40,104 +36,50 @@ const testVector2 = `{"status":"success","data":{"resultType":"vector","result": `"value":[1577836800,"1"]}]}}` func TestMergeAndWriteVector(t *testing.T) { + unmarshaler := func(data []byte, trq *timeseries.TimeRangeQuery) (timeseries.Timeseries, error) { + if trq == nil { + trq = ×eries.TimeRangeQuery{} + } + return UnmarshalTimeseries(data, trq) + } + marshaler := MarshalTimeseriesWriter + mergeFunc := MergeAndWriteVectorMergeFunc(unmarshaler) + respondFunc := MergeAndWriteVectorRespondFunc(marshaler) + w := httptest.NewRecorder() - MergeAndWriteVector(w, nil, nil) + r, _ := http.NewRequest("GET", "/", nil) + accum := merge.NewAccumulator() + respondFunc(w, r, accum, http.StatusOK) if w.Code != http.StatusBadGateway { t.Errorf("expected %d got %d", http.StatusBadGateway, w.Code) } w = httptest.NewRecorder() - MergeAndWriteVector(w, nil, testResponseGates7()) + accum = merge.NewAccumulator() + err := mergeFunc(accum, []byte(testVector), 0) + if err != nil { + t.Errorf("unexpected error merging first vector: %v", err) + } + _ = mergeFunc(accum, []byte(`{"stat`), 1) // bad JSON, should be skipped (error ignored) + err = mergeFunc(accum, []byte(testVector2), 2) + if err != nil { + t.Errorf("unexpected error merging second vector: %v", err) + } + respondFunc(w, r, accum, http.StatusOK) if w.Code != http.StatusOK { t.Errorf("expected %d got %d", http.StatusOK, w.Code) } w = httptest.NewRecorder() - MergeAndWriteVector(w, nil, testResponseGates8()) - if w.Code != http.StatusBadRequest { - t.Errorf("expected %d got %d", http.StatusOK, w.Code) - } -} - -func testResponseGates7() merge.ResponseGates { - logger.SetLogger(testLogger) - b1 := []byte(testVector) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 200, - } - rsc1.TimeRangeQuery = ×eries.TimeRangeQuery{} - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2bad := []byte(`{"stat`) - closer2 := io.NopCloser(bytes.NewReader(b2bad)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 200, + accum = merge.NewAccumulator() + err = mergeFunc(accum, []byte(`{"status":"error","data":{}}`), 0) + if err != nil { + t.Errorf("unexpected error: %v", err) } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc2, - ) - rg2.Write(b2bad) - - b3 := []byte(testVector2) - closer3 := io.NopCloser(bytes.NewReader(b3)) - rsc3 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc3.Response = &http.Response{ - Body: closer3, - StatusCode: 200, - } - rg3 := merge.NewResponseGate( - nil, // w - nil, // r - rsc3, - ) - rg3.Write(b3) - - var rg4 *merge.ResponseGate - - return merge.ResponseGates{rg1, rg2, rg4, rg3} -} - -func testResponseGates8() merge.ResponseGates { - logger.SetLogger(testLogger) - b1 := []byte(`{"status":"error","data":{}`) - closer1 := io.NopCloser(bytes.NewReader(b1)) - rsc1 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc1.Response = &http.Response{ - Body: closer1, - StatusCode: 400, - } - rg1 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg1.Write(b1) - - b2 := []byte(`{"status":"error","data":{}`) - closer2 := io.NopCloser(bytes.NewReader(b1)) - rsc2 := request.NewResources(nil, nil, nil, nil, nil, nil) - rsc2.Response = &http.Response{ - Body: closer2, - StatusCode: 400, + respondFunc(w, r, accum, http.StatusOK) + // Error status in envelope doesn't change HTTP status code + if w.Code != http.StatusOK { + t.Errorf("expected %d got %d", http.StatusOK, w.Code) } - rg2 := merge.NewResponseGate( - nil, // w - nil, // r - rsc1, - ) - rg2.Write(b2) - return merge.ResponseGates{rg1, rg2} } diff --git a/pkg/backends/prometheus/transformations_test.go b/pkg/backends/prometheus/transformations_test.go index 2cd94c746..4f8ff296a 100644 --- a/pkg/backends/prometheus/transformations_test.go +++ b/pkg/backends/prometheus/transformations_test.go @@ -17,14 +17,12 @@ package prometheus import ( - "net/http" "net/http/httptest" "testing" "github.com/trickstercache/trickster/v2/pkg/observability/logging" "github.com/trickstercache/trickster/v2/pkg/observability/logging/logger" "github.com/trickstercache/trickster/v2/pkg/proxy/request" - "github.com/trickstercache/trickster/v2/pkg/proxy/response/merge" "github.com/trickstercache/trickster/v2/pkg/timeseries/dataset" ) @@ -51,14 +49,11 @@ func TestProcessVectorTransformations(t *testing.T) { logger.SetLogger(testLogger) c := &Client{} w := httptest.NewRecorder() - r, _ := http.NewRequest(http.MethodGet, "http://example.com/", nil) - resp := &http.Response{StatusCode: 200} rsc := &request.Resources{} - rg := merge.NewResponseGate(w, r, rsc) - rg.Response = resp - rg.Write([]byte("trickster")) - c.processVectorTransformations(w, rg) + body := []byte("trickster") + statusCode := 200 + c.processVectorTransformations(w, body, statusCode, rsc) if w.Code != 200 { t.Errorf("expected %d got %d", 200, w.Code) } diff --git a/pkg/cache/options/options_test.go b/pkg/cache/options/options_test.go index 2ebcedc84..5f1dc3219 100644 --- a/pkg/cache/options/options_test.go +++ b/pkg/cache/options/options_test.go @@ -70,7 +70,7 @@ func TestInitialize(t *testing.T) { options: func() *Options { o := New() o.Provider = "redis" - o.ProviderID = providers.Redis + o.ProviderID = providers.RedisID o.Redis.ClientType = "standard" o.Redis.Endpoint = "127.0.0.1:6379" return o diff --git a/pkg/cache/providers/providers_test.go b/pkg/cache/providers/providers_test.go index 2fd52de1c..0436d2156 100644 --- a/pkg/cache/providers/providers_test.go +++ b/pkg/cache/providers/providers_test.go @@ -21,8 +21,8 @@ import ( ) func TestProviderIDString(t *testing.T) { - t1 := Memory - t2 := Filesystem + t1 := MemoryID + t2 := FilesystemID var t3 Provider = 13 if t1.String() != "memory" { diff --git a/pkg/config/loader_test.go b/pkg/config/loader_test.go index 3bcc706e5..ac19d801c 100644 --- a/pkg/config/loader_test.go +++ b/pkg/config/loader_test.go @@ -25,6 +25,7 @@ import ( "github.com/trickstercache/trickster/v2/pkg/backends/providers" "github.com/trickstercache/trickster/v2/pkg/cache/evictionmethods" + cacheproviders "github.com/trickstercache/trickster/v2/pkg/cache/providers" "github.com/trickstercache/trickster/v2/pkg/errors" tlstest "github.com/trickstercache/trickster/v2/pkg/testutil/tls" ) @@ -247,32 +248,38 @@ func TestFullLoadConfiguration(t *testing.T) { t.Errorf("expected redis, got %s", c.Provider) } - if c.Index.ReapInterval != 4000 { - t.Errorf("expected 4000, got %d", c.Index.ReapInterval) - } - - if c.Index.FlushInterval != 6000 { - t.Errorf("expected 6000, got %d", c.Index.FlushInterval) - } + if cacheproviders.UsesIndex(c.Provider) { + if c.Index == nil { + t.Errorf("expected index for provider %s, got nil", c.Provider) + } else { + if c.Index.ReapInterval != 4000 { + t.Errorf("expected 4000, got %d", c.Index.ReapInterval) + } - if c.Index.MaxSizeBytes != 536870913 { - t.Errorf("expected 536870913, got %d", c.Index.MaxSizeBytes) - } + if c.Index.FlushInterval != 6000 { + t.Errorf("expected 6000, got %d", c.Index.FlushInterval) + } - if c.Index.MaxSizeBackoffBytes != 16777217 { - t.Errorf("expected 16777217, got %d", c.Index.MaxSizeBackoffBytes) - } + if c.Index.MaxSizeBytes != 536870913 { + t.Errorf("expected 536870913, got %d", c.Index.MaxSizeBytes) + } - if c.Index.MaxSizeObjects != 80 { - t.Errorf("expected 80, got %d", c.Index.MaxSizeObjects) - } + if c.Index.MaxSizeBackoffBytes != 16777217 { + t.Errorf("expected 16777217, got %d", c.Index.MaxSizeBackoffBytes) + } - if c.Index.MaxSizeBackoffObjects != 20 { - t.Errorf("expected 20, got %d", c.Index.MaxSizeBackoffObjects) - } + if c.Index.MaxSizeObjects != 80 { + t.Errorf("expected 80, got %d", c.Index.MaxSizeObjects) + } - if c.Index.ReapInterval != 4000 { - t.Errorf("expected 4000, got %d", c.Index.ReapInterval) + if c.Index.MaxSizeBackoffObjects != 20 { + t.Errorf("expected 20, got %d", c.Index.MaxSizeBackoffObjects) + } + } + } else { + if c.Index != nil { + t.Errorf("expected nil index for provider %s, got non-nil", c.Provider) + } } if c.Redis.ClientType != "test_redis_type" { @@ -343,24 +350,40 @@ func TestFullLoadConfiguration(t *testing.T) { t.Errorf("expected 300001, got %d", c.Redis.ConnMaxIdleTime) } - if c.Filesystem.CachePath != "test_cache_path" { - t.Errorf("expected test_cache_path, got %s", c.Filesystem.CachePath) - } - - if c.BBolt.Filename != "test_filename" { - t.Errorf("expected test_filename, got %s", c.BBolt.Filename) - } + if c.Provider == "redis" { + if c.Filesystem != nil { + t.Errorf("expected nil Filesystem for redis provider, got non-nil") + } + if c.BBolt != nil { + t.Errorf("expected nil BBolt for redis provider, got non-nil") + } + if c.Badger != nil { + t.Errorf("expected nil Badger for redis provider, got non-nil") + } + } else { + if c.Filesystem != nil && c.Filesystem.CachePath != "test_cache_path" { + t.Errorf("expected test_cache_path, got %s", c.Filesystem.CachePath) + } + + if c.BBolt != nil { + if c.BBolt.Filename != "test_filename" { + t.Errorf("expected test_filename, got %s", c.BBolt.Filename) + } - if c.BBolt.Bucket != "test_bucket" { - t.Errorf("expected test_bucket, got %s", c.BBolt.Bucket) - } + if c.BBolt.Bucket != "test_bucket" { + t.Errorf("expected test_bucket, got %s", c.BBolt.Bucket) + } + } - if c.Badger.Directory != "test_directory" { - t.Errorf("expected test_directory, got %s", c.Badger.Directory) - } + if c.Badger != nil { + if c.Badger.Directory != "test_directory" { + t.Errorf("expected test_directory, got %s", c.Badger.Directory) + } - if c.Badger.ValueDirectory != "test_value_directory" { - t.Errorf("expected test_value_directory, got %s", c.Badger.ValueDirectory) + if c.Badger.ValueDirectory != "test_value_directory" { + t.Errorf("expected test_value_directory, got %s", c.Badger.ValueDirectory) + } + } } } @@ -412,88 +435,134 @@ func TestEmptyLoadConfiguration(t *testing.T) { return } - if c.Index.ReapInterval != 3000*time.Millisecond { - t.Errorf("expected 3000, got %d", c.Index.ReapInterval) - } - - if c.Redis.Endpoint != "redis:6379" { - t.Errorf("expected redis:6379, got %s", c.Redis.Endpoint) - } - - if c.Redis.SentinelMaster != "" { - t.Errorf("expected '', got %s", c.Redis.SentinelMaster) - } - - if c.Redis.Password != "" { - t.Errorf("expected '', got %s", c.Redis.Password) - } - - if c.Redis.DB != 0 { - t.Errorf("expected 0, got %d", c.Redis.DB) - } + if cacheproviders.UsesIndex(c.Provider) { + if c.Index == nil { + t.Errorf("expected index for provider %s, got nil", c.Provider) + } else if c.Index.ReapInterval != 3000*time.Millisecond { + t.Errorf("expected 3000, got %d", c.Index.ReapInterval) + } + } else { + if c.Index != nil { + t.Errorf("expected nil index for provider %s, got non-nil", c.Provider) + } + } + + if c.Provider == "redis" { + if c.Redis == nil { + t.Errorf("expected redis config for redis provider, got nil") + } else { + if c.Redis.Endpoint != "redis:6379" { + t.Errorf("expected redis:6379, got %s", c.Redis.Endpoint) + } - if c.Redis.MaxRetries != 0 { - t.Errorf("expected 0, got %d", c.Redis.MaxRetries) - } + if c.Redis.SentinelMaster != "" { + t.Errorf("expected '', got %s", c.Redis.SentinelMaster) + } - if c.Redis.MinRetryBackoff != 0 { - t.Errorf("expected 0, got %d", c.Redis.MinRetryBackoff) - } + if c.Redis.Password != "" { + t.Errorf("expected '', got %s", c.Redis.Password) + } - if c.Redis.MaxRetryBackoff != 0 { - t.Errorf("expected 0, got %d", c.Redis.MaxRetryBackoff) - } + if c.Redis.DB != 0 { + t.Errorf("expected 0, got %d", c.Redis.DB) + } - if c.Redis.DialTimeout != 0 { - t.Errorf("expected 0, got %d", c.Redis.DialTimeout) - } + if c.Redis.MaxRetries != 0 { + t.Errorf("expected 0, got %d", c.Redis.MaxRetries) + } - if c.Redis.ReadTimeout != 0 { - t.Errorf("expected 0, got %d", c.Redis.ReadTimeout) - } + if c.Redis.MinRetryBackoff != 0 { + t.Errorf("expected 0, got %d", c.Redis.MinRetryBackoff) + } - if c.Redis.WriteTimeout != 0 { - t.Errorf("expected 0, got %d", c.Redis.WriteTimeout) - } + if c.Redis.MaxRetryBackoff != 0 { + t.Errorf("expected 0, got %d", c.Redis.MaxRetryBackoff) + } - if c.Redis.PoolSize != 0 { - t.Errorf("expected 0, got %d", c.Redis.PoolSize) - } + if c.Redis.DialTimeout != 0 { + t.Errorf("expected 0, got %d", c.Redis.DialTimeout) + } - if c.Redis.MinIdleConns != 0 { - t.Errorf("expected 0, got %d", c.Redis.PoolSize) - } + if c.Redis.ReadTimeout != 0 { + t.Errorf("expected 0, got %d", c.Redis.ReadTimeout) + } - if c.Redis.ConnMaxLifetime != 0 { - t.Errorf("expected 0, got %d", c.Redis.ConnMaxLifetime) - } + if c.Redis.WriteTimeout != 0 { + t.Errorf("expected 0, got %d", c.Redis.WriteTimeout) + } - if c.Redis.PoolTimeout != 0 { - t.Errorf("expected 0, got %d", c.Redis.PoolTimeout) - } + if c.Redis.PoolSize != 0 { + t.Errorf("expected 0, got %d", c.Redis.PoolSize) + } - if c.Redis.ConnMaxIdleTime != 0 { - t.Errorf("expected 0, got %d", c.Redis.ConnMaxIdleTime) - } + if c.Redis.MinIdleConns != 0 { + t.Errorf("expected 0, got %d", c.Redis.PoolSize) + } - if c.Filesystem.CachePath != "/tmp/trickster" { - t.Errorf("expected /tmp/trickster, got %s", c.Filesystem.CachePath) - } + if c.Redis.ConnMaxLifetime != 0 { + t.Errorf("expected 0, got %d", c.Redis.ConnMaxLifetime) + } - if c.BBolt.Filename != "trickster.db" { - t.Errorf("expected trickster.db, got %s", c.BBolt.Filename) - } + if c.Redis.PoolTimeout != 0 { + t.Errorf("expected 0, got %d", c.Redis.PoolTimeout) + } - if c.BBolt.Bucket != "trickster" { - t.Errorf("expected trickster, got %s", c.BBolt.Bucket) - } + if c.Redis.ConnMaxIdleTime != 0 { + t.Errorf("expected 0, got %d", c.Redis.ConnMaxIdleTime) + } + } + } else { + if c.Redis != nil { + t.Errorf("expected nil redis config for provider %s, got non-nil", c.Provider) + } + } + + if c.Provider == "filesystem" { + if c.Filesystem == nil { + t.Errorf("expected filesystem config for filesystem provider, got nil") + } else if c.Filesystem.CachePath != "/tmp/trickster" { + t.Errorf("expected /tmp/trickster, got %s", c.Filesystem.CachePath) + } + } else { + if c.Filesystem != nil { + t.Errorf("expected nil filesystem config for provider %s, got non-nil", c.Provider) + } + } + + if c.Provider == "bbolt" { + if c.BBolt == nil { + t.Errorf("expected bbolt config for bbolt provider, got nil") + } else { + if c.BBolt.Filename != "trickster.db" { + t.Errorf("expected trickster.db, got %s", c.BBolt.Filename) + } - if c.Badger.Directory != "/tmp/trickster" { - t.Errorf("expected /tmp/trickster, got %s", c.Badger.Directory) - } + if c.BBolt.Bucket != "trickster" { + t.Errorf("expected trickster, got %s", c.BBolt.Bucket) + } + } + } else { + if c.BBolt != nil { + t.Errorf("expected nil bbolt config for provider %s, got non-nil", c.Provider) + } + } + + if c.Provider == "badger" { + if c.Badger == nil { + t.Errorf("expected badger config for badger provider, got nil") + } else { + if c.Badger.Directory != "/tmp/trickster" { + t.Errorf("expected /tmp/trickster, got %s", c.Badger.Directory) + } - if c.Badger.ValueDirectory != "/tmp/trickster" { - t.Errorf("expected /tmp/trickster, got %s", c.Badger.ValueDirectory) + if c.Badger.ValueDirectory != "/tmp/trickster" { + t.Errorf("expected /tmp/trickster, got %s", c.Badger.ValueDirectory) + } + } + } else { + if c.Badger != nil { + t.Errorf("expected nil badger config for provider %s, got non-nil", c.Provider) + } } } diff --git a/pkg/config/testdata/example.alb.yaml b/pkg/config/testdata/example.alb.yaml index 836f6a0d2..c01db2c05 100644 --- a/pkg/config/testdata/example.alb.yaml +++ b/pkg/config/testdata/example.alb.yaml @@ -37,6 +37,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: origin1 healthcheck: interval: 1s path: /health @@ -70,6 +71,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: origin2 healthcheck: interval: 1s path: /health @@ -103,6 +105,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: prom1:9090 healthcheck: interval: 1s timeseries_retention_factor: 1024 @@ -135,6 +138,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: prom2:9090 healthcheck: interval: 1s timeseries_retention_factor: 1024 @@ -191,6 +195,7 @@ backends: pool: - prom1 - prom1 + output_format: prometheus tls: {} forwarded_headers: standard latency_min: 0s diff --git a/pkg/config/testdata/example.auth.yaml b/pkg/config/testdata/example.auth.yaml index 962631ed4..6c5b725d8 100644 --- a/pkg/config/testdata/example.auth.yaml +++ b/pkg/config/testdata/example.auth.yaml @@ -8,6 +8,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: example.com healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest @@ -40,6 +41,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: example.com healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest @@ -72,6 +74,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: example.com healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest diff --git a/pkg/config/testdata/example.full.yaml b/pkg/config/testdata/example.full.yaml index 51a769438..e85dd9289 100644 --- a/pkg/config/testdata/example.full.yaml +++ b/pkg/config/testdata/example.full.yaml @@ -8,6 +8,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: prometheus:9090 healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest diff --git a/pkg/config/testdata/exmple.sharding.yaml b/pkg/config/testdata/exmple.sharding.yaml index 45bad712f..08d21de52 100644 --- a/pkg/config/testdata/exmple.sharding.yaml +++ b/pkg/config/testdata/exmple.sharding.yaml @@ -8,6 +8,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: prometheus:9090 healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest @@ -28,6 +29,7 @@ backends: - application/javascript - application/xml tracing_name: default + shard_max_size_time: 2h0m0s shard_step: 2h0m0s tls: {} forwarded_headers: standard diff --git a/pkg/config/testdata/simple.prometheus.yaml b/pkg/config/testdata/simple.prometheus.yaml index de590ed42..b72714073 100644 --- a/pkg/config/testdata/simple.prometheus.yaml +++ b/pkg/config/testdata/simple.prometheus.yaml @@ -8,6 +8,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: prometheus:9090 healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest diff --git a/pkg/config/testdata/simple.reverseproxycache.yaml b/pkg/config/testdata/simple.reverseproxycache.yaml index 9f8583076..4e59ec3d1 100644 --- a/pkg/config/testdata/simple.reverseproxycache.yaml +++ b/pkg/config/testdata/simple.reverseproxycache.yaml @@ -8,6 +8,7 @@ backends: keep_alive_timeout: 5m0s max_idle_conns: 20 cache_name: default + cache_key_prefix: www.example.com healthcheck: {} timeseries_retention_factor: 1024 timeseries_eviction_method: oldest From 33fcb436371de461869389527fb681ed8bcf80f3 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 15:42:24 -0700 Subject: [PATCH 16/28] merge vectors Signed-off-by: James Ranson --- pkg/backends/prometheus/transformations.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/backends/prometheus/transformations.go b/pkg/backends/prometheus/transformations.go index 72bb4987e..51dcd7a41 100644 --- a/pkg/backends/prometheus/transformations.go +++ b/pkg/backends/prometheus/transformations.go @@ -58,6 +58,7 @@ func (c *Client) processVectorTransformations(w http.ResponseWriter, var requestOptions *timeseries.RequestOptions if rsc != nil { requestOptions = rsc.TSReqestOptions + rsc.TS = t2 } model.MarshalTSOrVectorWriter(ds, requestOptions, statusCode, w, true) } From 1957dc3fefe1185cb08550b5d4903a2f3c6808e2 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 16:25:41 -0700 Subject: [PATCH 17/28] add nil check Signed-off-by: James Ranson --- pkg/backends/alb/mech/tsm/time_series_merge.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/backends/alb/mech/tsm/time_series_merge.go b/pkg/backends/alb/mech/tsm/time_series_merge.go index f2a77470c..1999029e4 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge.go @@ -154,16 +154,21 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { r2, _ := request.Clone(r) rsc2 := &request.Resources{IsMergeMember: true} r2 = request.SetResources(r2, rsc2) - crw := capture.NewCaptureResponseWriter() hl[i].Handler().ServeHTTP(crw, r2) + rsc2 = request.GetResources(r2) + if rsc2 == nil { + logger.Warn("tsm gather failed due to nil resources", nil) + return + } + // Ensure merge functions are set on cloned request if rsc2.MergeFunc == nil || rsc2.MergeRespondFunc == nil { logger.Warn("tsm gather failed due to nil func", nil) } // As soon as response is complete, unmarshal and merge // This happens in parallel for each response as it arrives - if rsc2.MergeFunc != nil { + if rsc2.MergeFunc != nil && rsc2.TS != nil { rsc2.MergeFunc(accumulator, rsc2.TS, i) } // Update status code and headers (take best status code) From 3557160a8f08dd5618ec166237c48146de23b3ad Mon Sep 17 00:00:00 2001 From: James Ranson Date: Tue, 16 Dec 2025 16:45:36 -0700 Subject: [PATCH 18/28] wording Signed-off-by: James Ranson --- pkg/observability/logging/logging.go | 30 ++++++++++------------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/pkg/observability/logging/logging.go b/pkg/observability/logging/logging.go index 13ce51c23..7a315aa09 100644 --- a/pkg/observability/logging/logging.go +++ b/pkg/observability/logging/logging.go @@ -320,7 +320,7 @@ func (l *logger) HasLoggedOnce(logLevel level.Level, key string) bool { } func (l *logger) logAsyncronous(logLevel level.Level, event string, detail Pairs) { - go l.logWithStack(logLevel, event, detail, getCallerStack(1)) + go l.logWithCaller(logLevel, event, detail, getCaller(1)) } type item struct { @@ -338,8 +338,8 @@ const ( newline = "\n" ) -// getCallerStack returns the first path in the call stack from /pkg not in -func getCallerStack(skip int) string { +// getCaller returns the first path in the call stack from /pkg not in +func getCaller(skip int) string { for s := skip; s < skip+20; s++ { pc, file, line, ok := runtime.Caller(s) if !ok { @@ -359,19 +359,11 @@ func getCallerStack(skip int) string { } func (l *logger) log(logLevel level.Level, event string, detail Pairs) { - // For synchronous logging, capture caller here - // Call stack from getCallerStack's perspective: - // runtime.Caller(1) = log - // runtime.Caller(2) = logConditionally or logFuncConditionally - // runtime.Caller(3) = Warn/Info/etc in logging package - // runtime.Caller(4) = Warn/Info/etc in logger package - // runtime.Caller(5) = actual caller - // Start from skip 1 to check log itself, then walk up - stack := getCallerStack(1) - l.logWithStack(logLevel, event, detail, stack) -} - -func (l *logger) logWithStack(logLevel level.Level, event string, detail Pairs, stack string) { + caller := getCaller(1) + l.logWithCaller(logLevel, event, detail, caller) +} + +func (l *logger) logWithCaller(logLevel level.Level, event string, detail Pairs, caller string) { if l.writer == nil { return } @@ -388,9 +380,9 @@ func (l *logger) logWithStack(logLevel level.Level, event string, detail Pairs, "event=" + quoteAsNeeded(event), ) - // Add stack field if available - if stack != "" { - logLine = append(logLine, []byte(space+"stack="+stack)...) + // Add caller field if available + if caller != "" { + logLine = append(logLine, []byte(space+"caller="+caller)...) } if ld > 0 { From 88c3bdb07925ef9a1b1ca91c21aa21816dab3326 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 13:05:51 -0700 Subject: [PATCH 19/28] cleanup comments Signed-off-by: James Ranson --- .../alb/mech/tsm/time_series_merge.go | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/backends/alb/mech/tsm/time_series_merge.go b/pkg/backends/alb/mech/tsm/time_series_merge.go index 1999029e4..e024dff81 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge.go @@ -127,8 +127,8 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { defaultHandler.ServeHTTP(w, r) return } - // just proxy 1:1 if no folds in the fan - // there are now merge functions attached to the request + // just proxy 1:1 if no folds in the fan or if there + // are no merge functions attached to the request rsc := request.GetResources(r) if rsc == nil || l == 1 { defaultHandler.ServeHTTP(w, r) @@ -139,7 +139,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Scatter/Gather section - // Perform streaming merge: each goroutine merges as soon as response arrives accumulator := merge.NewAccumulator() var wg sync.WaitGroup var statusCode int @@ -152,7 +151,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } wg.Go(func() { r2, _ := request.Clone(r) - rsc2 := &request.Resources{IsMergeMember: true} + rsc2 := &request.Resources{IsMergeMember: true, TSReqestOptions: rsc.TSReqestOptions} r2 = request.SetResources(r2, rsc2) crw := capture.NewCaptureResponseWriter() hl[i].Handler().ServeHTTP(crw, r2) @@ -162,16 +161,16 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // Ensure merge functions are set on cloned request + // ensure merge functions are set on cloned request if rsc2.MergeFunc == nil || rsc2.MergeRespondFunc == nil { logger.Warn("tsm gather failed due to nil func", nil) } - // As soon as response is complete, unmarshal and merge - // This happens in parallel for each response as it arrives + // as soon as response is complete, unmarshal and merge + // this happens in parallel for each response as it arrives if rsc2.MergeFunc != nil && rsc2.TS != nil { rsc2.MergeFunc(accumulator, rsc2.TS, i) } - // Update status code and headers (take best status code) + // update status code and headers (take best status code) mu.Lock() if mrf == nil { mrf = rsc2.MergeRespondFunc @@ -190,15 +189,15 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { }) } - // Wait for all fanout requests to complete + // wait for all fanout requests to complete wg.Wait() - // Set aggregated status header + // set aggregated status header if statusHeader != "" { w.Header().Set(headers.NameTricksterResult, statusHeader) } - // Marshal and write the merged series to the client + // marshal and write the merged series to the client if statusCode == 0 { statusCode = http.StatusOK } From c50c26b7a675d1f96ca1e5331bcaa50d46372a30 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 13:06:05 -0700 Subject: [PATCH 20/28] respect timeout configs Signed-off-by: James Ranson --- pkg/proxy/proxy.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 3010d17b3..3fa074055 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -24,11 +24,14 @@ import ( "net" "net/http" "os" + "time" "github.com/prometheus/common/sigv4" bo "github.com/trickstercache/trickster/v2/pkg/backends/options" ) +const connectTimeout = time.Second * 10 + // NewHTTPClient returns an HTTP client configured to the specifications of the // running Trickster config. func NewHTTPClient(o *bo.Options) (*http.Client, error) { @@ -81,10 +84,17 @@ func NewHTTPClient(o *bo.Options) (*http.Client, error) { return http.ErrUseLastResponse }, Transport: &http.Transport{ - Dial: (&net.Dialer{KeepAlive: o.KeepAliveTimeout}).Dial, - MaxIdleConns: o.MaxIdleConns, - MaxIdleConnsPerHost: o.MaxIdleConns, - TLSClientConfig: TLSConfig, + Dial: (&net.Dialer{ + KeepAlive: o.KeepAliveTimeout, + Timeout: connectTimeout, + }).Dial, + MaxIdleConns: o.MaxIdleConns, + MaxIdleConnsPerHost: o.MaxIdleConns, + IdleConnTimeout: o.KeepAliveTimeout, + TLSHandshakeTimeout: connectTimeout, + ExpectContinueTimeout: o.Timeout, + ResponseHeaderTimeout: o.Timeout, + TLSClientConfig: TLSConfig, }, } From a8f1c75426a1a7b90a63563db098a77ca84941b5 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 13:06:19 -0700 Subject: [PATCH 21/28] update timeout defaults Signed-off-by: James Ranson --- examples/conf/example.full.yaml | 8 +++--- pkg/backends/options/defaults.go | 4 +-- pkg/config/testdata/example.alb.yaml | 28 +++++++++---------- pkg/config/testdata/example.auth.yaml | 16 +++++------ pkg/config/testdata/example.full.yaml | 4 +-- pkg/config/testdata/exmple.sharding.yaml | 4 +-- pkg/config/testdata/simple.prometheus.yaml | 4 +-- .../testdata/simple.reverseproxycache.yaml | 4 +-- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/examples/conf/example.full.yaml b/examples/conf/example.full.yaml index 2b9564188..6c1647098 100644 --- a/examples/conf/example.full.yaml +++ b/examples/conf/example.full.yaml @@ -300,12 +300,12 @@ backends: # compressable_types: # - text/javascript, text/css, text/plain, text/xml, text/json, application/json, application/javascript, application/xml ] -# # timeout defines how long Trickster will wait before aborting and upstream http request. Default: 180s -# timeout: 180s +# # timeout defines how long Trickster will wait before aborting and upstream http request. Default: 60s +# timeout: 60s # # keep_alive_timeout defines how long Trickster will wait before closing a keep-alive connection due to inactivity -# # if the origins keep-alive timeout is shorter than Tricksters, the connect will be closed sooner. Default: 300 -# keep_alive_timeout: 5m +# # if the origins keep-alive timeout is shorter than Tricksters, the connect will be closed sooner. Default: 120s +# keep_alive_timeout: 120s # # max_idle_conns set the maximum concurrent keep-alive connections Trickster may have opened to this backend # # additional requests will be queued. Default: 20 diff --git a/pkg/backends/options/defaults.go b/pkg/backends/options/defaults.go index 5980a8524..1e73e3000 100644 --- a/pkg/backends/options/defaults.go +++ b/pkg/backends/options/defaults.go @@ -42,7 +42,7 @@ const ( // DefaultBackendTEMName is the default Timeseries Eviction Method name for Time Series-based Backends DefaultBackendTEMName = "oldest" // DefaultBackendTimeout is the default Upstream Request Timeout for Backends - DefaultBackendTimeout = 3 * time.Minute + DefaultBackendTimeout = 1 * time.Minute // DefaultBackendCacheName is the default Cache Name for Backends DefaultBackendCacheName = "default" // DefaultBackendNegativeCacheName is the default Negative Cache Name for Backends @@ -54,7 +54,7 @@ const ( // DefaultBackfillTolerancePoints is the default Backfill Tolerance setting for Backends DefaultBackfillTolerancePoints = 0 // DefaultKeepAliveTimeout is the default Keep Alive Timeout for Backends' upstream client pools - DefaultKeepAliveTimeout = 5 * time.Minute + DefaultKeepAliveTimeout = 2 * time.Minute // DefaultMaxIdleConns is the default number of Idle Connections in Backends' upstream client pools DefaultMaxIdleConns = 20 // DefaultForwardedHeaders defines which class of 'Forwarded' headers are attached to upstream requests diff --git a/pkg/config/testdata/example.alb.yaml b/pkg/config/testdata/example.alb.yaml index aef946828..324d4a6af 100644 --- a/pkg/config/testdata/example.alb.yaml +++ b/pkg/config/testdata/example.alb.yaml @@ -2,8 +2,8 @@ main: server_name: trickster-test backends: default: - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default chunk_read_concurrency_limit: 16 @@ -35,8 +35,8 @@ backends: origin1: provider: rpc origin_url: https://origin1/ - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: origin1 @@ -71,8 +71,8 @@ backends: origin2: provider: rp origin_url: http://origin2 - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: origin2 @@ -107,8 +107,8 @@ backends: prom1: provider: prometheus origin_url: http://prom1:9090 - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: prom1:9090 @@ -142,8 +142,8 @@ backends: prom2: provider: prometheus origin_url: http://prom2:9090 - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: prom2:9090 @@ -176,8 +176,8 @@ backends: latency_max: 0s promalb: provider: alb - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default chunk_read_concurrency_limit: 16 @@ -214,8 +214,8 @@ backends: latency_max: 0s proxycache: provider: alb - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default chunk_read_concurrency_limit: 16 diff --git a/pkg/config/testdata/example.auth.yaml b/pkg/config/testdata/example.auth.yaml index 35556fe60..2b6a792dc 100644 --- a/pkg/config/testdata/example.auth.yaml +++ b/pkg/config/testdata/example.auth.yaml @@ -4,8 +4,8 @@ backends: backend01: provider: reverseproxy origin_url: https://example.com/ - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: example.com @@ -39,8 +39,8 @@ backends: backend02: provider: reverseproxy origin_url: https://example.com/ - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: example.com @@ -74,8 +74,8 @@ backends: backend03: provider: reverseproxy origin_url: https://example.com/ - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: example.com @@ -107,8 +107,8 @@ backends: latency_min: 0s latency_max: 0s default: - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default chunk_read_concurrency_limit: 16 diff --git a/pkg/config/testdata/example.full.yaml b/pkg/config/testdata/example.full.yaml index 9d5da20c0..81720be65 100644 --- a/pkg/config/testdata/example.full.yaml +++ b/pkg/config/testdata/example.full.yaml @@ -4,8 +4,8 @@ backends: default: provider: prometheus origin_url: http://prometheus:9090 - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: prometheus:9090 diff --git a/pkg/config/testdata/exmple.sharding.yaml b/pkg/config/testdata/exmple.sharding.yaml index db2178441..936b3334d 100644 --- a/pkg/config/testdata/exmple.sharding.yaml +++ b/pkg/config/testdata/exmple.sharding.yaml @@ -4,8 +4,8 @@ backends: default: provider: prometheus origin_url: http://prometheus:9090 - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: prometheus:9090 diff --git a/pkg/config/testdata/simple.prometheus.yaml b/pkg/config/testdata/simple.prometheus.yaml index 1ebe77a27..7db4901b8 100644 --- a/pkg/config/testdata/simple.prometheus.yaml +++ b/pkg/config/testdata/simple.prometheus.yaml @@ -4,8 +4,8 @@ backends: default: provider: prometheus origin_url: http://prometheus:9090 - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: prometheus:9090 diff --git a/pkg/config/testdata/simple.reverseproxycache.yaml b/pkg/config/testdata/simple.reverseproxycache.yaml index 3499ce29f..3887c226b 100644 --- a/pkg/config/testdata/simple.reverseproxycache.yaml +++ b/pkg/config/testdata/simple.reverseproxycache.yaml @@ -4,8 +4,8 @@ backends: default: provider: reverseproxycache origin_url: https://www.example.com - timeout: 3m0s - keep_alive_timeout: 5m0s + timeout: 1m0s + keep_alive_timeout: 2m0s max_idle_conns: 20 cache_name: default cache_key_prefix: www.example.com From 303867b7bd0e95dfbf8bba9fbeef7bf5c8ee3a20 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 13:14:32 -0700 Subject: [PATCH 22/28] fix typo Signed-off-by: James Ranson --- examples/conf/example.full.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/conf/example.full.yaml b/examples/conf/example.full.yaml index 6c1647098..d110c411d 100644 --- a/examples/conf/example.full.yaml +++ b/examples/conf/example.full.yaml @@ -300,7 +300,7 @@ backends: # compressable_types: # - text/javascript, text/css, text/plain, text/xml, text/json, application/json, application/javascript, application/xml ] -# # timeout defines how long Trickster will wait before aborting and upstream http request. Default: 60s +# # timeout defines how long Trickster will wait before aborting an upstream http request. Default: 60s # timeout: 60s # # keep_alive_timeout defines how long Trickster will wait before closing a keep-alive connection due to inactivity From bcd26aff6a0cf18929b0583845086d629d946daf Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 13:24:53 -0700 Subject: [PATCH 23/28] remove redundant responseWriter Signed-off-by: James Ranson --- pkg/backends/prometheus/handler_query.go | 35 +++--------------------- pkg/proxy/response/capture/capture.go | 8 ++++-- pkg/util/middleware/metrics.go | 10 +++---- 3 files changed, 15 insertions(+), 38 deletions(-) diff --git a/pkg/backends/prometheus/handler_query.go b/pkg/backends/prometheus/handler_query.go index 085594cc6..a4dd9b532 100644 --- a/pkg/backends/prometheus/handler_query.go +++ b/pkg/backends/prometheus/handler_query.go @@ -25,6 +25,7 @@ import ( "github.com/trickstercache/trickster/v2/pkg/proxy/engines" "github.com/trickstercache/trickster/v2/pkg/proxy/params" "github.com/trickstercache/trickster/v2/pkg/proxy/request" + "github.com/trickstercache/trickster/v2/pkg/proxy/response/capture" "github.com/trickstercache/trickster/v2/pkg/proxy/urls" "github.com/trickstercache/trickster/v2/pkg/timeseries" ) @@ -73,43 +74,15 @@ func (c *Client) QueryHandler(w http.ResponseWriter, r *http.Request) { // if there are labels to append to the dataset, if c.hasTransformations { // use a streaming response writer to capture the response body for transformation - sw := &transformationResponseWriter{ - ResponseWriter: w, - header: make(http.Header), - body: make([]byte, 0), - } + sw := capture.NewCaptureResponseWriter() engines.ObjectProxyCacheRequest(sw, r) - statusCode := sw.statusCode - if statusCode == 0 { - statusCode = http.StatusOK - } + statusCode := sw.StatusCode() if rsc != nil && rsc.Response != nil { statusCode = rsc.Response.StatusCode } - c.processVectorTransformations(w, sw.body, statusCode, rsc) + c.processVectorTransformations(w, sw.Body(), statusCode, rsc) return } engines.ObjectProxyCacheRequest(w, r) } - -// transformationResponseWriter captures the response for transformations -type transformationResponseWriter struct { - http.ResponseWriter - header http.Header - statusCode int - body []byte -} - -func (tw *transformationResponseWriter) Header() http.Header { - return tw.header -} - -func (tw *transformationResponseWriter) WriteHeader(code int) { - tw.statusCode = code -} - -func (tw *transformationResponseWriter) Write(b []byte) (int, error) { - tw.body = append(tw.body, b...) - return len(b), nil -} diff --git a/pkg/proxy/response/capture/capture.go b/pkg/proxy/response/capture/capture.go index b5ff0cb05..f9c95820f 100644 --- a/pkg/proxy/response/capture/capture.go +++ b/pkg/proxy/response/capture/capture.go @@ -33,7 +33,8 @@ type CaptureResponseWriter struct { // NewCaptureResponseWriter returns a new CaptureResponseWriter func NewCaptureResponseWriter() *CaptureResponseWriter { return &CaptureResponseWriter{ - header: make(http.Header), + header: make(http.Header), + statusCode: http.StatusOK, } } @@ -45,7 +46,7 @@ func (sw *CaptureResponseWriter) Header() http.Header { // WriteHeader sets the status code func (sw *CaptureResponseWriter) WriteHeader(code int) { if code == 0 { - code = 200 + code = http.StatusOK } sw.statusCode = code } @@ -64,5 +65,8 @@ func (sw *CaptureResponseWriter) Body() []byte { // StatusCode returns the captured status code func (sw *CaptureResponseWriter) StatusCode() int { + if sw.statusCode == 0 { + sw.statusCode = http.StatusOK + } return sw.statusCode } diff --git a/pkg/util/middleware/metrics.go b/pkg/util/middleware/metrics.go index 7e32c0d7d..7f8e947da 100644 --- a/pkg/util/middleware/metrics.go +++ b/pkg/util/middleware/metrics.go @@ -56,15 +56,15 @@ type responseObserver struct { func (w *responseObserver) WriteHeader(statusCode int) { w.ResponseWriter.WriteHeader(statusCode) switch { - case statusCode >= 100 && statusCode < 199: + case statusCode >= 100 && statusCode < 200: w.status = "1xx" - case statusCode >= 200 && statusCode < 299: + case statusCode >= 200 && statusCode < 300: w.status = "2xx" - case statusCode >= 300 && statusCode < 399: + case statusCode >= 300 && statusCode < 400: w.status = "3xx" - case statusCode >= 400 && statusCode < 499: + case statusCode >= 400 && statusCode < 500: w.status = "4xx" - case statusCode >= 500 && statusCode < 599: + case statusCode >= 500 && statusCode < 600: w.status = "5xx" } } From aa90f1c632f19baa5cf5d733e5864a6c8d63d767 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 14:18:42 -0700 Subject: [PATCH 24/28] simplify status aggregation Signed-off-by: James Ranson --- pkg/util/middleware/metrics.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/pkg/util/middleware/metrics.go b/pkg/util/middleware/metrics.go index 7f8e947da..31e3bc525 100644 --- a/pkg/util/middleware/metrics.go +++ b/pkg/util/middleware/metrics.go @@ -17,6 +17,7 @@ package middleware import ( + "fmt" "net/http" "time" @@ -55,18 +56,7 @@ type responseObserver struct { func (w *responseObserver) WriteHeader(statusCode int) { w.ResponseWriter.WriteHeader(statusCode) - switch { - case statusCode >= 100 && statusCode < 200: - w.status = "1xx" - case statusCode >= 200 && statusCode < 300: - w.status = "2xx" - case statusCode >= 300 && statusCode < 400: - w.status = "3xx" - case statusCode >= 400 && statusCode < 500: - w.status = "4xx" - case statusCode >= 500 && statusCode < 600: - w.status = "5xx" - } + w.status = fmt.Sprintf("%dxx", statusCode/100) } func (w *responseObserver) Write(b []byte) (int, error) { From c959edd3cd35b4ae7fa14f75a5f1b11576275110 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 14:30:21 -0700 Subject: [PATCH 25/28] avoid data race Signed-off-by: James Ranson --- pkg/backends/alb/mech/fr/first_response.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/backends/alb/mech/fr/first_response.go b/pkg/backends/alb/mech/fr/first_response.go index c860c1e5e..ab5e84ca8 100644 --- a/pkg/backends/alb/mech/fr/first_response.go +++ b/pkg/backends/alb/mech/fr/first_response.go @@ -164,7 +164,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { go func() { wg.Wait() // if claimed is still -1, the fallback case must be used - if atomic.LoadInt64(&claimed) == -1 && r.Context().Err() == nil { + if atomic.CompareAndSwapInt64(&claimed, -1, -2) && r.Context().Err() == nil { // this iterates the captures and serves the first non-nil response for i, crw := range captures { if crw != nil { From 14994d7fed886c7089dd901f7015f7c9c642e9fc Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 14:39:00 -0700 Subject: [PATCH 26/28] remove unnecessary cancels Signed-off-by: James Ranson --- .../alb/mech/nlm/newest_last_modified.go | 43 ++++++------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/pkg/backends/alb/mech/nlm/newest_last_modified.go b/pkg/backends/alb/mech/nlm/newest_last_modified.go index f0443f280..466d3f5ac 100644 --- a/pkg/backends/alb/mech/nlm/newest_last_modified.go +++ b/pkg/backends/alb/mech/nlm/newest_last_modified.go @@ -17,7 +17,6 @@ package nlm import ( - "context" "net/http" "sync" "time" @@ -86,11 +85,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Create contexts for cancellation - contexts := make([]context.Context, l) - cancels := make([]context.CancelFunc, l) - for i := range l { - contexts[i], cancels[i] = context.WithCancel(r.Context()) - } + ctx := r.Context() // Track the newest Last-Modified response newestIdx := -1 @@ -109,7 +104,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { idx := i wg.Go(func() { r2, _ := request.Clone(r) - r2 = request.ClearResources(r2.WithContext(contexts[idx])) + r2 = request.ClearResources(r2.WithContext(ctx)) crw := capture.NewCaptureResponseWriter() captures[idx] = crw hl[idx].ServeHTTP(crw, r2) @@ -138,30 +133,16 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { statusCode := crw.StatusCode() w.WriteHeader(statusCode) w.Write(crw.Body()) - - // Cancel all other contexts - for j, cancel := range cancels { - if j != newestIdx { - cancel() - } - } - } else { - // No valid response found, use the first available - for i, crw := range captures { - if crw != nil { - headers.Merge(w.Header(), crw.Header()) - statusCode := crw.StatusCode() - w.WriteHeader(statusCode) - w.Write(crw.Body()) - - // Cancel all other contexts - for j, cancel := range cancels { - if j != i { - cancel() - } - } - break - } + return + } + // No valid response found, use the first available + for _, crw := range captures { + if crw != nil { + headers.Merge(w.Header(), crw.Header()) + statusCode := crw.StatusCode() + w.WriteHeader(statusCode) + w.Write(crw.Body()) + break } } } From 5e8db0722efef1d0a0e9d9721c7039bb4fd3b2ee Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 14:43:56 -0700 Subject: [PATCH 27/28] remove unnecessary cancels Signed-off-by: James Ranson --- pkg/backends/alb/mech/fr/first_response.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/backends/alb/mech/fr/first_response.go b/pkg/backends/alb/mech/fr/first_response.go index ab5e84ca8..5314b8ed1 100644 --- a/pkg/backends/alb/mech/fr/first_response.go +++ b/pkg/backends/alb/mech/fr/first_response.go @@ -118,6 +118,13 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var wg sync.WaitGroup responseWritten := make(chan struct{}, 1) + serve := func(crw *capture.CaptureResponseWriter) { + headers.Merge(w.Header(), crw.Header()) + w.WriteHeader(crw.StatusCode()) + w.Write(crw.Body()) + responseWritten <- struct{}{} + } + serveAndCancelOthers := func(i int, crw *capture.CaptureResponseWriter) { go func() { // cancels all other contexts @@ -127,10 +134,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } }() - headers.Merge(w.Header(), crw.Header()) - w.WriteHeader(crw.StatusCode()) - w.Write(crw.Body()) - responseWritten <- struct{}{} + serve(crw) } // fanout to all healthy targets @@ -166,9 +170,9 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // if claimed is still -1, the fallback case must be used if atomic.CompareAndSwapInt64(&claimed, -1, -2) && r.Context().Err() == nil { // this iterates the captures and serves the first non-nil response - for i, crw := range captures { + for _, crw := range captures { if crw != nil { - serveAndCancelOthers(i, crw) + serve(crw) break } } From 1234ef375c7fa9e84c136159a280c38ef3e188f9 Mon Sep 17 00:00:00 2001 From: James Ranson Date: Wed, 17 Dec 2025 14:50:16 -0700 Subject: [PATCH 28/28] fix stray comment Signed-off-by: James Ranson --- pkg/backends/alb/mech/fr/first_response.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/backends/alb/mech/fr/first_response.go b/pkg/backends/alb/mech/fr/first_response.go index 5314b8ed1..60cded5c1 100644 --- a/pkg/backends/alb/mech/fr/first_response.go +++ b/pkg/backends/alb/mech/fr/first_response.go @@ -122,6 +122,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { headers.Merge(w.Header(), crw.Header()) w.WriteHeader(crw.StatusCode()) w.Write(crw.Body()) + // this signals the response is written responseWritten <- struct{}{} } @@ -158,7 +159,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { atomic.CompareAndSwapInt64(&claimed, -1, int64(i)) { // this serves only the first qualifying response serveAndCancelOthers(i, crw) - // this signals the response is written } }) }