From 53cffad06024625d10c9439c67d42857c7a79647 Mon Sep 17 00:00:00 2001 From: Benjamin Ramser Date: Thu, 23 Mar 2023 09:37:24 +0100 Subject: [PATCH] fix: prometheus with multiple providers we encountered an issue where tegola would panic if you'd try to register duplicate metrics collectors. that would happen if you have more than one mvt provider in the same config file. this fix makes provider metrics unique by adding the respective provider name to an array of constant labels. Upon collecting metrics on start up we'd pass unique metrics to the registration step. resolves: #886 chore: fixing casing chore: add constant labels to hana provider --- internal/build/build.go | 4 ++-- provider/hana/hana.go | 36 +++++++++++++++++++++------- provider/postgis/postgis.go | 48 +++++++++++++++++++++++++++---------- server/server.go | 9 +++---- 4 files changed, 70 insertions(+), 27 deletions(-) diff --git a/internal/build/build.go b/internal/build/build.go index 4acb3291c..17810bbb0 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -8,10 +8,10 @@ import ( ) var ( - Version = "Version not set" + Version = "version not set" GitRevision = "not set" GitBranch = "not set" - uiVersionDefaultText = "Viewer not build" + uiVersionDefaultText = "viewer not build" Tags []string Commands = []string{"tegola"} ) diff --git a/provider/hana/hana.go b/provider/hana/hana.go index 50a418e12..bddc996ff 100644 --- a/provider/hana/hana.go +++ b/provider/hana/hana.go @@ -26,7 +26,12 @@ import ( const Name = "hana" type connectionPoolCollector struct { - pool *sql.DB + pool *sql.DB + + // providerName the pool is created for + // required to make metrics unique + providerName string + maxConnectionDesc *prometheus.Desc currentConnectionsDesc *prometheus.Desc availableConnectionsDesc *prometheus.Desc @@ -106,28 +111,30 @@ func (c *connectionPoolCollector) Collectors(prefix string, _ func(configKey str prefix+"hana_max_connections", "Max number of hana connections in the pool", nil, - nil, + prometheus.Labels{"provider_name": c.providerName}, ) c.currentConnectionsDesc = prometheus.NewDesc( prefix+"hana_current_connections", "Current number of hana connections in the pool", nil, - nil, + prometheus.Labels{"provider_name": c.providerName}, ) c.availableConnectionsDesc = prometheus.NewDesc( prefix+"hana_available_connections", "Current number of available hana connections in the pool", nil, - nil, + prometheus.Labels{"provider_name": c.providerName}, ) + return []observability.Collector{c}, nil } // Provider provides the HANA data provider. type Provider struct { dbVersion uint + name string pool *connectionPoolCollector // map of layer name and corresponding sql layers map[string]Layer @@ -387,7 +394,13 @@ func CreateProvider(config dict.Dicter, maps []provider.Map, providerType string return nil, err } + name, err := config.String(ConfigKeyName, nil) + if err != nil { + return nil, err + } + p := Provider{ + name: name, dbVersion: uint(majorVersion), srid: uint64(srid), pool: &connectionPoolCollector{pool: conn}, @@ -405,7 +418,7 @@ func CreateProvider(config dict.Dicter, maps []provider.Map, providerType string lName, err := layer.String(ConfigKeyLayerName, nil) if err != nil { - return nil, fmt.Errorf("For layer (%v) we got the following error trying to get the layer's name field: %w", i, err) + return nil, fmt.Errorf("for layer (%v) we got the following error trying to get the layer's name field: %w", i, err) } if j, ok := lyrsSeen[lName]; ok { @@ -480,7 +493,7 @@ func CreateProvider(config dict.Dicter, maps []provider.Map, providerType string if isSrsRoundEarth(p.pool, uint64(lsrid)) { if !hasSrsPlanarEquivalent(p.pool, uint64(lsrid)) { - return nil, fmt.Errorf("Unable to find a planar equivalent for srid %v in layer: %v", lsrid, lName) + return nil, fmt.Errorf("unable to find a planar equivalent for srid %v in layer: %v", lsrid, lName) } lsrid = int(toPlanarEquivalenSrid(uint64(lsrid))) } @@ -562,7 +575,7 @@ func CreateProvider(config dict.Dicter, maps []provider.Map, providerType string return nil, err } - var clipGeom bool = true + var clipGeom = true if clipGeom, err = layer.Bool(ConfigKeyClipGeometry, &clipGeom); err != nil { return nil, err } @@ -665,6 +678,10 @@ func (p Provider) inspectLayerGeomType(pname string, l *Layer, maps []provider.M } fields, err := getFieldDescriptions(l.Name(), l.GeomFieldName(), l.IDFieldName(), columns, false) + if err != nil { + return err + } + rowValues := make([]interface{}, len(fields)) for rows.Next() { @@ -681,7 +698,10 @@ func (p Provider) inspectLayerGeomType(pname string, l *Layer, maps []provider.M } value := *(rowValues[i].(*sql.NullString)) - p.setLayerGeomType(l, strings.Trim(value.String, "ST_")) + err := p.setLayerGeomType(l, strings.Trim(value.String, "ST_")) + if err != nil { + return err + } break } diff --git a/provider/postgis/postgis.go b/provider/postgis/postgis.go index 8c7be5014..9dbe1314a 100644 --- a/provider/postgis/postgis.go +++ b/provider/postgis/postgis.go @@ -34,6 +34,11 @@ const Name = "postgis" type connectionPoolCollector struct { *pgxpool.Pool + + // providerName the pool is created for + // required to make metrics unique + providerName string + maxConnectionDesc *prometheus.Desc currentConnectionsDesc *prometheus.Desc availableConnectionsDesc *prometheus.Desc @@ -47,7 +52,7 @@ func (c connectionPoolCollector) Collect(ch chan<- prometheus.Metric) { if c.Pool == nil { return } - stat := c.Pool.Stat() + stat := c.Stat() ch <- prometheus.MustNewConstMetric( c.maxConnectionDesc, prometheus.GaugeValue, @@ -73,25 +78,28 @@ func (c *connectionPoolCollector) Collectors(prefix string, _ func(configKey str prefix = prefix + "_" } + // a constant label ensures that the metrics are unique + // this allows the registration of multiple providers in the same + // config. c.maxConnectionDesc = prometheus.NewDesc( prefix+"postgres_max_connections", "Max number of postgres connections in the pool", nil, - nil, + prometheus.Labels{"provider_name": c.providerName}, ) c.currentConnectionsDesc = prometheus.NewDesc( prefix+"postgres_current_connections", "Current number of postgres connections in the pool", nil, - nil, + prometheus.Labels{"provider_name": c.providerName}, ) c.availableConnectionsDesc = prometheus.NewDesc( prefix+"postgres_available_connections", "Current number of available postgres connections in the pool", nil, - nil, + prometheus.Labels{"provider_name": c.providerName}, ) return []observability.Collector{c}, nil } @@ -99,7 +107,9 @@ func (c *connectionPoolCollector) Collectors(prefix string, _ func(configKey str // Provider provides the postgis data provider. type Provider struct { config pgxpool.Config + name string pool *connectionPoolCollector + // map of layer name and corresponding sql layers map[string]Layer srid uint64 @@ -122,31 +132,37 @@ func (p *Provider) Collectors(prefix string, cfgFn func(configKey string) map[st } buckets := []float64{.1, 1, 5, 20} - collectors, err := p.pool.Collectors(prefix, cfgFn) + c, err := p.pool.Collectors(prefix, cfgFn) if err != nil { return nil, err } + // a constant label ensures that the metrics are unique + // this allows the registration of multiple providers in the same + // config. + // Additional label names will be appended to the constant labels. p.mvtProviderQueryHistogramSeconds = prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Name: prefix + "_mvt_provider_sql_query_seconds", - Help: "A histogram of query time for sql for mvt providers", - Buckets: buckets, + Name: prefix + "_mvt_provider_sql_query_seconds", + Help: "A histogram of query time for sql for mvt providers", + Buckets: buckets, + ConstLabels: prometheus.Labels{"provider_name": p.name}, }, []string{"map_name", "z"}, ) p.queryHistogramSeconds = prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Name: prefix + "_provider_sql_query_seconds", - Help: "A histogram of query time for sql for providers", - Buckets: buckets, + Name: prefix + "_provider_sql_query_seconds", + Help: "A histogram of query time for sql for providers", + Buckets: buckets, + ConstLabels: prometheus.Labels{"provider_name": p.name}, }, []string{"map_name", "layer_name", "z"}, ) p.collectorsRegistered = true - return append(collectors, p.mvtProviderQueryHistogramSeconds, p.queryHistogramSeconds), nil + return append(c, p.mvtProviderQueryHistogramSeconds, p.queryHistogramSeconds), nil } const ( @@ -460,6 +476,11 @@ func CreateProvider(config dict.Dicter, maps []provider.Map, providerType string return nil, err } + name, err := config.String(ConfigKeyName, nil) + if err != nil { + return nil, err + } + if err = ConfigTLS(sslmode, sslkey, sslcert, sslrootcert, dbconfig); err != nil { return nil, err } @@ -467,13 +488,14 @@ func CreateProvider(config dict.Dicter, maps []provider.Map, providerType string p := Provider{ srid: uint64(srid), config: *dbconfig, + name: name, } pool, err := pgxpool.ConnectConfig(context.Background(), &p.config) if err != nil { return nil, fmt.Errorf("failed while creating connection pool: %w", err) } - p.pool = &connectionPoolCollector{Pool: pool} + p.pool = &connectionPoolCollector{Pool: pool, providerName: name} layers, err := config.MapSlice(ConfigKeyLayers) if err != nil { diff --git a/server/server.go b/server/server.go index 2e7cd61ac..1f4e7461d 100644 --- a/server/server.go +++ b/server/server.go @@ -3,11 +3,12 @@ package server import ( "fmt" - "github.com/go-spatial/tegola/internal/build" "net/http" "net/url" "path" + "github.com/go-spatial/tegola/internal/build" + "github.com/go-spatial/tegola/observability" "github.com/dimfeld/httptreemux" @@ -25,7 +26,7 @@ const ( var ( // Version is the version of the software, this should be set by the main program, before starting up. // It is used by various Middleware to determine the version. - Version string = "Version Not Set" + Version string = "version not set" // HostName is the name of the host to use for construction of URLS. // configurable via the tegola config.toml file (set in main.go) @@ -71,7 +72,7 @@ func NewRouter(a *atlas.Atlas) *httptreemux.TreeMux { ) if h := o.Handler(metricsRoute); h != nil { // Only set up the /metrics endpoint if we have a configured observer - log.Infof("Setting up observer: %v", o.Name()) + log.Infof("setting up observer: %v", o.Name()) group.UsingContext().Handler(http.MethodGet, metricsRoute, h) } } @@ -98,7 +99,7 @@ func NewRouter(a *atlas.Atlas) *httptreemux.TreeMux { func Start(a *atlas.Atlas, port string) *http.Server { // notify the user the server is starting - log.Infof("starting tegola server(%v) on port %v", build.Version, port) + log.Infof("starting tegola server (%v) on port %v", build.Version, port) srv := &http.Server{Addr: port, Handler: NewRouter(a)}