From 9066dca75047ca47dac17121ad461b7e5f14aed1 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Tue, 5 Sep 2017 23:08:36 +0300 Subject: [PATCH 1/8] Use context-bound SQL methods Changes: Exec -> ExecContext Query -> QueryContext QueryRow -> QueryRowContext Rebind and store all possible queries before using them Closes: #276 --- api/datastore/sql/sql.go | 167 +++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 79 deletions(-) diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 2d8844f9e..70f88b392 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -72,9 +72,22 @@ const ( type sqlStore struct { db *sqlx.DB - - // TODO we should prepare all of the statements, rebind them - // and store them all here. + insertAppQuery string + updateAppQuery string + getAppQuery string + selectAppConfigQuery string + removeAppQuery string + insertRouteQuery string + updateRouteQuery string + checkRouteAppQuery string + checkRouteQuery string + getRouteQuery string + removeRouteQuery string + insertCallQuery string + getCallQuery string + insertLogQuery string + getLogQuery string + deleteLogQuery string } // New will open the db specified by url, create any tables necessary @@ -133,7 +146,56 @@ func New(url *url.URL) (models.Datastore, error) { } } - return &sqlStore{db: db}, nil + dstore := &sqlStore{ + db: db, + insertAppQuery: db.Rebind("INSERT INTO apps (name, config) VALUES (?, ?);"), + selectAppConfigQuery: db.Rebind(`SELECT config FROM apps WHERE name=?`), + updateAppQuery: db.Rebind(`UPDATE apps SET config=? WHERE name=?`), + removeAppQuery: db.Rebind(`DELETE FROM apps WHERE name = ?`), + getAppQuery: db.Rebind(`SELECT name, config FROM apps WHERE name=?`), + checkRouteAppQuery: db.Rebind(`SELECT 1 FROM apps WHERE name=?`), + checkRouteQuery: db.Rebind(`SELECT 1 FROM routes WHERE app_name=? AND path=?`), + insertRouteQuery: db.Rebind(`INSERT INTO routes ( + app_name, + path, + image, + format, + memory, + type, + timeout, + idle_timeout, + headers, + config + ) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?);`), + getRouteQuery: db.Rebind(fmt.Sprintf("%s WHERE app_name=? AND path=?", routeSelector)), + updateRouteQuery: db.Rebind(`UPDATE routes SET + image = ?, + format = ?, + memory = ?, + type = ?, + timeout = ?, + idle_timeout = ?, + headers = ?, + config = ? + WHERE app_name=? AND path=?;`), + removeRouteQuery: db.Rebind(`DELETE FROM routes WHERE path = ? AND app_name = ?`), + insertCallQuery: db.Rebind(`INSERT INTO calls ( + id, + created_at, + started_at, + completed_at, + status, + app_name, + path + ) + VALUES (?, ?, ?, ?, ?, ?, ?);`), + getCallQuery: db.Rebind(fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector)), + insertLogQuery: db.Rebind(`INSERT INTO logs (id, log) VALUES (?, ?);`), + getLogQuery: db.Rebind(`SELECT log FROM logs WHERE id=?`), + deleteLogQuery: db.Rebind(`DELETE FROM logs WHERE id=?`), + } + return dstore, nil } func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { @@ -146,8 +208,7 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App } } - query := ds.db.Rebind("INSERT INTO apps (name, config) VALUES (?, ?);") - _, err = ds.db.Exec(query, app.Name, string(cbyte)) + _, err = ds.db.ExecContext(ctx, ds.insertAppQuery, app.Name, string(cbyte)) if err != nil { switch err := err.(type) { case *mysql.MySQLError: @@ -172,8 +233,7 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { app := &models.App{Name: newapp.Name} err := ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(`SELECT config FROM apps WHERE name=?`) - row := tx.QueryRow(query, app.Name) + row := tx.QueryRowContext(ctx, ds.selectAppConfigQuery, app.Name) var config string if err := row.Scan(&config); err != nil { @@ -197,8 +257,7 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. return err } - query = tx.Rebind(`UPDATE apps SET config=? WHERE name=?`) - res, err := tx.Exec(query, string(cbyte), app.Name) + res, err := tx.ExecContext(ctx, ds.updateAppQuery, string(cbyte), app.Name) if err != nil { return err } @@ -220,14 +279,12 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. } func (ds *sqlStore) RemoveApp(ctx context.Context, appName string) error { - query := ds.db.Rebind(`DELETE FROM apps WHERE name = ?`) - _, err := ds.db.Exec(query, appName) + _, err := ds.db.ExecContext(ctx, ds.removeAppQuery, appName) return err } func (ds *sqlStore) GetApp(ctx context.Context, name string) (*models.App, error) { - query := ds.db.Rebind(`SELECT name, config FROM apps WHERE name=?`) - row := ds.db.QueryRow(query, name) + row := ds.db.QueryRowContext(ctx, ds.getAppQuery, name) var resName, config string err := row.Scan(&resName, &config) @@ -257,7 +314,7 @@ func (ds *sqlStore) GetApps(ctx context.Context, filter *models.AppFilter) ([]*m res := []*models.App{} query, args := buildFilterAppQuery(filter) query = ds.db.Rebind(fmt.Sprintf("SELECT DISTINCT name, config FROM apps %s", query)) - rows, err := ds.db.Query(query, args...) + rows, err := ds.db.QueryContext(ctx, query, args...) if err != nil { return nil, err } @@ -294,15 +351,13 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode } err = ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(`SELECT 1 FROM apps WHERE name=?`) - r := tx.QueryRow(query, route.AppName) + r := tx.QueryRowContext(ctx, ds.checkRouteAppQuery, route.AppName) if err := r.Scan(new(int)); err != nil { if err == sql.ErrNoRows { return models.ErrAppsNotFound } } - query = tx.Rebind(`SELECT 1 FROM routes WHERE app_name=? AND path=?`) - same, err := tx.Query(query, route.AppName, route.Path) + same, err := tx.QueryContext(ctx, ds.checkRouteQuery, route.AppName, route.Path) if err != nil { return err } @@ -311,21 +366,7 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode return models.ErrRoutesAlreadyExists } - query = tx.Rebind(`INSERT INTO routes ( - app_name, - path, - image, - format, - memory, - type, - timeout, - idle_timeout, - headers, - config - ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?);`) - - _, err = tx.Exec(query, + _, err = tx.ExecContext(ctx, ds.insertRouteQuery, route.AppName, route.Path, route.Image, @@ -347,8 +388,7 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { var route models.Route err := ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(fmt.Sprintf("%s WHERE app_name=? AND path=?", routeSelector)) - row := tx.QueryRow(query, newroute.AppName, newroute.Path) + row := tx.QueryRowContext(ctx, ds.getRouteQuery, newroute.AppName, newroute.Path) if err := scanRoute(row, &route); err == sql.ErrNoRows { return models.ErrRoutesNotFound } else if err != nil { @@ -367,18 +407,7 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m return err } - query = tx.Rebind(`UPDATE routes SET - image = ?, - format = ?, - memory = ?, - type = ?, - timeout = ?, - idle_timeout = ?, - headers = ?, - config = ? - WHERE app_name=? AND path=?;`) - - res, err := tx.Exec(query, + res, err := tx.ExecContext(ctx, ds.updateAppQuery, route.Image, route.Format, route.Memory, @@ -412,8 +441,7 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m } func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) error { - query := ds.db.Rebind(`DELETE FROM routes WHERE path = ? AND app_name = ?`) - res, err := ds.db.Exec(query, routePath, appName) + res, err := ds.db.ExecContext(ctx, ds.removeRouteQuery, routePath, appName) if err != nil { return err } @@ -431,9 +459,7 @@ func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) } func (ds *sqlStore) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - rSelectCondition := "%s WHERE app_name=? AND path=?" - query := ds.db.Rebind(fmt.Sprintf(rSelectCondition, routeSelector)) - row := ds.db.QueryRow(query, appName, routePath) + row := ds.db.QueryRowContext(ctx, ds.getRouteQuery, appName, routePath) var route models.Route err := scanRoute(row, &route) @@ -449,9 +475,8 @@ func (ds *sqlStore) GetRoute(ctx context.Context, appName, routePath string) (*m func (ds *sqlStore) GetRoutes(ctx context.Context, filter *models.RouteFilter) ([]*models.Route, error) { res := []*models.Route{} query, args := buildFilterRouteQuery(filter) - query = fmt.Sprintf("%s %s", routeSelector, query) - query = ds.db.Rebind(query) - rows, err := ds.db.Query(query, args...) + query = ds.db.Rebind(fmt.Sprintf("%s %s", routeSelector, query)) + rows, err := ds.db.QueryContext(ctx, query, args...) // todo: check for no rows so we don't respond with a sql 500 err if err != nil { return nil, err @@ -490,7 +515,7 @@ func (ds *sqlStore) GetRoutesByApp(ctx context.Context, appName string, filter * query := fmt.Sprintf("%s %s", routeSelector, filterQuery) query = ds.db.Rebind(query) - rows, err := ds.db.Query(query, args...) + rows, err := ds.db.QueryContext(ctx, query, args...) // todo: check for no rows so we don't respond with a sql 500 err if err != nil { return nil, err @@ -527,18 +552,7 @@ func (ds *sqlStore) Tx(f func(*sqlx.Tx) error) error { } func (ds *sqlStore) InsertCall(ctx context.Context, call *models.Call) error { - query := ds.db.Rebind(`INSERT INTO calls ( - id, - created_at, - started_at, - completed_at, - status, - app_name, - path - ) - VALUES (?, ?, ?, ?, ?, ?, ?);`) - - _, err := ds.db.Exec(query, call.ID, call.CreatedAt.String(), + _, err := ds.db.ExecContext(ctx, ds.insertCallQuery, call.ID, call.CreatedAt.String(), call.StartedAt.String(), call.CompletedAt.String(), call.Status, call.AppName, call.Path) if err != nil { @@ -552,9 +566,7 @@ func (ds *sqlStore) InsertCall(ctx context.Context, call *models.Call) error { // if we store the whole thing then it adds a lot of disk space and then we can // make async only queue hints instead of entire calls (mq a lot smaller space wise). pick. func (ds *sqlStore) GetCall(ctx context.Context, appName, callID string) (*models.Call, error) { - query := fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector) - query = ds.db.Rebind(query) - row := ds.db.QueryRow(query, callID, appName) + row := ds.db.QueryRowContext(ctx, ds.getCallQuery, callID, appName) var call models.Call err := scanCall(row, &call) @@ -569,7 +581,7 @@ func (ds *sqlStore) GetCalls(ctx context.Context, filter *models.CallFilter) ([] query, args := buildFilterCallQuery(filter) query = fmt.Sprintf("%s %s", callSelector, query) query = ds.db.Rebind(query) - rows, err := ds.db.Query(query, args...) + rows, err := ds.db.QueryContext(ctx, query, args...) if err != nil { return nil, err } @@ -590,14 +602,12 @@ func (ds *sqlStore) GetCalls(ctx context.Context, filter *models.CallFilter) ([] } func (ds *sqlStore) InsertLog(ctx context.Context, callID, callLog string) error { - query := ds.db.Rebind(`INSERT INTO logs (id, log) VALUES (?, ?);`) - _, err := ds.db.Exec(query, callID, callLog) + _, err := ds.db.ExecContext(ctx, ds.insertLogQuery, callID, callLog) return err } func (ds *sqlStore) GetLog(ctx context.Context, callID string) (*models.CallLog, error) { - query := ds.db.Rebind(`SELECT log FROM logs WHERE id=?`) - row := ds.db.QueryRow(query, callID) + row := ds.db.QueryRowContext(ctx, ds.getLogQuery, callID) var log string err := row.Scan(&log) @@ -615,8 +625,7 @@ func (ds *sqlStore) GetLog(ctx context.Context, callID string) (*models.CallLog, } func (ds *sqlStore) DeleteLog(ctx context.Context, callID string) error { - query := ds.db.Rebind(`DELETE FROM logs WHERE id=?`) - _, err := ds.db.Exec(query, callID) + _, err := ds.db.ExecContext(ctx, ds.deleteLogQuery, callID) return err } From 6ac579f2966a2d273f8382ca00c32945f3e3c36b Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Tue, 5 Sep 2017 23:11:08 +0300 Subject: [PATCH 2/8] Formatting issues Aren't we running go-fmt.sh in CI? --- api/agent/drivers/driver_test.go | 8 +-- api/datastore/internal/datastoretest/test.go | 2 +- api/datastore/sql/sql.go | 52 ++++++++++---------- api/logs/log.go | 2 +- api/mqs/bolt.go | 2 +- api/mqs/ironmq.go | 2 +- api/mqs/new.go | 2 +- api/server/apps_test.go | 2 +- api/server/init.go | 2 +- fnlb/lb/allgrouper.go | 2 +- fnlb/lb/proxy.go | 2 +- fnlb/main.go | 2 +- 12 files changed, 40 insertions(+), 40 deletions(-) diff --git a/api/agent/drivers/driver_test.go b/api/agent/drivers/driver_test.go index 7c5a84337..f309f8536 100644 --- a/api/agent/drivers/driver_test.go +++ b/api/agent/drivers/driver_test.go @@ -94,14 +94,14 @@ func TestDecimate(t *testing.T) { func TestParseImage(t *testing.T) { cases := map[string][]string{ - "fnproject/hello": {"", "fnproject/hello", "latest"}, - "fnproject/hello:v1": {"", "fnproject/hello", "v1"}, + "fnproject/hello": {"", "fnproject/hello", "latest"}, + "fnproject/hello:v1": {"", "fnproject/hello", "v1"}, "my.registry/hello": {"my.registry", "hello", "latest"}, "my.registry/hello:v1": {"my.registry", "hello", "v1"}, "mongo": {"", "library/mongo", "latest"}, "mongo:v1": {"", "library/mongo", "v1"}, - "quay.com/fnproject/hello": {"quay.com", "fnproject/hello", "latest"}, - "quay.com:8080/fnproject/hello:v2": {"quay.com:8080", "fnproject/hello", "v2"}, + "quay.com/fnproject/hello": {"quay.com", "fnproject/hello", "latest"}, + "quay.com:8080/fnproject/hello:v2": {"quay.com:8080", "fnproject/hello", "v2"}, "localhost.localdomain:5000/samalba/hipache:latest": {"localhost.localdomain:5000", "samalba/hipache", "latest"}, } diff --git a/api/datastore/internal/datastoretest/test.go b/api/datastore/internal/datastoretest/test.go index 84e95ef0b..144f50779 100644 --- a/api/datastore/internal/datastoretest/test.go +++ b/api/datastore/internal/datastoretest/test.go @@ -13,9 +13,9 @@ import ( "reflect" "time" - "github.com/sirupsen/logrus" "github.com/gin-gonic/gin" "github.com/go-openapi/strfmt" + "github.com/sirupsen/logrus" ) func setLogBuffer() *bytes.Buffer { diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 70f88b392..9baf8232d 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -12,7 +12,6 @@ import ( "path/filepath" "strings" - "github.com/sirupsen/logrus" "github.com/fnproject/fn/api/models" "github.com/go-sql-driver/mysql" _ "github.com/go-sql-driver/mysql" @@ -21,6 +20,7 @@ import ( _ "github.com/lib/pq" "github.com/mattn/go-sqlite3" _ "github.com/mattn/go-sqlite3" + "github.com/sirupsen/logrus" ) // this aims to be an ANSI-SQL compliant package that uses only question @@ -71,23 +71,23 @@ const ( ) type sqlStore struct { - db *sqlx.DB - insertAppQuery string - updateAppQuery string - getAppQuery string + db *sqlx.DB + insertAppQuery string + updateAppQuery string + getAppQuery string selectAppConfigQuery string - removeAppQuery string - insertRouteQuery string - updateRouteQuery string - checkRouteAppQuery string - checkRouteQuery string - getRouteQuery string - removeRouteQuery string - insertCallQuery string - getCallQuery string - insertLogQuery string - getLogQuery string - deleteLogQuery string + removeAppQuery string + insertRouteQuery string + updateRouteQuery string + checkRouteAppQuery string + checkRouteQuery string + getRouteQuery string + removeRouteQuery string + insertCallQuery string + getCallQuery string + insertLogQuery string + getLogQuery string + deleteLogQuery string } // New will open the db specified by url, create any tables necessary @@ -147,14 +147,14 @@ func New(url *url.URL) (models.Datastore, error) { } dstore := &sqlStore{ - db: db, - insertAppQuery: db.Rebind("INSERT INTO apps (name, config) VALUES (?, ?);"), + db: db, + insertAppQuery: db.Rebind("INSERT INTO apps (name, config) VALUES (?, ?);"), selectAppConfigQuery: db.Rebind(`SELECT config FROM apps WHERE name=?`), - updateAppQuery: db.Rebind(`UPDATE apps SET config=? WHERE name=?`), - removeAppQuery: db.Rebind(`DELETE FROM apps WHERE name = ?`), - getAppQuery: db.Rebind(`SELECT name, config FROM apps WHERE name=?`), - checkRouteAppQuery: db.Rebind(`SELECT 1 FROM apps WHERE name=?`), - checkRouteQuery: db.Rebind(`SELECT 1 FROM routes WHERE app_name=? AND path=?`), + updateAppQuery: db.Rebind(`UPDATE apps SET config=? WHERE name=?`), + removeAppQuery: db.Rebind(`DELETE FROM apps WHERE name = ?`), + getAppQuery: db.Rebind(`SELECT name, config FROM apps WHERE name=?`), + checkRouteAppQuery: db.Rebind(`SELECT 1 FROM apps WHERE name=?`), + checkRouteQuery: db.Rebind(`SELECT 1 FROM routes WHERE app_name=? AND path=?`), insertRouteQuery: db.Rebind(`INSERT INTO routes ( app_name, path, @@ -190,9 +190,9 @@ func New(url *url.URL) (models.Datastore, error) { path ) VALUES (?, ?, ?, ?, ?, ?, ?);`), - getCallQuery: db.Rebind(fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector)), + getCallQuery: db.Rebind(fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector)), insertLogQuery: db.Rebind(`INSERT INTO logs (id, log) VALUES (?, ?);`), - getLogQuery: db.Rebind(`SELECT log FROM logs WHERE id=?`), + getLogQuery: db.Rebind(`SELECT log FROM logs WHERE id=?`), deleteLogQuery: db.Rebind(`DELETE FROM logs WHERE id=?`), } return dstore, nil diff --git a/api/logs/log.go b/api/logs/log.go index b5daffa29..e6260d524 100644 --- a/api/logs/log.go +++ b/api/logs/log.go @@ -4,9 +4,9 @@ import ( "fmt" "net/url" - "github.com/sirupsen/logrus" "github.com/fnproject/fn/api/datastore/sql" "github.com/fnproject/fn/api/models" + "github.com/sirupsen/logrus" ) func New(dbURL string) (models.LogStore, error) { diff --git a/api/mqs/bolt.go b/api/mqs/bolt.go index b783554c8..7bcbfe015 100644 --- a/api/mqs/bolt.go +++ b/api/mqs/bolt.go @@ -11,10 +11,10 @@ import ( "path/filepath" "time" - "github.com/sirupsen/logrus" "github.com/boltdb/bolt" "github.com/fnproject/fn/api/common" "github.com/fnproject/fn/api/models" + "github.com/sirupsen/logrus" ) type BoltDbMQ struct { diff --git a/api/mqs/ironmq.go b/api/mqs/ironmq.go index 46b422b35..792289368 100644 --- a/api/mqs/ironmq.go +++ b/api/mqs/ironmq.go @@ -9,10 +9,10 @@ import ( "strings" "sync" - "github.com/sirupsen/logrus" "github.com/fnproject/fn/api/models" mq_config "github.com/iron-io/iron_go3/config" ironmq "github.com/iron-io/iron_go3/mq" + "github.com/sirupsen/logrus" ) type assoc struct { diff --git a/api/mqs/new.go b/api/mqs/new.go index 5b9733e39..0f46af388 100644 --- a/api/mqs/new.go +++ b/api/mqs/new.go @@ -6,9 +6,9 @@ import ( "net/url" "strings" - "github.com/sirupsen/logrus" "github.com/fnproject/fn/api/models" "github.com/opentracing/opentracing-go" + "github.com/sirupsen/logrus" ) // New will parse the URL and return the correct MQ implementation. diff --git a/api/server/apps_test.go b/api/server/apps_test.go index 33627fdfb..18f508164 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -7,12 +7,12 @@ import ( "strings" "testing" - "github.com/sirupsen/logrus" "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/logs" "github.com/fnproject/fn/api/models" "github.com/fnproject/fn/api/mqs" "github.com/gin-gonic/gin" + "github.com/sirupsen/logrus" ) func setLogBuffer() *bytes.Buffer { diff --git a/api/server/init.go b/api/server/init.go index d92d14430..2d2214565 100644 --- a/api/server/init.go +++ b/api/server/init.go @@ -7,8 +7,8 @@ import ( "os/signal" "strings" - "github.com/sirupsen/logrus" "github.com/gin-gonic/gin" + "github.com/sirupsen/logrus" "github.com/spf13/viper" ) diff --git a/fnlb/lb/allgrouper.go b/fnlb/lb/allgrouper.go index 5e9faf8df..28f825c36 100644 --- a/fnlb/lb/allgrouper.go +++ b/fnlb/lb/allgrouper.go @@ -17,12 +17,12 @@ import ( "time" "fmt" - "github.com/sirupsen/logrus" "github.com/coreos/go-semver/semver" "github.com/go-sql-driver/mysql" "github.com/jmoiron/sqlx" "github.com/lib/pq" "github.com/mattn/go-sqlite3" + "github.com/sirupsen/logrus" ) // NewAllGrouper returns a Grouper that will return the entire list of nodes diff --git a/fnlb/lb/proxy.go b/fnlb/lb/proxy.go index b295dfe07..f8c2b3892 100644 --- a/fnlb/lb/proxy.go +++ b/fnlb/lb/proxy.go @@ -7,11 +7,11 @@ import ( "net/http/httputil" "sync" - "github.com/sirupsen/logrus" "github.com/coreos/go-semver/semver" opentracing "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" "github.com/openzipkin/zipkin-go-opentracing" + "github.com/sirupsen/logrus" ) // TODO the load balancers all need to have the same list of nodes. gossip? diff --git a/fnlb/main.go b/fnlb/main.go index 911529756..a89ed20d0 100644 --- a/fnlb/main.go +++ b/fnlb/main.go @@ -12,9 +12,9 @@ import ( "syscall" "time" - "github.com/sirupsen/logrus" "github.com/coreos/go-semver/semver" "github.com/fnproject/fn/fnlb/lb" + "github.com/sirupsen/logrus" ) const VERSION = "0.0.56" From b971595e4415517f6f0e551369ef0a18278d1509 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Tue, 5 Sep 2017 23:14:19 +0300 Subject: [PATCH 3/8] Adding go-fmt.sh to circle config --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9d2f97068..8a27e01ee 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -33,6 +33,7 @@ jobs: # login here for tests - run: docker login -u $DOCKER_USER -p $DOCKER_PASS - run: ./test.sh + - run: ./go-fmt.sh - deploy: command: | if [ "${CIRCLE_BRANCH}" == "master" ]; then From d91d0fe79b568be68dd979f3b77c7c5319facd92 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Wed, 6 Sep 2017 01:58:00 +0300 Subject: [PATCH 4/8] Use appropriate query for route updating --- api/datastore/sql/sql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 9baf8232d..8726613b5 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -407,7 +407,7 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m return err } - res, err := tx.ExecContext(ctx, ds.updateAppQuery, + res, err := tx.ExecContext(ctx, ds.updateRouteQuery, route.Image, route.Format, route.Memory, From 6a541139a9a45b22055badacb2050c253357808f Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Wed, 6 Sep 2017 02:00:33 +0300 Subject: [PATCH 5/8] Make call.End more solid --- .circleci/config.yml | 1 - api/agent/agent.go | 4 +++- api/agent/call.go | 20 +++++++------------- go-fmt.sh | 2 +- 4 files changed, 11 insertions(+), 16 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8a27e01ee..9d2f97068 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -33,7 +33,6 @@ jobs: # login here for tests - run: docker login -u $DOCKER_USER -p $DOCKER_PASS - run: ./test.sh - - run: ./go-fmt.sh - deploy: command: | if [ "${CIRCLE_BRANCH}" == "master" ]; then diff --git a/api/agent/agent.go b/api/agent/agent.go index 0ec318297..0d3538232 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -224,7 +224,9 @@ func (a *agent) Submit(callI Call) error { // TODO if the context is timed out here we need to allocate some more time... // right now this only works b/c the db isn't using the context - return call.End(ctx, err) + call.End(ctx, err) + + return err } // getSlot must ensure that if it receives a slot, it will be returned, otherwise diff --git a/api/agent/call.go b/api/agent/call.go index 0cf89f15d..8faded451 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -35,7 +35,7 @@ type Call interface { // regardless of whether the execution failed or not. An error will be passed // to End, which if nil indicates a successful execution. Any error returned // from End will be returned as the error from Submit. - End(ctx context.Context, err error) error + End(ctx context.Context, err error) } // TODO build w/o closures... lazy @@ -267,7 +267,7 @@ func (c *call) Start(ctx context.Context) error { return nil } -func (c *call) End(ctx context.Context, err error) error { +func (c *call) End(ctx context.Context, err error) { span, ctx := opentracing.StartSpanFromContext(ctx, "agent_call_end") defer span.Finish() @@ -287,19 +287,13 @@ func (c *call) End(ctx context.Context, err error) error { // XXX (reed): delete MQ message, eventually } - // TODO since the function itself can reply directly to a client (or logs), - // this means that we could potentially store an error / timeout status for a - // call that ran successfully [by a user's perspective] - // TODO this should be update, really - if err := c.ds.InsertCall(ctx, c.Call); err != nil { - // TODO we should just log this error not return it to user? just issue storing call status but call is run + // need newer context because original one may be modified + // and no longer be valid for further context-bound operations + if err := c.ds.InsertCall(context.Background(), c.Call); err != nil { + // TODO we should just log this error not return it to user? + // just issue storing call status but call is run logrus.WithError(err).Error("error inserting call into datastore") } - - // return the original error so that this is returned from Submit (for sync) - // TODO we could just skip over (and log) and End errors and return slot.exec error - // in submit instead of doing this, it's a bit odd. thoughts? - return err } func (a *agent) route(ctx context.Context, appName, path string) (*models.Route, error) { diff --git a/go-fmt.sh b/go-fmt.sh index e147abab7..7b309228b 100755 --- a/go-fmt.sh +++ b/go-fmt.sh @@ -1,7 +1,7 @@ #! /bin/sh set -e -function listFilesExit() { +listFilesExit () { echo The following files need to have go fmt ran: echo $NEED_TO_FORMAT exit 1 From 9a89366d1bbf2f1f131e7fe123d341d270a0032f Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Wed, 6 Sep 2017 09:21:59 +0300 Subject: [PATCH 6/8] Addressing review comments reverting query string caching in favour of Go 1.9 sqlx features moving context definition out of call.End to upper level --- api/agent/agent.go | 5 +- api/agent/call.go | 6 +- api/agent/func_logger.go | 9 ++- api/datastore/sql/sql.go | 161 ++++++++++++++++++--------------------- go-fmt.sh | 2 +- 5 files changed, 86 insertions(+), 97 deletions(-) diff --git a/api/agent/agent.go b/api/agent/agent.go index 0d3538232..8187da493 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -222,9 +222,8 @@ func (a *agent) Submit(callI Call) error { a.stats.Complete() - // TODO if the context is timed out here we need to allocate some more time... - // right now this only works b/c the db isn't using the context - call.End(ctx, err) + // TODO if the context is timed out here we need to allocate new one + call.End(context.Background(), err) return err } diff --git a/api/agent/call.go b/api/agent/call.go index 8faded451..e636e3ef9 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -287,11 +287,7 @@ func (c *call) End(ctx context.Context, err error) { // XXX (reed): delete MQ message, eventually } - // need newer context because original one may be modified - // and no longer be valid for further context-bound operations - if err := c.ds.InsertCall(context.Background(), c.Call); err != nil { - // TODO we should just log this error not return it to user? - // just issue storing call status but call is run + if err := c.ds.InsertCall(opentracing.ContextWithSpan(ctx, span), c.Call); err != nil { logrus.WithError(err).Error("error inserting call into datastore") } } diff --git a/api/agent/func_logger.go b/api/agent/func_logger.go index fdd1bf4bc..ff394d7fa 100644 --- a/api/agent/func_logger.go +++ b/api/agent/func_logger.go @@ -10,6 +10,7 @@ import ( "github.com/fnproject/fn/api/common" "github.com/fnproject/fn/api/models" + "github.com/opentracing/opentracing-go" "github.com/sirupsen/logrus" ) @@ -56,7 +57,7 @@ func NewFuncLogger(ctx context.Context, appName, path, image, reqID string, logD reqID: reqID, }) - // TODO / NOTE: we want linew to be first becauase limitw may error if limit + // TODO / NOTE: we want linew to be first because limitw may error if limit // is reached but we still want to log. we should probably ignore hitting the // limit error since we really just want to not write too much to db and // that's handled as is. put buffers back last to avoid misuse, if there's @@ -110,7 +111,7 @@ type logWriter struct { func (l *logWriter) Write(b []byte) (int, error) { log := common.Logger(l.ctx) log = log.WithFields(logrus.Fields{"user_log": true, "app_name": l.appName, "path": l.path, "image": l.image, "call_id": l.reqID}) - log.Println(string(b)) + log.Debug(string(b)) return len(b), nil } @@ -182,7 +183,9 @@ type dbWriter struct { } func (w *dbWriter) Close() error { - return w.db.InsertLog(w.ctx, w.reqID, w.String()) + span, ctx := opentracing.StartSpanFromContext(context.Background(), "agent_log_write") + defer span.Finish() + return w.db.InsertLog(ctx, w.reqID, w.String()) } func (w *dbWriter) Write(b []byte) (int, error) { diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 8726613b5..87d3ad13a 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -71,23 +71,10 @@ const ( ) type sqlStore struct { - db *sqlx.DB - insertAppQuery string - updateAppQuery string - getAppQuery string - selectAppConfigQuery string - removeAppQuery string - insertRouteQuery string - updateRouteQuery string - checkRouteAppQuery string - checkRouteQuery string - getRouteQuery string - removeRouteQuery string - insertCallQuery string - getCallQuery string - insertLogQuery string - getLogQuery string - deleteLogQuery string + db *sqlx.DB + + // TODO we should prepare all of the statements, rebind them + // and store them all here. } // New will open the db specified by url, create any tables necessary @@ -146,56 +133,7 @@ func New(url *url.URL) (models.Datastore, error) { } } - dstore := &sqlStore{ - db: db, - insertAppQuery: db.Rebind("INSERT INTO apps (name, config) VALUES (?, ?);"), - selectAppConfigQuery: db.Rebind(`SELECT config FROM apps WHERE name=?`), - updateAppQuery: db.Rebind(`UPDATE apps SET config=? WHERE name=?`), - removeAppQuery: db.Rebind(`DELETE FROM apps WHERE name = ?`), - getAppQuery: db.Rebind(`SELECT name, config FROM apps WHERE name=?`), - checkRouteAppQuery: db.Rebind(`SELECT 1 FROM apps WHERE name=?`), - checkRouteQuery: db.Rebind(`SELECT 1 FROM routes WHERE app_name=? AND path=?`), - insertRouteQuery: db.Rebind(`INSERT INTO routes ( - app_name, - path, - image, - format, - memory, - type, - timeout, - idle_timeout, - headers, - config - ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?);`), - getRouteQuery: db.Rebind(fmt.Sprintf("%s WHERE app_name=? AND path=?", routeSelector)), - updateRouteQuery: db.Rebind(`UPDATE routes SET - image = ?, - format = ?, - memory = ?, - type = ?, - timeout = ?, - idle_timeout = ?, - headers = ?, - config = ? - WHERE app_name=? AND path=?;`), - removeRouteQuery: db.Rebind(`DELETE FROM routes WHERE path = ? AND app_name = ?`), - insertCallQuery: db.Rebind(`INSERT INTO calls ( - id, - created_at, - started_at, - completed_at, - status, - app_name, - path - ) - VALUES (?, ?, ?, ?, ?, ?, ?);`), - getCallQuery: db.Rebind(fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector)), - insertLogQuery: db.Rebind(`INSERT INTO logs (id, log) VALUES (?, ?);`), - getLogQuery: db.Rebind(`SELECT log FROM logs WHERE id=?`), - deleteLogQuery: db.Rebind(`DELETE FROM logs WHERE id=?`), - } - return dstore, nil + return &sqlStore{db: db}, nil } func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { @@ -208,7 +146,8 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App } } - _, err = ds.db.ExecContext(ctx, ds.insertAppQuery, app.Name, string(cbyte)) + query := ds.db.Rebind("INSERT INTO apps (name, config) VALUES (?, ?);") + _, err = ds.db.ExecContext(ctx, query, app.Name, string(cbyte)) if err != nil { switch err := err.(type) { case *mysql.MySQLError: @@ -233,7 +172,8 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { app := &models.App{Name: newapp.Name} err := ds.Tx(func(tx *sqlx.Tx) error { - row := tx.QueryRowContext(ctx, ds.selectAppConfigQuery, app.Name) + query := tx.Rebind(`SELECT config FROM apps WHERE name=?`) + row := tx.QueryRowContext(ctx, query, app.Name) var config string if err := row.Scan(&config); err != nil { @@ -257,7 +197,8 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. return err } - res, err := tx.ExecContext(ctx, ds.updateAppQuery, string(cbyte), app.Name) + query = tx.Rebind(`UPDATE apps SET config=? WHERE name=?`) + res, err := tx.ExecContext(ctx, query, string(cbyte), app.Name) if err != nil { return err } @@ -279,12 +220,14 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. } func (ds *sqlStore) RemoveApp(ctx context.Context, appName string) error { - _, err := ds.db.ExecContext(ctx, ds.removeAppQuery, appName) + query := ds.db.Rebind(`DELETE FROM apps WHERE name = ?`) + _, err := ds.db.ExecContext(ctx, query, appName) return err } func (ds *sqlStore) GetApp(ctx context.Context, name string) (*models.App, error) { - row := ds.db.QueryRowContext(ctx, ds.getAppQuery, name) + query := ds.db.Rebind(`SELECT name, config FROM apps WHERE name=?`) + row := ds.db.QueryRowContext(ctx, query, name) var resName, config string err := row.Scan(&resName, &config) @@ -351,13 +294,15 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode } err = ds.Tx(func(tx *sqlx.Tx) error { - r := tx.QueryRowContext(ctx, ds.checkRouteAppQuery, route.AppName) + query := tx.Rebind(`SELECT 1 FROM apps WHERE name=?`) + r := tx.QueryRowContext(ctx, query, route.AppName) if err := r.Scan(new(int)); err != nil { if err == sql.ErrNoRows { return models.ErrAppsNotFound } } - same, err := tx.QueryContext(ctx, ds.checkRouteQuery, route.AppName, route.Path) + query = tx.Rebind(`SELECT 1 FROM routes WHERE app_name=? AND path=?`) + same, err := tx.QueryContext(ctx, query, route.AppName, route.Path) if err != nil { return err } @@ -366,7 +311,21 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode return models.ErrRoutesAlreadyExists } - _, err = tx.ExecContext(ctx, ds.insertRouteQuery, + query = tx.Rebind(`INSERT INTO routes ( + app_name, + path, + image, + format, + memory, + type, + timeout, + idle_timeout, + headers, + config + ) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?);`) + + _, err = tx.ExecContext(ctx, query, route.AppName, route.Path, route.Image, @@ -388,7 +347,8 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*models.Route, error) { var route models.Route err := ds.Tx(func(tx *sqlx.Tx) error { - row := tx.QueryRowContext(ctx, ds.getRouteQuery, newroute.AppName, newroute.Path) + query := tx.Rebind(fmt.Sprintf("%s WHERE app_name=? AND path=?", routeSelector)) + row := tx.QueryRowContext(ctx, query, newroute.AppName, newroute.Path) if err := scanRoute(row, &route); err == sql.ErrNoRows { return models.ErrRoutesNotFound } else if err != nil { @@ -407,7 +367,18 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m return err } - res, err := tx.ExecContext(ctx, ds.updateRouteQuery, + query = tx.Rebind(`UPDATE routes SET + image = ?, + format = ?, + memory = ?, + type = ?, + timeout = ?, + idle_timeout = ?, + headers = ?, + config = ? + WHERE app_name=? AND path=?;`) + + res, err := tx.ExecContext(ctx, query, route.Image, route.Format, route.Memory, @@ -441,7 +412,8 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m } func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) error { - res, err := ds.db.ExecContext(ctx, ds.removeRouteQuery, routePath, appName) + query := ds.db.Rebind(`DELETE FROM routes WHERE path = ? AND app_name = ?`) + res, err := ds.db.ExecContext(ctx, query, routePath, appName) if err != nil { return err } @@ -459,7 +431,9 @@ func (ds *sqlStore) RemoveRoute(ctx context.Context, appName, routePath string) } func (ds *sqlStore) GetRoute(ctx context.Context, appName, routePath string) (*models.Route, error) { - row := ds.db.QueryRowContext(ctx, ds.getRouteQuery, appName, routePath) + rSelectCondition := "%s WHERE app_name=? AND path=?" + query := ds.db.Rebind(fmt.Sprintf(rSelectCondition, routeSelector)) + row := ds.db.QueryRowContext(ctx, query, appName, routePath) var route models.Route err := scanRoute(row, &route) @@ -475,7 +449,8 @@ func (ds *sqlStore) GetRoute(ctx context.Context, appName, routePath string) (*m func (ds *sqlStore) GetRoutes(ctx context.Context, filter *models.RouteFilter) ([]*models.Route, error) { res := []*models.Route{} query, args := buildFilterRouteQuery(filter) - query = ds.db.Rebind(fmt.Sprintf("%s %s", routeSelector, query)) + query = fmt.Sprintf("%s %s", routeSelector, query) + query = ds.db.Rebind(query) rows, err := ds.db.QueryContext(ctx, query, args...) // todo: check for no rows so we don't respond with a sql 500 err if err != nil { @@ -552,7 +527,18 @@ func (ds *sqlStore) Tx(f func(*sqlx.Tx) error) error { } func (ds *sqlStore) InsertCall(ctx context.Context, call *models.Call) error { - _, err := ds.db.ExecContext(ctx, ds.insertCallQuery, call.ID, call.CreatedAt.String(), + query := ds.db.Rebind(`INSERT INTO calls ( + id, + created_at, + started_at, + completed_at, + status, + app_name, + path + ) + VALUES (?, ?, ?, ?, ?, ?, ?);`) + + _, err := ds.db.ExecContext(ctx, query, call.ID, call.CreatedAt.String(), call.StartedAt.String(), call.CompletedAt.String(), call.Status, call.AppName, call.Path) if err != nil { @@ -566,7 +552,9 @@ func (ds *sqlStore) InsertCall(ctx context.Context, call *models.Call) error { // if we store the whole thing then it adds a lot of disk space and then we can // make async only queue hints instead of entire calls (mq a lot smaller space wise). pick. func (ds *sqlStore) GetCall(ctx context.Context, appName, callID string) (*models.Call, error) { - row := ds.db.QueryRowContext(ctx, ds.getCallQuery, callID, appName) + query := fmt.Sprintf(`%s WHERE id=? AND app_name=?`, callSelector) + query = ds.db.Rebind(query) + row := ds.db.QueryRowContext(ctx, query, callID, appName) var call models.Call err := scanCall(row, &call) @@ -602,12 +590,14 @@ func (ds *sqlStore) GetCalls(ctx context.Context, filter *models.CallFilter) ([] } func (ds *sqlStore) InsertLog(ctx context.Context, callID, callLog string) error { - _, err := ds.db.ExecContext(ctx, ds.insertLogQuery, callID, callLog) + query := ds.db.Rebind(`INSERT INTO logs (id, log) VALUES (?, ?);`) + _, err := ds.db.ExecContext(ctx, query, callID, callLog) return err } func (ds *sqlStore) GetLog(ctx context.Context, callID string) (*models.CallLog, error) { - row := ds.db.QueryRowContext(ctx, ds.getLogQuery, callID) + query := ds.db.Rebind(`SELECT log FROM logs WHERE id=?`) + row := ds.db.QueryRowContext(ctx, query, callID) var log string err := row.Scan(&log) @@ -625,7 +615,8 @@ func (ds *sqlStore) GetLog(ctx context.Context, callID string) (*models.CallLog, } func (ds *sqlStore) DeleteLog(ctx context.Context, callID string) error { - _, err := ds.db.ExecContext(ctx, ds.deleteLogQuery, callID) + query := ds.db.Rebind(`DELETE FROM logs WHERE id=?`) + _, err := ds.db.ExecContext(ctx, query, callID) return err } diff --git a/go-fmt.sh b/go-fmt.sh index 7b309228b..49769b25c 100755 --- a/go-fmt.sh +++ b/go-fmt.sh @@ -1,7 +1,7 @@ #! /bin/sh set -e -listFilesExit () { +function listFilesExit () { echo The following files need to have go fmt ran: echo $NEED_TO_FORMAT exit 1 From 57a577dfc972cb7af778e0a28fff911cebc8ff06 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Wed, 6 Sep 2017 21:55:52 +0300 Subject: [PATCH 7/8] Wiring new context with initial span --- api/agent/agent.go | 3 ++- api/agent/call.go | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/agent/agent.go b/api/agent/agent.go index 8187da493..2dd1329e1 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -223,7 +223,8 @@ func (a *agent) Submit(callI Call) error { a.stats.Complete() // TODO if the context is timed out here we need to allocate new one - call.End(context.Background(), err) + ctx = opentracing.ContextWithSpan(context.Background(), span) + call.End(ctx, err) return err } diff --git a/api/agent/call.go b/api/agent/call.go index e636e3ef9..3d58dc147 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -103,10 +103,10 @@ func FromRequest(appName, path string, req *http.Request) CallOpt { envVars[toEnvName("PARAM", param.Key)] = param.Value } - headerVars := make(map[string]string,len(req.Header)) + headerVars := make(map[string]string, len(req.Header)) for k, v := range req.Header { - headerVars[toEnvName("HEADER",k)] = strings.Join(v, ", ") + headerVars[toEnvName("HEADER", k)] = strings.Join(v, ", ") } // add all the env vars we build to the request headers @@ -115,7 +115,7 @@ func FromRequest(appName, path string, req *http.Request) CallOpt { req.Header.Add(k, v) } - for k,v := range headerVars { + for k, v := range headerVars { envVars[k] = v } From 8a337e744ba5fefa3d897216e7c08af8ffd9699b Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Thu, 7 Sep 2017 15:16:18 +0300 Subject: [PATCH 8/8] Addresing new comments --- api/agent/agent.go | 3 ++- api/agent/call.go | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/api/agent/agent.go b/api/agent/agent.go index 2dd1329e1..d8aa525d6 100644 --- a/api/agent/agent.go +++ b/api/agent/agent.go @@ -222,7 +222,8 @@ func (a *agent) Submit(callI Call) error { a.stats.Complete() - // TODO if the context is timed out here we need to allocate new one + // TODO: we need to allocate more time to store the call + logs in case the call timed out, + // but this could put us over the timeout if the call did not reply yet (need better policy). ctx = opentracing.ContextWithSpan(context.Background(), span) call.End(ctx, err) diff --git a/api/agent/call.go b/api/agent/call.go index 3d58dc147..033561b3e 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -287,7 +287,10 @@ func (c *call) End(ctx context.Context, err error) { // XXX (reed): delete MQ message, eventually } - if err := c.ds.InsertCall(opentracing.ContextWithSpan(ctx, span), c.Call); err != nil { + // this means that we could potentially store an error / timeout status for a + // call that ran successfully [by a user's perspective] + // TODO: this should be update, really + if err := c.ds.InsertCall(ctx, c.Call); err != nil { logrus.WithError(err).Error("error inserting call into datastore") } }