Review feedback

stellar · May 14, 2021 · 2dcfb5c · 2dcfb5c
1 parent 95f619e
commit 2dcfb5c
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 23 deletions.
diff --git a/services/horizon/internal/db2/history/accounts.go b/services/horizon/internal/db2/history/accounts.go
@@ -191,7 +191,7 @@ func (q *Q) UpsertAccounts(ctx context.Context, accounts []xdr.LedgerEntry) erro
 		num_sponsoring = excluded.num_sponsoring`
 
 	_, err := q.ExecRaw(
-		context.WithValue(ctx, &db.QueryTypeContextKey, "upsert"),
+		context.WithValue(ctx, &db.QueryTypeContextKey, db.UpsertQueryType),
 		sql,
 		pq.Array(accountID),
 		pq.Array(balance),

diff --git a/services/horizon/internal/db2/history/trust_lines.go b/services/horizon/internal/db2/history/trust_lines.go
@@ -223,7 +223,7 @@ func (q *Q) UpsertTrustLines(ctx context.Context, trustLines []xdr.LedgerEntry)
 		sponsor = excluded.sponsor`
 
 	_, err := q.ExecRaw(
-		context.WithValue(ctx, &db.QueryTypeContextKey, "upsert"),
+		context.WithValue(ctx, &db.QueryTypeContextKey, db.UpsertQueryType),
 		sql,
 		pq.Array(ledgerKey),
 		pq.Array(accountID),

diff --git a/services/horizon/internal/httpx/middleware.go b/services/horizon/internal/httpx/middleware.go
@@ -142,7 +142,7 @@ func sanitizeMetricRoute(routePattern string) string {
 	route = strings.ReplaceAll(route, "\"", "\\\"")
 	route = strings.ReplaceAll(route, "\n", "\\n")
 	if route == "" {
-		// Can be empty when request did not reached the final route (ex. blocked by
+		// Can be empty when request did not reach the final route (ex. blocked by
 		// a middleware). More info: https://github.com/go-chi/chi/issues/270
 		return "undefined"
 	}

diff --git a/services/horizon/internal/init.go b/services/horizon/internal/init.go
@@ -18,7 +18,7 @@ import (
 	"github.com/stellar/go/support/log"
 )
 
-func mustNewDBSession(subsystem string, databaseURL string, maxIdle, maxOpen int, registry *prometheus.Registry) db.SessionInterface {
+func mustNewDBSession(subsystem db.Subsystem, databaseURL string, maxIdle, maxOpen int, registry *prometheus.Registry) db.SessionInterface {
 	session, err := db.Open("postgres", databaseURL)
 	if err != nil {
 		log.Fatalf("cannot open Horizon DB: %v", err)
@@ -44,7 +44,7 @@ func mustInitHorizonDB(app *App) {
 	}
 
 	app.historyQ = &history.Q{mustNewDBSession(
-		"history",
+		db.HistorySubsystem,
 		app.config.DatabaseURL,
 		maxIdle,
 		maxOpen,
@@ -56,12 +56,13 @@ func initIngester(app *App) {
 	var err error
 	var coreSession db.SessionInterface
 	if !app.config.EnableCaptiveCoreIngestion {
-		coreSession = mustNewDBSession("core", app.config.StellarCoreDatabaseURL, ingest.MaxDBConnections, ingest.MaxDBConnections, app.prometheusRegistry)
+		coreSession = mustNewDBSession(
+			db.CoreSubsystem, app.config.StellarCoreDatabaseURL, ingest.MaxDBConnections, ingest.MaxDBConnections, app.prometheusRegistry)
 	}
 	app.ingester, err = ingest.NewSystem(ingest.Config{
 		CoreSession: coreSession,
 		HistorySession: mustNewDBSession(
-			"ingest", app.config.DatabaseURL, ingest.MaxDBConnections, ingest.MaxDBConnections, app.prometheusRegistry,
+			db.IngestSubsystem, app.config.DatabaseURL, ingest.MaxDBConnections, ingest.MaxDBConnections, app.prometheusRegistry,
 		),
 		NetworkPassphrase: app.config.NetworkPassphrase,
 		// TODO:

diff --git a/support/db/metrics.go b/support/db/metrics.go
@@ -16,6 +16,21 @@ type CtxKey string
 var RouteContextKey = CtxKey("route")
 var QueryTypeContextKey = CtxKey("query_type")
 
+type Subsystem string
+
+var CoreSubsystem = Subsystem("core")
+var HistorySubsystem = Subsystem("history")
+var IngestSubsystem = Subsystem("ingest")
+
+type QueryType string
+
+var DeleteQueryType = QueryType("delete")
+var InsertQueryType = QueryType("insert")
+var SelectQueryType = QueryType("select")
+var UndefinedQueryType = QueryType("undefined")
+var UpdateQueryType = QueryType("update")
+var UpsertQueryType = QueryType("upsert")
+
 // contextRoute returns a string representing the request endpoint, or "undefined" if it wasn't found
 func contextRoute(ctx context.Context) string {
 	if endpoint, ok := ctx.Value(&RouteContextKey).(string); ok {
@@ -40,7 +55,7 @@ type SessionWithMetrics struct {
 	maxLifetimeClosedCounter prometheus.CounterFunc
 }
 
-func RegisterMetrics(base *Session, namespace, sub string, registry *prometheus.Registry) SessionInterface {
+func RegisterMetrics(base *Session, namespace string, sub Subsystem, registry *prometheus.Registry) SessionInterface {
 	subsystem := fmt.Sprintf("db_%s", sub)
 	s := &SessionWithMetrics{
 		SessionInterface: base,
@@ -214,10 +229,10 @@ func (s *SessionWithMetrics) TruncateTables(ctx context.Context, tables []string
 
 func (s *SessionWithMetrics) Clone() SessionInterface {
 	return &SessionWithMetrics{
-		SessionInterface:         s.SessionInterface.Clone(),
-		registry:                 s.registry,
-		queryCounter:             s.queryCounter,
-		queryDurationSummary:     s.queryDurationSummary,
+		SessionInterface:     s.SessionInterface.Clone(),
+		registry:             s.registry,
+		queryCounter:         s.queryCounter,
+		queryDurationSummary: s.queryDurationSummary,
 		// txnCounter:               s.txnCounter,
 		// txnDurationSummary:       s.txnDurationSummary,
 		maxOpenConnectionsGauge:  s.maxOpenConnectionsGauge,
@@ -232,25 +247,25 @@ func (s *SessionWithMetrics) Clone() SessionInterface {
 	}
 }
 
-func getQueryType(ctx context.Context, query squirrel.Sqlizer) string {
+func getQueryType(ctx context.Context, query squirrel.Sqlizer) QueryType {
 	// Do we have an explicit query type set in the context? For raw execs, in
 	// lieu of better detection. e.g. "upsert"
-	if q, ok := ctx.Value(&QueryTypeContextKey).(string); ok {
+	if q, ok := ctx.Value(&QueryTypeContextKey).(QueryType); ok {
 		return q
 	}
 
 	// is it a squirrel builder?
 	if _, ok := query.(squirrel.DeleteBuilder); ok {
-		return "delete"
+		return DeleteQueryType
 	}
 	if _, ok := query.(squirrel.InsertBuilder); ok {
-		return "insert"
+		return InsertQueryType
 	}
 	if _, ok := query.(squirrel.SelectBuilder); ok {
-		return "select"
+		return SelectQueryType
 	}
 	if _, ok := query.(squirrel.UpdateBuilder); ok {
-		return "update"
+		return UpdateQueryType
 	}
 
 	// Try to guess based on the first word of the string.
@@ -262,17 +277,17 @@ func getQueryType(ctx context.Context, query squirrel.Sqlizer) string {
 		// complex query.
 		for _, word := range []string{"delete", "insert", "select", "update"} {
 			if word == words[0] {
-				return word
+				return QueryType(word)
 			}
 		}
 	}
 
 	// Fresh out of ideas.
-	return "undefined"
+	return UndefinedQueryType
 }
 
 func (s *SessionWithMetrics) Get(ctx context.Context, dest interface{}, query squirrel.Sqlizer) (err error) {
-	queryType := getQueryType(ctx, query)
+	queryType := string(getQueryType(ctx, query))
 	timer := prometheus.NewTimer(prometheus.ObserverFunc(func(v float64) {
 		s.queryDurationSummary.With(prometheus.Labels{
 			"query_type": queryType,
@@ -298,7 +313,7 @@ func (s *SessionWithMetrics) GetRaw(ctx context.Context, dest interface{}, query
 }
 
 func (s *SessionWithMetrics) Select(ctx context.Context, dest interface{}, query squirrel.Sqlizer) (err error) {
-	queryType := getQueryType(ctx, query)
+	queryType := string(getQueryType(ctx, query))
 	timer := prometheus.NewTimer(prometheus.ObserverFunc(func(v float64) {
 		s.queryDurationSummary.With(prometheus.Labels{
 			"query_type": queryType,
@@ -324,7 +339,7 @@ func (s *SessionWithMetrics) SelectRaw(ctx context.Context, dest interface{}, qu
 }
 
 func (s *SessionWithMetrics) Exec(ctx context.Context, query squirrel.Sqlizer) (result sql.Result, err error) {
-	queryType := getQueryType(ctx, query)
+	queryType := string(getQueryType(ctx, query))
 	timer := prometheus.NewTimer(prometheus.ObserverFunc(func(v float64) {
 		s.queryDurationSummary.With(prometheus.Labels{
 			"query_type": queryType,