Skip to content

Commit ebe09e7

Browse files
SantiagoDePoloniaSantiagoDePolonia
committed
fix: fixed - to much requests logged
1 parent 2035067 commit ebe09e7

1 file changed

Lines changed: 78 additions & 82 deletions

File tree

‎internal/pkg/llmclient/client.go‎

Lines changed: 78 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -204,15 +204,73 @@ func (c *Client) Do(ctx context.Context, req Request, result interface{}) error
204204
return nil
205205
}
206206

207-
// DoRaw executes a request with retries and circuit breaking, returning the raw response
207+
// DoRaw executes a request with retries and circuit breaking, returning the raw response.
208+
//
209+
// # Metrics Behavior
210+
//
211+
// Metrics hooks (OnRequestStart/OnRequestEnd) are called at this level to track logical
212+
// requests from the caller's perspective, not individual retry attempts. This ensures:
213+
//
214+
// - Request counts reflect user-facing requests, not internal HTTP calls
215+
// - Duration metrics include total time across all retries (useful for SLOs)
216+
// - In-flight gauge accurately reflects concurrent logical requests
217+
//
218+
// Behavior comparison (hooks at DoRaw vs per-attempt):
219+
//
220+
// | Scenario | Per-attempt (old) | DoRaw level (current) |
221+
// |--------------------------------------|-----------------------------|----------------------------------|
222+
// | 1 request, succeeds first try | 1 observation | 1 observation |
223+
// | 1 request, fails twice then succeeds | 3 observations | 1 observation (success) |
224+
// | 1 request, fails all 3 retries | 3 observations | 1 observation (error) |
225+
// | Duration metric | Each attempt's duration | Total duration including retries |
226+
// | In-flight gauge | Bounces up/down per attempt | Accurate concurrent count |
227+
//
228+
// The final status code and error in metrics reflect the outcome after all retry attempts.
208229
func (c *Client) DoRaw(ctx context.Context, req Request) (*Response, error) {
230+
start := time.Now()
231+
232+
// Extract model for observability
233+
modelName := extractModel(req.Body)
234+
235+
// Build request info for hooks
236+
reqInfo := RequestInfo{
237+
Provider: c.config.ProviderName,
238+
Model: modelName,
239+
Endpoint: req.Endpoint,
240+
Method: req.Method,
241+
Stream: false,
242+
}
243+
244+
// Call OnRequestStart hook (once per logical request, not per retry)
245+
if c.config.Hooks.OnRequestStart != nil {
246+
ctx = c.config.Hooks.OnRequestStart(ctx, reqInfo)
247+
}
248+
249+
// Helper to call OnRequestEnd hook
250+
callEndHook := func(statusCode int, err error) {
251+
if c.config.Hooks.OnRequestEnd != nil {
252+
c.config.Hooks.OnRequestEnd(ctx, ResponseInfo{
253+
Provider: c.config.ProviderName,
254+
Model: modelName,
255+
Endpoint: req.Endpoint,
256+
StatusCode: statusCode,
257+
Duration: time.Since(start),
258+
Stream: false,
259+
Error: err,
260+
})
261+
}
262+
}
263+
209264
// Check circuit breaker
210265
if c.circuitBreaker != nil && !c.circuitBreaker.Allow() {
211-
return nil, core.NewProviderError(c.config.ProviderName, http.StatusServiceUnavailable,
266+
err := core.NewProviderError(c.config.ProviderName, http.StatusServiceUnavailable,
212267
"circuit breaker is open - provider temporarily unavailable", nil)
268+
callEndHook(http.StatusServiceUnavailable, err)
269+
return nil, err
213270
}
214271

215272
var lastErr error
273+
var lastStatusCode int
216274
maxAttempts := c.config.MaxRetries + 1
217275
if maxAttempts < 1 {
218276
maxAttempts = 1
@@ -224,6 +282,7 @@ func (c *Client) DoRaw(ctx context.Context, req Request) (*Response, error) {
224282
backoff := c.calculateBackoff(attempt)
225283
select {
226284
case <-ctx.Done():
285+
callEndHook(0, ctx.Err())
227286
return nil, ctx.Err()
228287
case <-time.After(backoff):
229288
}
@@ -232,6 +291,7 @@ func (c *Client) DoRaw(ctx context.Context, req Request) (*Response, error) {
232291
resp, err := c.doRequest(ctx, req)
233292
if err != nil {
234293
lastErr = err
294+
lastStatusCode = extractStatusCode(err)
235295
// Only retry on network errors
236296
if c.circuitBreaker != nil {
237297
c.circuitBreaker.RecordFailure()
@@ -245,6 +305,7 @@ func (c *Client) DoRaw(ctx context.Context, req Request) (*Response, error) {
245305
c.circuitBreaker.RecordFailure()
246306
}
247307
lastErr = core.ParseProviderError(c.config.ProviderName, resp.StatusCode, resp.Body, nil)
308+
lastStatusCode = resp.StatusCode
248309
continue
249310
}
250311

@@ -256,21 +317,27 @@ func (c *Client) DoRaw(ctx context.Context, req Request) (*Response, error) {
256317
c.circuitBreaker.RecordFailure()
257318
}
258319
}
259-
return nil, core.ParseProviderError(c.config.ProviderName, resp.StatusCode, resp.Body, nil)
320+
err := core.ParseProviderError(c.config.ProviderName, resp.StatusCode, resp.Body, nil)
321+
callEndHook(resp.StatusCode, err)
322+
return nil, err
260323
}
261324

262325
// Success
263326
if c.circuitBreaker != nil {
264327
c.circuitBreaker.RecordSuccess()
265328
}
329+
callEndHook(resp.StatusCode, nil)
266330
return resp, nil
267331
}
268332

269333
// All retries exhausted
270334
if lastErr != nil {
335+
callEndHook(lastStatusCode, lastErr)
271336
return nil, lastErr
272337
}
273-
return nil, core.NewProviderError(c.config.ProviderName, http.StatusBadGateway, "request failed after retries", nil)
338+
err := core.NewProviderError(c.config.ProviderName, http.StatusBadGateway, "request failed after retries", nil)
339+
callEndHook(http.StatusBadGateway, err)
340+
return nil, err
274341
}
275342

276343
// DoStream executes a streaming request, returning a ReadCloser
@@ -436,103 +503,32 @@ func extractStatusCode(err error) int {
436503
return 0
437504
}
438505

439-
// doRequest executes a single HTTP request without retries
506+
// doRequest executes a single HTTP request without retries.
507+
// Note: Metrics hooks are called at the DoRaw level, not here, to avoid
508+
// counting each retry attempt as a separate request.
440509
func (c *Client) doRequest(ctx context.Context, req Request) (*Response, error) {
441-
start := time.Now()
442-
443-
// Extract model for observability
444-
modelName := extractModel(req.Body)
445-
446-
// Build request info for hooks
447-
reqInfo := RequestInfo{
448-
Provider: c.config.ProviderName,
449-
Model: modelName,
450-
Endpoint: req.Endpoint,
451-
Method: req.Method,
452-
Stream: false,
453-
}
454-
455-
// Call OnRequestStart hook
456-
if c.config.Hooks.OnRequestStart != nil {
457-
ctx = c.config.Hooks.OnRequestStart(ctx, reqInfo)
458-
}
459-
460-
// Build and execute HTTP request
461510
httpReq, err := c.buildRequest(ctx, req)
462511
if err != nil {
463-
// Call OnRequestEnd hook on error
464-
if c.config.Hooks.OnRequestEnd != nil {
465-
c.config.Hooks.OnRequestEnd(ctx, ResponseInfo{
466-
Provider: c.config.ProviderName,
467-
Model: modelName,
468-
Endpoint: req.Endpoint,
469-
StatusCode: extractStatusCode(err),
470-
Duration: time.Since(start),
471-
Stream: false,
472-
Error: err,
473-
})
474-
}
475512
return nil, err
476513
}
477514

478515
resp, err := c.httpClient.Do(httpReq)
479516
if err != nil {
480-
providerErr := core.NewProviderError(c.config.ProviderName, http.StatusBadGateway, "failed to send request: "+err.Error(), err)
481-
// Call OnRequestEnd hook on error
482-
if c.config.Hooks.OnRequestEnd != nil {
483-
c.config.Hooks.OnRequestEnd(ctx, ResponseInfo{
484-
Provider: c.config.ProviderName,
485-
Model: modelName,
486-
Endpoint: req.Endpoint,
487-
StatusCode: extractStatusCode(providerErr),
488-
Duration: time.Since(start),
489-
Stream: false,
490-
Error: providerErr,
491-
})
492-
}
493-
return nil, providerErr
517+
return nil, core.NewProviderError(c.config.ProviderName, http.StatusBadGateway, "failed to send request: "+err.Error(), err)
494518
}
495519
defer func() {
496520
_ = resp.Body.Close()
497521
}()
498522

499523
body, err := io.ReadAll(resp.Body)
500524
if err != nil {
501-
readErr := core.NewProviderError(c.config.ProviderName, http.StatusBadGateway, "failed to read response: "+err.Error(), err)
502-
// Call OnRequestEnd hook on error
503-
if c.config.Hooks.OnRequestEnd != nil {
504-
c.config.Hooks.OnRequestEnd(ctx, ResponseInfo{
505-
Provider: c.config.ProviderName,
506-
Model: modelName,
507-
Endpoint: req.Endpoint,
508-
StatusCode: resp.StatusCode,
509-
Duration: time.Since(start),
510-
Stream: false,
511-
Error: readErr,
512-
})
513-
}
514-
return nil, readErr
525+
return nil, core.NewProviderError(c.config.ProviderName, http.StatusBadGateway, "failed to read response: "+err.Error(), err)
515526
}
516527

517-
response := &Response{
528+
return &Response{
518529
StatusCode: resp.StatusCode,
519530
Body: body,
520-
}
521-
522-
// Call OnRequestEnd hook on success
523-
if c.config.Hooks.OnRequestEnd != nil {
524-
c.config.Hooks.OnRequestEnd(ctx, ResponseInfo{
525-
Provider: c.config.ProviderName,
526-
Model: modelName,
527-
Endpoint: req.Endpoint,
528-
StatusCode: resp.StatusCode,
529-
Duration: time.Since(start),
530-
Stream: false,
531-
Error: nil,
532-
})
533-
}
534-
535-
return response, nil
531+
}, nil
536532
}
537533

538534
// buildRequest creates an HTTP request from a Request

0 commit comments

Comments
 (0)