Add GUI metrics to Button.js and refactor metrics tracker and frontend metric architecture (part of #508) (#642)

* 1 - changes from existing PR 582

* 2 - changes from my comments on PR

* 3 - pass in correct baseURL to sendMetricEvent from Button.js

* 4 - fix check for metrics when in local dev mode (no metrics tracker)

* 5 - metrics tracker should throw when amplitude returns invalid response

* 6 - fix API usage of /sendMetricEvent

* 7 - warn when no event name on button

* 8 - use correct event name type and data across the frontend, API, backend

* 9 - simplify metrics tracker event props naming

* 10 - refactor and simplify metricsTracker factory methods to use a single factory method

this ensures we do not send cli props for gui actions etc since they are separated

* 11 - prefix gui specific event properties as gui_

* 12 - isTestnet should be hardcoded to true by default

* 13 - change warning when no event name to an error

* 14 - set button names for welcome screen

* 15 - set button names for all remaining buttons by threading eventPrefix

* 16 - rename event_name to gui_event_name

* 17 - fix warnings in Button.js

Author: nikhilsaraf

cmd/server_amd64.go

@@ -298,24 +298,27 @@ func init() {
 			if e != nil {
 				panic(fmt.Errorf("could not generate machine id: %s", e))
 			}
-
-			httpClient := &http.Client{}
-			metricsTracker, e = plugins.MakeMetricsTrackerGui(
-				deviceID,
-				deviceID,
+			userID := deviceID // reuse for now
+			metricsTracker, e = plugins.MakeMetricsTracker(
+				http.DefaultClient,
 				amplitudeAPIKey,
-				httpClient,
-				time.Now(), // TODO: Find proper time.
-				version,
-				gitHash,
-				env,
-				runtime.GOOS,
-				runtime.GOARCH,
-				"unknown_todo", // TODO DS Determine how to get GOARM.
-				runtime.Version(),
-				guiVersion,
+				userID,
+				deviceID,
+				time.Now(),         // TODO: Find proper time.
 				*options.noHeaders, // disable metrics if the CLI specified no headers
-
+				plugins.MakeCommonProps(
+					version,
+					gitHash,
+					env,
+					runtime.GOOS,
+					runtime.GOARCH,
+					"unknown_todo", // TODO DS Determine how to get GOARM.
+					runtime.Version(),
+					0,
+					true, // isTestnet hardcoded to true for now, but once we allow it on the GUI via enablePubnetBots then this should be set accordingly
+					guiVersion,
+				),
+				nil,
 			)
 			if e != nil {
 				panic(e)

cmd/trade.go

@@ -243,7 +243,7 @@ func makeFeeFn(l logger.Logger, botConfig trader.BotConfig, newClient *horizoncl
 	return feeFn
 }
 
-func readBotConfig(l logger.Logger, options inputs, botStart time.Time) trader.BotConfig {
+func readBotConfig(l logger.Logger, options inputs, botStartTime time.Time) trader.BotConfig {
 	var botConfig trader.BotConfig
 	e := config.Read(*options.botConfigPath, &botConfig)
 	utils.CheckConfigError(botConfig, e, *options.botConfigPath)
@@ -253,7 +253,7 @@ func readBotConfig(l logger.Logger, options inputs, botStart time.Time) trader.B
 	}
 
 	if *options.logPrefix != "" {
-		logFilename := makeLogFilename(*options.logPrefix, botConfig, botStart)
+		logFilename := makeLogFilename(*options.logPrefix, botConfig, botStartTime)
 		setLogFile(l, logFilename)
 	}
 
@@ -427,7 +427,7 @@ func makeBot(
 	threadTracker *multithreading.ThreadTracker,
 	options inputs,
 	metricsTracker *plugins.MetricsTracker,
-	botStart time.Time,
+	botStartTime time.Time,
 ) *trader.Trader {
 	timeController := plugins.MakeIntervalTimeController(
 		time.Duration(botConfig.TickIntervalMillis)*time.Millisecond,
@@ -530,7 +530,7 @@ func makeBot(
 		dataKey,
 		alert,
 		metricsTracker,
-		botStart,
+		botStartTime,
 	)
 }
 
@@ -558,66 +558,67 @@ func convertDeprecatedBotConfigValues(l logger.Logger, botConfig trader.BotConfi
 
 func runTradeCmd(options inputs) {
 	l := logger.MakeBasicLogger()
-	botStart := time.Now()
-	botConfig := readBotConfig(l, options, botStart)
+	botStartTime := time.Now()
+	botConfig := readBotConfig(l, options, botStartTime)
 	botConfig = convertDeprecatedBotConfigValues(l, botConfig)
 	l.Infof("Trading %s:%s for %s:%s\n", botConfig.AssetCodeA, botConfig.IssuerA, botConfig.AssetCodeB, botConfig.IssuerB)
 
-	userID, e := getUserID(l, botConfig)
-	if e != nil {
-		logger.Fatal(l, fmt.Errorf("could not get user id: %s", e))
-	}
-
-	httpClient := &http.Client{}
 	var guiVersionFlag string
 	if *options.ui {
 		guiVersionFlag = guiVersion
 	}
 
+	userID, e := getUserID(l, botConfig)
+	if e != nil {
+		logger.Fatal(l, fmt.Errorf("could not get user id: %s", e))
+	}
 	deviceID, e := machineid.ID()
 	if e != nil {
 		logger.Fatal(l, fmt.Errorf("could not generate machine id: %s", e))
 	}
-
 	isTestnet := strings.Contains(botConfig.HorizonURL, "test") && botConfig.IsTradingSdex()
-
-	metricsTracker, e := plugins.MakeMetricsTrackerCli(
+	metricsTracker, e := plugins.MakeMetricsTracker(
+		http.DefaultClient,
+		amplitudeAPIKey,
 		userID,
 		deviceID,
-		amplitudeAPIKey,
-		httpClient,
-		botStart,
-		version,
-		gitHash,
-		env,
-		runtime.GOOS,
-		runtime.GOARCH,
-		goarm,
-		runtime.Version(),
-		guiVersionFlag,
-		*options.strategy,
-		float64(botConfig.TickIntervalMillis)/1000,
-		botConfig.TradingExchange,
-		botConfig.TradingPair(),
+		botStartTime,
 		*options.noHeaders, // disable metrics if the CLI specified no headers
-		isTestnet,
-		botConfig.MaxTickDelayMillis,
-		botConfig.SubmitMode,
-		botConfig.DeleteCyclesThreshold,
-		botConfig.FillTrackerSleepMillis,
-		botConfig.FillTrackerDeleteCyclesThreshold,
-		botConfig.SynchronizeStateLoadEnable,
-		botConfig.SynchronizeStateLoadMaxRetries,
-		botConfig.DollarValueFeedBaseAsset != "" && botConfig.DollarValueFeedQuoteAsset != "",
-		botConfig.AlertType,
-		int(botConfig.MonitoringPort) != 0,
-		len(botConfig.Filters) > 0,
-		botConfig.PostgresDbConfig != nil,
-		*options.logPrefix != "",
-		*options.operationalBuffer,
-		*options.operationalBufferNonNativePct,
-		*options.simMode,
-		*options.fixedIterations,
+		plugins.MakeCommonProps(
+			version,
+			gitHash,
+			env,
+			runtime.GOOS,
+			runtime.GOARCH,
+			goarm,
+			runtime.Version(),
+			0,
+			isTestnet,
+			guiVersionFlag,
+		),
+		plugins.MakeCliProps(
+			*options.strategy,
+			float64(botConfig.TickIntervalMillis)/1000,
+			botConfig.TradingExchange,
+			botConfig.TradingPair(),
+			botConfig.MaxTickDelayMillis,
+			botConfig.SubmitMode,
+			botConfig.DeleteCyclesThreshold,
+			botConfig.FillTrackerSleepMillis,
+			botConfig.FillTrackerDeleteCyclesThreshold,
+			botConfig.SynchronizeStateLoadEnable,
+			botConfig.SynchronizeStateLoadMaxRetries,
+			botConfig.DollarValueFeedBaseAsset != "" && botConfig.DollarValueFeedQuoteAsset != "",
+			botConfig.AlertType,
+			int(botConfig.MonitoringPort) != 0,
+			len(botConfig.Filters) > 0,
+			botConfig.PostgresDbConfig != nil,
+			*options.logPrefix != "",
+			*options.operationalBuffer,
+			*options.operationalBufferNonNativePct,
+			*options.simMode,
+			*options.fixedIterations,
+		),
 	)
 	if e != nil {
 		logger.Fatal(l, fmt.Errorf("could not generate metrics tracker: %s", e))
@@ -774,7 +775,7 @@ func runTradeCmd(options inputs) {
 		threadTracker,
 		options,
 		metricsTracker,
-		botStart,
+		botStartTime,
 	)
 	// --- end initialization of objects ---
 	// --- start initialization of services ---
@@ -1048,8 +1049,8 @@ func setLogFile(l logger.Logger, filename string) {
 	defer logPanic(l, false)
 }
 
-func makeLogFilename(logPrefix string, botConfig trader.BotConfig, botStart time.Time) string {
-	botStartStr := botStart.Format("20060102T150405MST")
+func makeLogFilename(logPrefix string, botConfig trader.BotConfig, botStartTime time.Time) string {
+	botStartStr := botStartTime.Format("20060102T150405MST")
 	if botConfig.IsTradingSdex() {
 		return fmt.Sprintf("%s_%s_%s_%s_%s_%s.log", logPrefix, botConfig.AssetCodeA, botConfig.IssuerA, botConfig.AssetCodeB, botConfig.IssuerB, botStartStr)
 	}

gui/backend/send_metric_event.go

@@ -10,7 +10,7 @@ import (
 )
 
 type sendMetricEventRequest struct {
-	EventName string                 `json:"event_name"`
+	EventType string                 `json:"event_type"`
 	EventData map[string]interface{} `json:"event_data"`
 }
 
@@ -34,9 +34,9 @@ func (s *APIServer) sendMetricEvent(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// TODO DS Properly extract and compute time for SendEvent
-	e = s.metricsTracker.SendEvent(req.EventName, req.EventData, time.Now())
+	e = s.metricsTracker.SendEvent(req.EventType, req.EventData, time.Now())
 	if e != nil {
-		s.writeErrorJson(w, fmt.Sprintf("error sending gui event %s: %s", req.EventName, e))
+		s.writeErrorJson(w, fmt.Sprintf("error sending gui event %s: %s", string(bodyBytes), e))
 		return
 	}
 

gui/web/src/App.js

@@ -13,13 +13,14 @@ import removeKelpErrors from './kelp-ops-api/removeKelpErrors';
 import Welcome from './components/molecules/Welcome/Welcome';
 
 let baseUrl = function () {
-  let origin = window.location.origin
+  let base_url = window.location.origin;
   if (process.env.REACT_APP_API_PORT) {
-    let parts = origin.split(":")
-    return parts[0] + ":" + parts[1] + ":" + process.env.REACT_APP_API_PORT;
+    let parts = origin.split(":");
+    base_url = parts[0] + ":" + parts[1] + ":" + process.env.REACT_APP_API_PORT;
   }
-  return origin;
-}()
+  Constants.setGlobalBaseURL(base_url);
+  return base_url;
+}();
 
 class App extends Component {
   constructor(props) {
@@ -296,6 +297,7 @@ class App extends Component {
 
     let banner = (<div className={styles.banner}>
       <Button
+        eventName="main-quit"
         className={styles.quit}
         size="small"
         onClick={this.quit}

gui/web/src/Constants.js

@@ -1,4 +1,4 @@
-export default {
+const constants = {
     BotState: {
         initializing: "initializing",
         stopped: "stopped",
@@ -15,4 +15,12 @@ export default {
         error: "error",
         warning: "warning",
     },
-}
+
+    BaseURL: "",
+
+    setGlobalBaseURL: (baseUrl) => {
+        constants.BaseURL = baseUrl;
+    },
+};
+
+export default constants;

gui/web/src/components/atoms/Button/Button.js

@@ -1,9 +1,11 @@
 import React, { Component } from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
+import Constants from '../../../Constants';
 import styles from './Button.module.scss';
 import Icon from '../Icon/Icon';
 import LoadingAnimation from '../LoadingAnimation/LoadingAnimation';
+import sendMetricEvent from '../../../kelp-ops-api/sendMetricEvent';
 
 const iconSizes = {
   small:'10px',
@@ -18,6 +20,14 @@ const iconSizesRound = {
 }
 
 class Button extends Component {
+  constructor(props) {
+    super(props);
+    this.trackOnClick = this.trackOnClick.bind(this);
+    this.sendMetricEvent = this.sendMetricEvent.bind(this);
+
+    this._asyncRequests = {};
+  }
+
   static defaultProps = {
     icon: null,
     size: 'medium',
@@ -34,9 +44,53 @@ class Button extends Component {
     variant: PropTypes.string,
     onClick: PropTypes.func,
     loading: PropTypes.bool,
-    disabled: PropTypes.bool
+    disabled: PropTypes.bool,
+    // we specify a custom validator. It should return an Error object if the validation fails
+    // don't `console.warn` or throw, as this won't work inside `oneOfType`.
+    eventName: function(props, propName, componentName) {
+      // "-" needs to be first or last character to be used literally
+      // source: https://stackoverflow.com/questions/8833963/allow-dash-in-regular-expression
+      if (!/^[-a-zA-Z0-9]+$/.test(props[propName])) {
+        return new Error('Invalid prop `' + propName + '` supplied to `' + componentName + '`. Validation failed.');
+      }
+    },
   };
 
+  sendMetricEvent() {
+    if (this._asyncRequests["sendMetricEvent"]) {
+      return
+    }
+
+    if (!this.props.eventName || this.props.eventName === "") {
+      console.error("programmer error: no event name provided for this Button, not sending button click event!");
+      return
+    }
+
+    const _this = this;
+    const eventData = {
+      gui_event_name: this.props.eventName,
+      gui_category: "generic",
+      gui_component: "button"
+    };
+    this._asyncRequests["sendMetricEvent"] = sendMetricEvent(Constants.BaseURL, "gui-button", eventData).then(resp => {
+      if (!_this._asyncRequests["sendMetricEvent"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+      delete _this._asyncRequests["sendMetricEvent"];
+
+      if (!resp.success) {
+        console.log(resp.error);
+      }
+      // else do nothing on success
+    });
+  }
+
+  trackOnClick() {
+    this.sendMetricEvent();
+    this.props.onClick();
+  }
+
   render() {
     const iconOnly = this.props.children ? null : styles.iconOnly;
     const isLoading = this.props.loading ? styles.isLoading : null;
@@ -57,7 +111,7 @@ class Button extends Component {
       <button 
         className={classNameList} 
         disabled={this.props.disabled || this.props.loading } 
-        onClick= {this.props.onClick}
+        onClick= {this.trackOnClick}
       >
         {this.props.loading &&
           <span className={styles.loader}>

gui/web/src/components/atoms/StartStop/StartStop.js

@@ -17,19 +17,22 @@ class StartStop extends Component {
   render() {
     let icon = "";
     let variant = "";
-    let text = ""
+    let text = "";
+    let eventName = "";
     if (this.props.state === Constants.BotState.running) {
       icon = "stop";
       variant = "stop";
       text = "Stop";
+      eventName = "bot-stop";
     } else {
       icon = "start";
       variant = "start";
       text = "Start";
+      eventName = "bot-start";
     }
     let disabled = this.props.state === Constants.BotState.initializing || this.props.state === Constants.BotState.stopping;
 
-    return (<Button icon={icon} size="small" variant={variant} onClick={this.props.onClick} disabled={disabled}>{text}</Button>);
+    return (<Button eventName={eventName} icon={icon} size="small" variant={variant} onClick={this.props.onClick} disabled={disabled}>{text}</Button>);
   }
 }