Add Basic Metering Report to Workflow Engine #16398
Conversation
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
core/services/workflows/metering.go
Outdated
| return MeteringSpendValue(value.String()) | ||
| } | ||
|
|
||
| type MeteringSpendValue string |
There was a problem hiding this comment.
Why did you think to represent this as a string value?
This one is tricky to me - we can either represent the value in double (i.e. float64) to match the user facing credit values. Or we can do away with rounding error and establish a true minimum unit for each credit type and track everything in integer values. What do you think?
There was a problem hiding this comment.
I went with a string value here as a starting point because it is the least common denominator between all possible numeric value representations and is the standard for JSON encoding floats. However, if we intend to do division calculations, we probably should use a decimal.Decimal.
The intent of this code was to define a distinct type that represented the value such that we can change it to our needs as we learn more.
There was a problem hiding this comment.
Maybe we can use the type to contain our expected calculations:
type MeteringSpendValue struct {
value int64 // we can choose int64, decimal.Decimal, *big.Int, float64, etc.
decimal uint8 // examples: BTC: 8, ETH: 18, USDT: 0
}
func (v *MeteringSpendValue) Add(*MeteringSpendValue) *MeteringSpendValue {
// ... do addition
}
// ... other calculation functions
// String will likely be necessary for encoding in some transport layer either GRPC or something which could handle the rounding operation (since we likely should not do rounding after every calculation)
func (v MeteringSpendValue) String() string {
// ... do a string representation based on desired rounding method and decimal (standard, bank, floor, ceil)
}
Doing this, we can control the operations we need per value denomination (btc, eth, solana, usdt, etc.)
| "sort" | ||
| "sync" | ||
|
|
||
| "github.com/shopspring/decimal" |
There was a problem hiding this comment.
I don't recognize this import - do we use other decimal libraries in core?
There was a problem hiding this comment.
I'm quite certain this is the package we use in core. I have used it quite a bit before and it handles floating point errors quite well to my knowledge.
core/services/workflows/engine.go
Outdated
| @@ -616,6 +619,12 @@ func (e *Engine) handleStepUpdate(ctx context.Context, stepUpdate store.Workflow | |||
| return err | |||
| } | |||
|
|
|||
| e.meterReport.AddStep(MeteringReportStepRef(stepUpdate.ExecutionID), MeteringReportStep{ | |||
There was a problem hiding this comment.
we can keep this first PR clean of adding any mocked reports
This commit adds a metering report struct to the workflow engine such that each step can be individually added to the report. Metering units and values are generally treated as string values, but post values are verified to be valid numeric. The metering report can provide a calculated median value for all posted units.
EasterTheBunny
force-pushed
the
CRE-282/implement-aggregated-metering
branch
from
bfbfcf2 to
3133362
Compare
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌4 Results
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
|
This commit adds a metering report struct to the workflow engine such that each step can be individually added to the report. Metering units and values are generally treated as string values, but post values are verified to be valid numeric.
The metering report can provide a calculated median value for all posted units.
CRE-282