Skip to content

Commit 36db767

Browse files
Addresses reviver's comments
Signed-off-by: David Breitgand <davidbreitgand@users.noreply.github.com>
1 parent fb86bad commit 36db767

1 file changed

Lines changed: 28 additions & 11 deletions

File tree

  • pkg/framework/plugins/datalayer/requestcostmetadata

pkg/framework/plugins/datalayer/requestcostmetadata/plugin.go

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import (
4040

4141
const (
4242
// PluginType is the identifier used when registering this extractor.
43-
PluginType = "request-cost-metadata-extractor"
43+
PluginType = "model-cost-extractor"
4444

4545
// defaultCompression matches the t-digest compression value used in the
4646
// CostGuard proposal (docs/proposals/050-costguard-scorer/README.md).
@@ -172,6 +172,8 @@ func (e *RequestCostMetadataExtractor) Extract(ctx context.Context, events []dls
172172

173173
now := time.Now()
174174
updated := map[string]bool{}
175+
// Cache token prices per-model to avoid repeated lookups within this batch
176+
tokenPricesCache := make(map[string]*pricing.TokenPrices)
175177

176178
for _, ev := range events {
177179
if ev.Type != dlsrc.ResponseEventType {
@@ -203,10 +205,16 @@ func (e *RequestCostMetadataExtractor) Extract(ctx context.Context, events []dls
203205
continue
204206
}
205207

206-
tp, ok := lookupTokenPrices(e.ds, model)
208+
// Check cache first; only lookup if not already cached
209+
tp, ok := tokenPricesCache[model]
207210
if !ok {
208-
debugLogger.Info("model has no TokenPrices attribute, skipping cost sample", "model", model)
209-
continue
211+
found := false
212+
tp, found = lookupTokenPrices(e.ds, model)
213+
if !found {
214+
debugLogger.Info("model has no TokenPrices attribute, skipping cost sample", "model", model)
215+
continue
216+
}
217+
tokenPricesCache[model] = tp
210218
}
211219

212220
cost := promptTokens*tp.InputTokenPrice + completionTokens*tp.OutputTokenPrice
@@ -223,23 +231,32 @@ func (e *RequestCostMetadataExtractor) Extract(ctx context.Context, events []dls
223231
updated[model] = true
224232
}
225233

234+
// After extracting all valid samples, pre-fetch accumulators for all models
235+
// to avoid repeated lookups during flush.
236+
modelToAcc := make(map[string]*modelCostAccumulator)
237+
for model := range updated {
238+
acc, err := e.getOrCreateAccumulator(model, now)
239+
if err != nil {
240+
debugLogger.Info("failed to create tdigest accumulator", "model", model, "err", err)
241+
delete(updated, model) // mark as failed
242+
continue
243+
}
244+
modelToAcc[model] = acc
245+
}
246+
226247
// updated contains exactly the models that received a fresh sample in
227248
// this batch, so the flushInterval gate below only consults
228249
// tdigest accumulators whose digest actually changed since the last publish.
229250
for model := range updated {
230-
acc := e.state[model]
251+
acc := modelToAcc[model]
231252
// flushInterval == 0 means publish on every event
232253
if e.flushInterval > 0 && now.Sub(acc.lastFlush) < e.flushInterval {
233254
continue
234255
}
235256
acc.lastFlush = now
236257
snapshot := acc.digest.Clone()
237-
// TODO: using GetOrCreateModel() is potentially a problem, because instead of
238-
// skipping the unconfigured models (in terms of pricing), we create
239-
// empty models. To fix: extend the datastore interface to have Get()
240-
// this is beyond the scope of this PR. Should handle in a separate PR
241-
// and remove this TODO afterwards.
242-
// Note that this is not a problem in requestmetadata. Only in this plugin.
258+
// assumes that all models are configured with the pricing attributes and validated
259+
// in the modelconfig collector
243260
e.ds.GetOrCreateModel(model).GetAttributes().Put(
244261
pricing.CostDigestAttributeKey,
245262
&pricing.CostDigest{Digest: snapshot},

0 commit comments

Comments
 (0)