-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sets cost estimation and tracking options #850
Sets cost estimation and tracking options #850
Conversation
The cost might not set properly for other ext library as well. It might be out of scope for this PR but do we want to track it somewhere in an issue? (I am happy to create one if you are ok with it :) ) |
@cici37 Yes, please, if you wouldn't mind filing an issue to introduce costs for the other extension libraries that would be great. Most of these costs are currently tracked within the K8s cost estimation extensions and should be moved back into the CEL libraries. |
0176a41
to
67e5aeb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we merge this into 0.17 release and cut another 0.17.7 patch release for this fix? I would like to avoid bumping to 0.18 only because of the bug fix if possible. :)
@cici37 PTAL |
/lgtm |
/hold |
Introduce library-based options for cost estimation and cost tracking
Breaking change - This change introduced a minor shift to ensure that all instances
of the
interpreter.CostTracker
were instantiated usinginterpreter.NewCostTracker()
.This change revealed a bug where presence test,
has()
, costs were not being countedwhen calling
ContextEval()
orEval()
with state tracking enabled. This bug has beenfixed, but revealed an inconsistency in expectations between the two method calls.
Note, these options only match against type-checked expressions. This is the
same behavior as is used internally within estimators; however, it is probably
worth indicating the cost estimates are only accurate against type-checked
expressions at a future date.