PMM-4474 Add extra labels. #32

AlekSi · 2019-12-03T06:46:53Z

No description provided.

golangcibot · 2019-12-03T06:48:13Z

enhanced/metrics.go


 // makePrometheusMetrics returns all Prometheus metrics for given osMetrics.
-func (m *osMetrics) makePrometheusMetrics(region string) []prometheus.Metric {
+func (m *osMetrics) makePrometheusMetrics(region string, labels map[string]string) []prometheus.Metric {


Function 'makePrometheusMetrics' has too many statements (44 > 40) (from funlen)

golangcibot · 2019-12-03T06:48:14Z

enhanced/metrics.go

 		"instance": m.InstanceID,
 		"region":   region,
 	}
+	for n, v := range labels {


ranges should only be cuddled with assignments used in the iteration (from wsl)

golangcibot · 2019-12-03T06:48:14Z

config/config.go

+	AWSSecretKey string            `yaml:"aws_secret_key"` // may be empty
+	Labels       map[string]string `yaml:"labels"`         // may be empty
+
+	// TODO Type InstanceType `yaml:"type"` // may be empty for old pmm-managed


config/config.go:27: Line contains TODO/BUG/FIXME: "TODO Type InstanceType yaml:"type" // ..." (from godox)

golangcibot · 2019-12-03T07:27:04Z

basic/collector_test.go

 )

-func getExporter(t *testing.T) *Exporter {
+func getCollector(t *testing.T) *Collector {


func getCollector is unused (from unused)

golangcibot · 2019-12-03T07:27:04Z

basic/collector.go

 }

-type Exporter struct {
+type Collector struct {


exported type Collector should have comment or be unexported (from golint)

golangcibot · 2019-12-03T07:27:05Z

basic/collector.go

 }

-func (e *Exporter) Collect(ch chan<- prometheus.Metric) {
+func (e *Collector) Describe(ch chan<- *prometheus.Desc) {


exported method Collector.Describe should have comment or be unexported (from golint)

golangcibot · 2019-12-03T07:27:05Z

basic/collector.go

+	// unchecked collector
+}
+
+func (e *Collector) Collect(ch chan<- prometheus.Metric) {


exported method Collector.Collect should have comment or be unexported (from golint)

golangcibot · 2019-12-03T07:27:05Z

basic/scraper.go

 }

-func NewScraper(instance *config.Instance, exporter *Exporter, ch chan<- prometheus.Metric) *Scraper {
+func NewScraper(instance *config.Instance, collector *Collector, ch chan<- prometheus.Metric) *Scraper {


exported function NewScraper should have comment or be unexported (from golint)

golangcibot · 2019-12-03T07:27:06Z

basic/collector.go

-	wg.Add(len(instances))
-	for _, instance := range instances {
+	wg.Add(len(e.config.Instances))
+	for _, instance := range e.config.Instances {


ranges should only be cuddled with assignments used in the iteration (from wsl)

golangcibot · 2019-12-03T07:27:06Z

basic/scraper.go

+		"region":   instance.Region,
+		"instance": instance.Instance,
+	}
+	for n, v := range instance.Labels {


ranges should only be cuddled with assignments used in the iteration (from wsl)

golangcibot · 2019-12-03T07:27:06Z

basic/scraper.go

+	wg.Add(len(s.collector.metrics))
+	for _, metric := range s.collector.metrics {
+		metric := metric
+		go func() {


go statements can only invoke functions assigned on line above (from wsl)

golangcibot · 2019-12-03T07:27:06Z

basic/scraper.go

 	// Get the metric.
 	v := aws.Float64Value(dp.Average)
-	switch metric.Name {
+	switch metric.cwName {


switch statements should only be cuddled with variables switched (from wsl)

BupycHuk

Tests are failing.
Let's add labels to readme.

BupycHuk · 2019-12-03T08:51:11Z

basic/scraper.go

-	labels := []string{
-		instance.Instance,
-		instance.Region,
+	constLabels := prometheus.Labels{


constLabels? What's the difference?

Constant labels are part of the metric's description; variable labels are not. The distinction is important for checked Collectors, but we switched to unchecked in this PR. Some more info: https://godoc.org/github.com/prometheus/client_golang/prometheus#hdr-Custom_Collectors_and_constant_Metrics

AlekSi · 2019-12-03T09:04:14Z

Tests are failing.

Due to https://jira.percona.com/browse/PMM-1772. Nothing we can do right now.

BupycHuk

Let's update readme and merge.

golangcibot · 2019-12-04T11:58:48Z

basic/scraper.go

-				s.exporter.l.With("metric", metric.Name).Error(err)
-			}
+	wg.Add(len(s.collector.metrics))
+	for _, metric := range s.collector.metrics {


ranges should only be cuddled with assignments used in the iteration (from wsl)

AlekSi added 4 commits December 3, 2019 08:22

PMM-4474 Update deps.

6bcf941

PMM-4474 Add extra labels to enhanced metrics.

2df0556

PMM-4474 Remove old generator.

da49575

PMM-4474 Add extra labels example.

2f0e384

golangcibot reviewed Dec 3, 2019

View reviewed changes

PMM-4474 Add labels to basic metrics.

7f90000

golangcibot reviewed Dec 3, 2019

View reviewed changes

AlekSi marked this pull request as ready for review December 3, 2019 07:27

AlekSi requested a review from BupycHuk December 3, 2019 07:27

AlekSi self-assigned this Dec 3, 2019

BupycHuk requested changes Dec 3, 2019

View reviewed changes

BupycHuk previously approved these changes Dec 3, 2019

View reviewed changes

AlekSi mentioned this pull request Dec 3, 2019

PMM-4949 PMM-4897 RDS. Percona-Lab/pmm-submodules#552

Closed

7 tasks

PMM-4474 Update README.

d357014

AlekSi dismissed BupycHuk’s stale review via d357014 December 4, 2019 11:57

golangcibot reviewed Dec 4, 2019

View reviewed changes

AlekSi merged commit 80355c9 into master Dec 4, 2019

AlekSi deleted the PMM-4474-extra-labels branch December 4, 2019 12:05

Uh oh!

PMM-4474 Add extra labels. #32

PMM-4474 Add extra labels. #32

Uh oh!

Conversation

AlekSi commented Dec 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BupycHuk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekSi commented Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BupycHuk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AlekSi commented Dec 3, 2019 •

edited

Loading