-
Notifications
You must be signed in to change notification settings - Fork 725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gitlab Collector: Index size collector and test #835
base: master
Are you sure you want to change the base?
Changes from 3 commits
7c4b2e6
7456362
d526f44
d0bc3e3
97d244e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
// Copyright 2023 The Prometheus Authors | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package collector | ||
|
||
import ( | ||
"context" | ||
"database/sql" | ||
|
||
"github.com/go-kit/log" | ||
"github.com/prometheus/client_golang/prometheus" | ||
) | ||
|
||
const indexSizeSubsystem = "index_size" | ||
|
||
func init() { | ||
registerCollector(indexSizeSubsystem, defaultDisabled, NewPGIndexSizeCollector) | ||
} | ||
|
||
type PGIndexSizeCollector struct { | ||
log log.Logger | ||
} | ||
|
||
func NewPGIndexSizeCollector(config collectorConfig) (Collector, error) { | ||
return &PGIndexSizeCollector{log: config.logger}, nil | ||
} | ||
|
||
var ( | ||
indexSizeDesc = prometheus.NewDesc( | ||
prometheus.BuildFQName(namespace, indexSizeSubsystem, "bytes"), | ||
"Size of the index as per pg_table_size function", | ||
[]string{"schemaname", "relname", "indexrelname"}, | ||
prometheus.Labels{}, | ||
) | ||
|
||
indexSizeQuery = ` | ||
SELECT | ||
schemaname, | ||
tablename AS relname, | ||
indexname AS indexrelname, | ||
pg_class.relpages * 8192::bigint AS index_size | ||
FROM | ||
pg_indexes | ||
INNER JOIN pg_namespace | ||
ON pg_indexes.schemaname = pg_namespace.nspname | ||
INNER JOIN pg_class | ||
ON pg_class.relnamespace = pg_namespace.oid AND pg_class.relname = pg_indexes.indexname | ||
WHERE | ||
pg_indexes.schemaname != 'pg_catalog' | ||
` | ||
) | ||
|
||
func (PGIndexSizeCollector) Update(ctx context.Context, instance *instance, ch chan<- prometheus.Metric) error { | ||
db := instance.getDB() | ||
rows, err := db.QueryContext(ctx, | ||
indexSizeQuery) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
defer rows.Close() | ||
|
||
for rows.Next() { | ||
var schemaname, relname, indexrelname sql.NullString | ||
var indexSize sql.NullFloat64 | ||
|
||
if err := rows.Scan(&schemaname, &relname, &indexrelname, &indexSize); err != nil { | ||
return err | ||
} | ||
schemanameLabel := "unknown" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm really not sure that these metrics even make sense if all of these are NULL. I understand that we used NULL to fix all of the bugs in the 0.13.1 release, but these are net new collectors and I would like to be much more thoughtful about how we handle them in code. For example, if the indexSize metric is NULL, why would we even report the metric? How is the zero value useful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, if the labels are null, we should Debug log and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this code is still emitting metrics when these values are NULL. |
||
if schemaname.Valid { | ||
schemanameLabel = schemaname.String | ||
} | ||
relnameLabel := "unknown" | ||
if relname.Valid { | ||
relnameLabel = relname.String | ||
} | ||
indexrelnameLabel := "unknown" | ||
if indexrelname.Valid { | ||
indexrelnameLabel = indexrelname.String | ||
} | ||
labels := []string{schemanameLabel, relnameLabel, indexrelnameLabel} | ||
|
||
indexSizeMetric := 0.0 | ||
if indexSize.Valid { | ||
indexSizeMetric = indexSize.Float64 | ||
} | ||
ch <- prometheus.MustNewConstMetric( | ||
indexSizeDesc, | ||
prometheus.GaugeValue, | ||
indexSizeMetric, | ||
labels..., | ||
) | ||
} | ||
if err := rows.Err(); err != nil { | ||
return err | ||
} | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
// Copyright 2023 The Prometheus Authors | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
package collector | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
"github.com/DATA-DOG/go-sqlmock" | ||
"github.com/prometheus/client_golang/prometheus" | ||
dto "github.com/prometheus/client_model/go" | ||
"github.com/smartystreets/goconvey/convey" | ||
) | ||
|
||
func TestPgIndexSizeCollector(t *testing.T) { | ||
db, mock, err := sqlmock.New() | ||
if err != nil { | ||
t.Fatalf("Error opening a stub db connection: %s", err) | ||
} | ||
defer db.Close() | ||
inst := &instance{db: db} | ||
columns := []string{ | ||
"schemaname", | ||
"relname", | ||
"indexrelname", | ||
"index_size", | ||
} | ||
rows := sqlmock.NewRows(columns). | ||
AddRow("public", "foo", "foo_key", 100) | ||
|
||
mock.ExpectQuery(sanitizeQuery(indexSizeQuery)).WillReturnRows(rows) | ||
|
||
ch := make(chan prometheus.Metric) | ||
go func() { | ||
defer close(ch) | ||
c := PGIndexSizeCollector{} | ||
|
||
if err := c.Update(context.Background(), inst, ch); err != nil { | ||
t.Errorf("Error calling PGIndexSizeCollector.Update: %s", err) | ||
} | ||
}() | ||
expected := []MetricResult{ | ||
{labels: labelMap{"schemaname": "public", "relname": "foo", "indexrelname": "foo_key"}, value: 100, metricType: dto.MetricType_GAUGE}, | ||
} | ||
convey.Convey("Metrics comparison", t, func() { | ||
for _, expect := range expected { | ||
m := readMetric(<-ch) | ||
convey.So(expect, convey.ShouldResemble, m) | ||
} | ||
}) | ||
if err := mock.ExpectationsWereMet(); err != nil { | ||
t.Errorf("there were unfulfilled exceptions: %s", err) | ||
} | ||
} | ||
|
||
func TestPgIndexSizeCollectorNull(t *testing.T) { | ||
db, mock, err := sqlmock.New() | ||
if err != nil { | ||
t.Fatalf("Error opening a stub db connection: %s", err) | ||
} | ||
defer db.Close() | ||
inst := &instance{db: db} | ||
columns := []string{ | ||
"schemaname", | ||
"relname", | ||
"indexrelname", | ||
"index_size", | ||
} | ||
rows := sqlmock.NewRows(columns). | ||
AddRow(nil, nil, nil, nil) | ||
|
||
mock.ExpectQuery(sanitizeQuery(indexSizeQuery)).WillReturnRows(rows) | ||
|
||
ch := make(chan prometheus.Metric) | ||
go func() { | ||
defer close(ch) | ||
c := PGIndexSizeCollector{} | ||
|
||
if err := c.Update(context.Background(), inst, ch); err != nil { | ||
t.Errorf("Error calling PGIndexSizeCollector.Update: %s", err) | ||
} | ||
}() | ||
expected := []MetricResult{ | ||
{labels: labelMap{"schemaname": "unknown", "relname": "unknown", "indexrelname": "unknown"}, value: 0, metricType: dto.MetricType_GAUGE}, | ||
} | ||
convey.Convey("Metrics comparison", t, func() { | ||
for _, expect := range expected { | ||
m := readMetric(<-ch) | ||
convey.So(expect, convey.ShouldResemble, m) | ||
} | ||
}) | ||
if err := mock.ExpectationsWereMet(); err != nil { | ||
t.Errorf("there were unfulfilled exceptions: %s", err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't rely on 8192 being the block size. It's default, but could be changed. See references:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so we have to select block size and multiply by that