From 2dcfb5ca2883c6350d7b31eae67709a80a818488 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 13 May 2021 16:22:37 +0100 Subject: [PATCH] Review feedback --- .../horizon/internal/db2/history/accounts.go | 2 +- .../internal/db2/history/trust_lines.go | 2 +- services/horizon/internal/httpx/middleware.go | 2 +- services/horizon/internal/init.go | 9 ++-- support/db/metrics.go | 47 ++++++++++++------- 5 files changed, 39 insertions(+), 23 deletions(-) diff --git a/services/horizon/internal/db2/history/accounts.go b/services/horizon/internal/db2/history/accounts.go index f5740013c0..a4b60fd382 100644 --- 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 index 98cc7450fc..3dfe50b80d 100644 --- 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 index 08ce7bea00..cfc36274f4 100644 --- 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 index 9c66912d18..5665e125e0 100644 --- 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 index 1d53c59c6f..1dbfed3d8e 100644 --- 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,