Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Add metrics for trade tracking. #574

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
3 changes: 2 additions & 1 deletion cmd/server_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ func init() {
}

httpClient := &http.Client{}
metricsHandler := plugins.MakeTradeMetricsHandler()
metricsTracker, e = plugins.MakeMetricsTrackerGui(
deviceID,
deviceID,
Expand All @@ -315,7 +316,7 @@ func init() {
runtime.Version(),
guiVersion,
*options.noHeaders, // disable metrics if the CLI specified no headers

metricsHandler,
)
if e != nil {
panic(e)
Expand Down
6 changes: 6 additions & 0 deletions cmd/trade.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ func runTradeCmd(options inputs) {

isTestnet := strings.Contains(botConfig.HorizonURL, "test") && botConfig.IsTradingSdex()

metricsHandler := plugins.MakeTradeMetricsHandler()
metricsTracker, e := plugins.MakeMetricsTrackerCli(
userID,
deviceID,
Expand Down Expand Up @@ -603,6 +604,7 @@ func runTradeCmd(options inputs) {
*options.operationalBufferNonNativePct,
*options.simMode,
*options.fixedIterations,
metricsHandler,
)
if e != nil {
logger.Fatal(l, fmt.Errorf("could not generate metrics tracker: %s", e))
Expand Down Expand Up @@ -921,6 +923,10 @@ func makeFillTracker(
}
}

metricsHandler := metricsTracker.GetHandler()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. we should not pull the metricsHandler from within the metricsTracker but pass it directly to this function. i.e. we don't need the GetHandler() method on metricsTracker either.

  2. this registration of the handler should happen before the if strategyFillHandlers != nil { paragraph. we want the strategy handlers to always be last because we execute the handler in the order in which they are added as well. I should have added this as a comment, can you please add this as well (that "handlers are executed in the order in which they are added")?

if metricsHandler != nil {
fillTracker.RegisterHandler(metricsHander)
}
return fillTracker
}

Expand Down
12 changes: 12 additions & 0 deletions plugins/metricsTracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"runtime/debug"
"time"

"github.com/stellar/kelp/plugins"
"github.com/stellar/kelp/support/networking"
"github.com/stellar/kelp/support/utils"
)
Expand All @@ -34,6 +35,8 @@ type MetricsTracker struct {
isDisabled bool
updateEventSentTime *time.Time
cliVersion string

handler *plugins.TradeMetricsHandler
}

// TODO DS Investigate other fields to add to this top-level event.
Expand Down Expand Up @@ -163,6 +166,7 @@ func MakeMetricsTrackerCli(
operationalBufferNonNativePct float64,
simMode bool,
fixedIterations uint64,
handler *plugins.TradeMetricsHandler,
) (*MetricsTracker, error) {
props := commonProps{
CliVersion: version,
Expand Down Expand Up @@ -212,6 +216,7 @@ func MakeMetricsTrackerCli(
botStartTime: botStartTime,
isDisabled: isDisabled,
updateEventSentTime: nil,
handler: handler,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

cliVersion: version,
}, nil
}
Expand All @@ -232,6 +237,7 @@ func MakeMetricsTrackerGui(
goVersion string,
guiVersion string,
isDisabled bool,
handler *plugins.TradeMetricsHandler,
) (*MetricsTracker, error) {
props := commonProps{
CliVersion: version,
Expand All @@ -258,6 +264,7 @@ func MakeMetricsTrackerGui(
botStartTime: botStartTime,
isDisabled: isDisabled,
updateEventSentTime: nil,
handler: handler,
cliVersion: version,
}, nil
}
Expand Down Expand Up @@ -367,3 +374,8 @@ func (mt *MetricsTracker) SendEvent(eventType string, eventPropsInterface interf
}
return nil
}

// GetHandler returns the TradeMetricsHandler
func (mt *MetricsTracker) GetHandler() *plugins.TradeMetricsHandler {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this (as commented above)

return mt.handler
}
100 changes: 100 additions & 0 deletions plugins/tradeMetricsHandler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package plugins

import (
"github.com/stellar/kelp/model"
)

// TradeMetricsHandler tracks the number of trades
type TradeMetricsHandler struct {
trades []model.Trade
}

// TradeMetrics stores the computed trade-related metrics.
// TODO DS Pre-pend interval__ to all JSON fields.
type TradeMetrics struct {
totalBaseVolume float64 `json:"total_base_volume"`
totalQuoteVolume float64 `json:"total_quote_volume"`
netBaseVolume float64 `json:"net_base_volume"`
netQuoteVolume float64 `json:"net_quote_volume"`
numTrades float64 `json:"num_trades"`
avgTradeSizeBase float64 `json:"avg_trade_size_base"`
avgTradeSizeQuote float64 `json:"avg_trade_size_quote"`
avgTradePrice float64 `json:"avg_trade_price"`
vwap float64 `json:"vwap"`
}

// MakeTradeMetricsHandler is a factory method for the TradeMetricsHandler
func MakeTradeMetricsHandler() *TradeMetricsHandler {
debnil marked this conversation as resolved.
Show resolved Hide resolved
return &TradeMetricsHandler{
trades: []model.Trade{},
}
}

// Reset sets the handler's trades to empty.
func (h *TradeMetricsHandler) Reset() {
h.trades = []model.Trade{}
}

// GetTrades returns all stored trades.
func (h *TradeMetricsHandler) GetTrades() []model.Trade {
debnil marked this conversation as resolved.
Show resolved Hide resolved
return h.trades
}

// HandleFill handles a new trade
// Implements FillHandler interface
func (h *TradeMetricsHandler) HandleFill(trade model.Trade) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to test this tradeMetricsHandler. For now let's have one simple test case, where we add 3 trades:

  • buy XLM/USDT
  • buy XLM/USDT
  • sell XLM/USDT

then we assert that the computed values on the struct returned from ComputeTradeMetrics is correct.

this TestComputeTradeMetrics would be very simple and would ensure our calculations are correct.

h.trades = append(h.trades, trade)
return nil
}

// ComputeTradeMetrics computes trade-related metrics.
func (h *TradeMetricsHandler) ComputeTradeMetrics(secondsSinceLastUpdate float64) TradeMetrics {
trades := h.GetTrades()
h.Reset()

// Note that we don't consider fees right now, as we don't have the infrastructure to understand fee units when tracking trades.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

totalBaseVolume := 0.0
totalQuoteVolume := 0.0
totalPrice := 0.0
netBaseVolume := 0.0
netQuoteVolume := 0.0

numTrades := float64(len(trades))
for _, t := range trades {
base := t.Volume.AsFloat()
price := t.Price.AsFloat()
quote := base * price

totalBaseVolume += base
totalPrice += price
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think the math works out this way for price to get avgTradePrice accurately.

avgTradePrice should be totalQuoteVolume / totalBaseVolume (i.e. we don't need to track totalPrice) -- which we call vwap

not sure if I had suggested adding this, but if I had that is wrong.
we should split this up into avgBuyPrice and avgSellPrice
we already have vwap which is the price inclusive of both buys and sells so getting these separate buy/sell prices will be more informative

totalQuoteVolume += quote

if t.OrderAction.IsBuy() {
netBaseVolume += base
netQuoteVolume -= quote
} else {
netBaseVolume -= base
netQuoteVolume -= quote
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be a typo, should be netQuoteVolume += quote instead of netQuoteVolume -= quote

}
}

avgTradeSizeBase := totalBaseVolume / numTrades
avgTradeSizeQuote := totalQuoteVolume / numTrades
avgTradePrice := totalPrice / numTrades
avgTradeThroughputBase := totalBaseVolume / secondsSinceLastUpdate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename this to tradeThroughputPerSecondBase (and quote equivalent)?

avgTradeThroughputQuote := totalQuoteVolume / secondsSinceLastUpdate

tradeMetrics := TradeMetrics{
totalBaseVolume: totalBaseVolume,
totalQuoteVolume: totalQuoteVolume,
netBaseVolume: netBaseVolume,
netQuoteVolume: netQuoteVolume,
numTrades: numTrades,
avgTradeSizeBase: avgTradeSizeBase,
avgTradeSizeQuote: avgTradeSizeQuote,
avgTradePrice: avgTradePrice,
vwap: totalQuoteVolume / totalBaseVolume,
}

return tradeMetrics
}