Skip to content
This repository was 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

Add metrics for trade tracking. #574

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
51b70ac
Initial commit - create trade metrics handler plugin
debnil Oct 30, 2020
2d78a35
Add handler and tracker interfaces and impls
debnil Nov 3, 2020
92018eb
Create and inject metrics handler.
debnil Nov 3, 2020
7a113bb
Refine API, hook in read trades.
debnil Nov 3, 2020
69bc642
Add metrics to common props.
debnil Nov 3, 2020
40c5d20
Address review - nikhilsaraf
debnil Nov 6, 2020
79364e7
Fix build issue.
debnil Nov 6, 2020
6876075
Merge branch 'master' into num-trades-metrics
debnil Nov 20, 2020
ebdc97a
Address review - refactor handler and plugin relation.
debnil Nov 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/server_amd64.go
View file
Open in desktop
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
View file
Open in desktop
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

@nikhilsaraf nikhilsaraf Nov 24, 2020

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
56 changes: 7 additions & 49 deletions plugins/metricsTracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func MakeMetricsTrackerCli(
operationalBufferNonNativePct float64,
simMode bool,
fixedIterations uint64,
handler *plugins.TradeMetricsHandler,
) (*MetricsTracker, error) {
props := commonProps{
CliVersion: version,
Expand Down Expand Up @@ -215,7 +216,7 @@ func MakeMetricsTrackerCli(
botStartTime: botStartTime,
isDisabled: isDisabled,
updateEventSentTime: nil,
handler: plugins.MakeTradeMetricsHandler(),
handler: handler,
Copy link
Contributor

@nikhilsaraf nikhilsaraf Nov 24, 2020

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 @@ -236,6 +237,7 @@ func MakeMetricsTrackerGui(
goVersion string,
guiVersion string,
isDisabled bool,
handler *plugins.TradeMetricsHandler,
) (*MetricsTracker, error) {
props := commonProps{
CliVersion: version,
Expand All @@ -262,6 +264,7 @@ func MakeMetricsTrackerGui(
botStartTime: botStartTime,
isDisabled: isDisabled,
updateEventSentTime: nil,
handler: handler,
cliVersion: version,
}, nil
}
Expand Down Expand Up @@ -372,52 +375,7 @@ func (mt *MetricsTracker) SendEvent(eventType string, eventPropsInterface interf
return nil
}

// ComputeTradeMetrics computes various trade metrics and outputs a map that can be combined with other metrics.
func (mt *MetricsTracker) ComputeTradeMetrics() map[string]interface{} {
// TODO NS Ensure proper locking around trades on metrics handler
trades := mt.handler.GetTrades()
mt.handler.Reset()

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
totalQuoteVolume += quote

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

avgTradeSizeBase := totalBaseVolume / numTrades
avgTradeSizeQuote := totalQuoteVolume / numTrades
avgTradePrice := totalPrice / numTrades

tradeMetrics := map[string]interface{}{
"total_base_volume": totalBaseVolume,
"total_quote_volume": totalQuoteVolume,
"net_base_volume": netBaseVolume,
"net_quote_volume": netQuoteVolume,
"num_trades": numTrades,
"avg_trade_size_base": avgTradeSizeBase,
"avg_trade_size_quote": avgTradeSizeQuote,
"avg_trade_price": avgTradePrice,
"vwap": totalQuoteVolume / totalBaseVolume,
}

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

@nikhilsaraf nikhilsaraf Nov 24, 2020

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
}
66 changes: 66 additions & 0 deletions plugins/tradeMetricsHandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,20 @@ 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 {
return &TradeMetricsHandler{
Expand All @@ -32,3 +46,55 @@ func (h *TradeMetricsHandler) HandleFill(trade model.Trade) error {
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

@nikhilsaraf nikhilsaraf Nov 24, 2020

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

@nikhilsaraf nikhilsaraf Nov 24, 2020

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

@nikhilsaraf nikhilsaraf Nov 24, 2020

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

@nikhilsaraf nikhilsaraf Nov 24, 2020

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
}