Why: - The plugin host can exist without active request interceptors. - The before-auth path still cloned large request bodies four times in that case. What: - Reuse the existing request interceptor capability detector before dispatch. - Add pointer-reuse regression coverage and allocation benchmarks. Validation: - go test ./... -count=1 - go build -o /tmp/test-output ./cmd/server - focused race test passed; the full handler race run still hits an existing stream-header race reproduced on origin/dev
Unified · Split
| @@ -2064,7 +2064,7 @@ func interceptStreamChunk(ctx context.Context, host PluginInterceptorHost, req p · 2063 unmodified lines | |||
| 2064 | 2064 | ||
| 2065 | func (h *BaseAPIHandler) applyRequestInterceptorsBeforeAuth(ctx context.Context, handlerType, requestedModel string, req coreexecutor.Request, opts coreexecutor.Options, skipPluginID string) (coreexecutor.Request, coreexecutor.Options) { | 2065 | func (h *BaseAPIHandler) applyRequestInterceptorsBeforeAuth(ctx context.Context, handlerType, requestedModel string, req coreexecutor.Request, opts coreexecutor.Options, skipPluginID string) (coreexecutor.Request, coreexecutor.Options) { |
| 2066 | host := h.interceptorHost() | 2066 | host := h.interceptorHost() |
| 2067 | if host == nil { | 2067 | if !requestInterceptorsEnabled(host) { |
| 2068 | return req, opts | 2068 | return req, opts |
| 2069 | } | 2069 | } |
| 2070 | resp := interceptRequestBeforeAuth(ctx, host, pluginapi.RequestInterceptRequest{ | 2070 | resp := interceptRequestBeforeAuth(ctx, host, pluginapi.RequestInterceptRequest{ |
| @@ -29,10 +29,18 @@ type handlerInterceptorNoStreamTestHost struct { · 28 unmodified lines | |||
| 29 | *handlerInterceptorTestHost | 29 | *handlerInterceptorTestHost |
| 30 | } | 30 | } |
| 31 | 31 | ||
| 32 | type handlerInterceptorDisabledRequestTestHost struct { | ||
| 33 | *handlerInterceptorTestHost | ||
| 34 | } | ||
| 35 | |||
| 32 | func (h *handlerInterceptorNoStreamTestHost) HasStreamInterceptors() bool { | 36 | func (h *handlerInterceptorNoStreamTestHost) HasStreamInterceptors() bool { |
| 33 | return false | 37 | return false |
| 34 | } | 38 | } |
| 35 | 39 | ||
| 40 | func (h *handlerInterceptorDisabledRequestTestHost) HasRequestInterceptors() bool { | ||
| 41 | return false | ||
| 42 | } | ||
| 43 | |||
| 36 | func (h *handlerInterceptorTestHost) InterceptRequestBeforeAuth(ctx context.Context, req pluginapi.RequestInterceptRequest) pluginapi.RequestInterceptResponse { | 44 | func (h *handlerInterceptorTestHost) InterceptRequestBeforeAuth(ctx context.Context, req pluginapi.RequestInterceptRequest) pluginapi.RequestInterceptResponse { |
| 37 | if h != nil && h.interceptRequestBeforeAuth != nil { | 45 | if h != nil && h.interceptRequestBeforeAuth != nil { |
| 38 | return h.interceptRequestBeforeAuth(ctx, req) | 46 | return h.interceptRequestBeforeAuth(ctx, req) |
| @@ -255,6 +263,77 @@ func TestHandlerRequestInterceptorRewritesExecutorRequest(t *testing.T) { · 216 unmodified lines | |||
| 255 | } | 263 | } |
| 256 | } | 264 | } |
| 257 | 265 | ||
| 266 | func TestHandlerSkipsDisabledRequestInterceptorsWithoutCopyingPayload(t *testing.T) { | ||
| 267 | payload := []byte(`{"model":"disabled-interceptor-model"}`) | ||
| 268 | called := false | ||
| 269 | handler := NewBaseAPIHandlers(&sdkconfig.SDKConfig{}, nil) | ||
| 270 | handler.SetPluginHost(&handlerInterceptorDisabledRequestTestHost{ | ||
| 271 | handlerInterceptorTestHost: &handlerInterceptorTestHost{ | ||
| 272 | interceptRequestBeforeAuth: func(context.Context, pluginapi.RequestInterceptRequest) pluginapi.RequestInterceptResponse { | ||
| 273 | called = true | ||
| 274 | return pluginapi.RequestInterceptResponse{Body: []byte(`{"unexpected":true}`)} | ||
| 275 | }, | ||
| 276 | }, | ||
| 277 | }) | ||
| 278 | |||
| 279 | req := coreexecutor.Request{Model: "disabled-interceptor-model", Payload: payload} | ||
| 280 | opts := coreexecutor.Options{OriginalRequest: payload} | ||
| 281 | gotReq, gotOpts := handler.applyRequestInterceptorsBeforeAuth(context.Background(), "openai", req.Model, req, opts, "") | ||
| 282 | |||
| 283 | if called { | ||
| 284 | t.Fatal("disabled request interceptor was called") | ||
| 285 | } | ||
| 286 | if &gotReq.Payload[0] != &payload[0] { | ||
| 287 | t.Fatal("request payload was copied") | ||
| 288 | } | ||
| 289 | if &gotOpts.OriginalRequest[0] != &payload[0] { | ||
| 290 | t.Fatal("original request was copied") | ||
| 291 | } | ||
| 292 | } | ||
| 293 | |||
| 294 | func BenchmarkHandlerRequestInterceptors(b *testing.B) { | ||
| 295 | sizes := []struct { | ||
| 296 | name string | ||
| 297 | bytes int | ||
| 298 | }{ | ||
| 299 | {name: "1KiB", bytes: 1 << 10}, | ||
| 300 | {name: "1MiB", bytes: 1 << 20}, | ||
| 301 | {name: "8MiB", bytes: 8 << 20}, | ||
| 302 | } | ||
| 303 | hosts := []struct { | ||
| 304 | name string | ||
| 305 | host PluginInterceptorHost | ||
| 306 | }{ | ||
| 307 | { | ||
| 308 | name: "disabled", | ||
| 309 | host: &handlerInterceptorDisabledRequestTestHost{ | ||
| 310 | handlerInterceptorTestHost: &handlerInterceptorTestHost{}, | ||
| 311 | }, | ||
| 312 | }, | ||
| 313 | {name: "active", host: &handlerInterceptorTestHost{}}, | ||
| 314 | } | ||
| 315 | |||
| 316 | for _, size := range sizes { | ||
| 317 | payload := make([]byte, size.bytes) | ||
| 318 | req := coreexecutor.Request{Model: "benchmark-model", Payload: payload} | ||
| 319 | opts := coreexecutor.Options{OriginalRequest: payload} | ||
| 320 | for _, host := range hosts { | ||
| 321 | b.Run(host.name+"/"+size.name, func(b *testing.B) { | ||
| 322 | handler := NewBaseAPIHandlers(&sdkconfig.SDKConfig{}, nil) | ||
| 323 | handler.SetPluginHost(host.host) | ||
| 324 | b.ReportAllocs() | ||
| 325 | b.ResetTimer() | ||
| 326 | for range b.N { | ||
| 327 | gotReq, gotOpts := handler.applyRequestInterceptorsBeforeAuth(context.Background(), "openai", req.Model, req, opts, "") | ||
| 328 | if len(gotReq.Payload) != size.bytes || len(gotOpts.OriginalRequest) != size.bytes { | ||
| 329 | b.Fatal("request payload length changed") | ||
| 330 | } | ||
| 331 | } | ||
| 332 | }) | ||
| 333 | } | ||
| 334 | } | ||
| 335 | } | ||
| 336 | |||
| 258 | func TestHandlerRequestInterceptorEmptyBodyKeepsOriginalPayload(t *testing.T) { | 337 | func TestHandlerRequestInterceptorEmptyBodyKeepsOriginalPayload(t *testing.T) { |
| 259 | model := "handler-interceptor-empty-body-model" | 338 | model := "handler-interceptor-empty-body-model" |
| 260 | executor := &interceptorCaptureExecutor{} | 339 | executor := &interceptorCaptureExecutor{} |