Skip to content

Commit

Permalink
internal/metrics: remove deprecated metrics and enable promlint (#3414)
Browse files Browse the repository at this point in the history 
Prometheus gauges with correct names were added in v1.13.0.
This PR removes the deprecated metrics.

Fixes: #3337
Signed-off-by: Amey Bhide <amey15@gmail.com>
  • Loading branch information
@abhide
abhide authored Mar 1, 2021
1 parent 94b4a32 commit 50f17a3
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 228 deletions.
8 changes: 3 additions & 5 deletions hack/generate-metrics-doc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/projectcontour/contour/internal/metrics"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil/promlint"
dto "github.com/prometheus/client_model/go"
)

Expand Down Expand Up @@ -52,7 +53,6 @@ func typeof(mf *dto.MetricFamily) string {
}
}

/*
// Executes promlint for metrics static analysis
func runPromlint(family []*dto.MetricFamily) {
linter := promlint.NewWithMetricFamilies(family)
Expand All @@ -66,7 +66,7 @@ func runPromlint(family []*dto.MetricFamily) {
}

os.Exit(len(problems))
}*/
}

func main() {
registry := prometheus.NewRegistry()
Expand Down Expand Up @@ -95,7 +95,5 @@ func main() {
f.Close()
}

//FIXME: Enable promlint check
// https://github.com/projectcontour/contour/issues/3337
// runPromlint(family)
runPromlint(family)
}
73 changes: 0 additions & 73 deletions internal/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package metrics

import (
"fmt"
"net/http"
"time"

Expand All @@ -28,12 +27,6 @@ import (
type Metrics struct {
buildInfoGauge *prometheus.GaugeVec

deprecatedProxyTotalGauge *prometheus.GaugeVec
deprecatedProxyRootTotalGauge *prometheus.GaugeVec
deprecatedProxyInvalidGauge *prometheus.GaugeVec
deprecatedProxyValidGauge *prometheus.GaugeVec
deprecatedProxyOrphanedGauge *prometheus.GaugeVec

proxyTotalGauge *prometheus.GaugeVec
proxyRootTotalGauge *prometheus.GaugeVec
proxyInvalidGauge *prometheus.GaugeVec
Expand Down Expand Up @@ -66,12 +59,6 @@ type Meta struct {
const (
BuildInfoGauge = "contour_build_info"

DeprecatedHTTPProxyTotalGauge = "contour_httpproxy_total"
DeprecatedHTTPProxyRootTotalGauge = "contour_httpproxy_root_total"
DeprecatedHTTPProxyInvalidGauge = "contour_httpproxy_invalid_total"
DeprecatedHTTPProxyValidGauge = "contour_httpproxy_valid_total"
DeprecatedHTTPProxyOrphanedGauge = "contour_httpproxy_orphaned_total"

HTTPProxyTotalGauge = "contour_httpproxy"
HTTPProxyRootTotalGauge = "contour_httpproxy_root"
HTTPProxyInvalidGauge = "contour_httpproxy_invalid"
Expand Down Expand Up @@ -100,51 +87,6 @@ func NewMetrics(registry *prometheus.Registry) *Metrics {
[]string{"branch", "revision", "version"},
),
proxyMetricCache: &RouteMetric{},
deprecatedProxyTotalGauge: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Name: DeprecatedHTTPProxyTotalGauge,
Help: fmt.Sprintf(
"(Deprecated): Total number of HTTPProxies that exist regardless of status. Use %s instead",
HTTPProxyTotalGauge),
},
[]string{"namespace"},
),
deprecatedProxyRootTotalGauge: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Name: DeprecatedHTTPProxyRootTotalGauge,
Help: fmt.Sprintf(
"(Deprecated): Total number of root HTTPProxies. Note there will only be a single root HTTPProxy per vhost. Use %s instead",
HTTPProxyRootTotalGauge),
},
[]string{"namespace"},
),
deprecatedProxyInvalidGauge: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Name: DeprecatedHTTPProxyInvalidGauge,
Help: fmt.Sprintf(
"(Deprecated): Total number of invalid HTTPProxies. Use %s instead.",
HTTPProxyInvalidGauge),
},
[]string{"namespace", "vhost"},
),
deprecatedProxyValidGauge: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Name: DeprecatedHTTPProxyValidGauge,
Help: fmt.Sprintf(
"(Deprecated): Total number of valid HTTPProxies. Use %s instead",
HTTPProxyValidGauge),
},
[]string{"namespace", "vhost"},
),
deprecatedProxyOrphanedGauge: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Name: DeprecatedHTTPProxyOrphanedGauge,
Help: fmt.Sprintf(
"(Deprecated): Total number of orphaned HTTPProxies which have no root delegating to them. Use %s instead",
HTTPProxyOrphanedGauge),
},
[]string{"namespace"},
),
proxyTotalGauge: prometheus.NewGaugeVec(
prometheus.GaugeOpts{
Name: HTTPProxyTotalGauge,
Expand Down Expand Up @@ -216,15 +158,10 @@ func (m *Metrics) register(registry *prometheus.Registry) {
registry.MustRegister(
m.buildInfoGauge,
m.proxyTotalGauge,
m.deprecatedProxyTotalGauge,
m.proxyRootTotalGauge,
m.deprecatedProxyRootTotalGauge,
m.proxyInvalidGauge,
m.deprecatedProxyInvalidGauge,
m.proxyValidGauge,
m.deprecatedProxyValidGauge,
m.proxyOrphanedGauge,
m.deprecatedProxyOrphanedGauge,
m.dagRebuildGauge,
m.dagRebuildTotal,
m.CacheHandlerOnUpdateSummary,
Expand Down Expand Up @@ -271,50 +208,40 @@ func (m *Metrics) SetHTTPProxyMetric(metrics RouteMetric) {
// Process metrics
for meta, value := range metrics.Total {
m.proxyTotalGauge.WithLabelValues(meta.Namespace).Set(float64(value))
m.deprecatedProxyTotalGauge.WithLabelValues(meta.Namespace).Set(float64(value))
delete(m.proxyMetricCache.Total, meta)
}
for meta, value := range metrics.Invalid {
m.proxyInvalidGauge.WithLabelValues(meta.Namespace, meta.VHost).Set(float64(value))
m.deprecatedProxyInvalidGauge.WithLabelValues(meta.Namespace, meta.VHost).Set(float64(value))
delete(m.proxyMetricCache.Invalid, meta)
}
for meta, value := range metrics.Orphaned {
m.proxyOrphanedGauge.WithLabelValues(meta.Namespace).Set(float64(value))
m.deprecatedProxyOrphanedGauge.WithLabelValues(meta.Namespace).Set(float64(value))
delete(m.proxyMetricCache.Orphaned, meta)
}
for meta, value := range metrics.Valid {
m.proxyValidGauge.WithLabelValues(meta.Namespace, meta.VHost).Set(float64(value))
m.deprecatedProxyValidGauge.WithLabelValues(meta.Namespace, meta.VHost).Set(float64(value))
delete(m.proxyMetricCache.Valid, meta)
}
for meta, value := range metrics.Root {
m.proxyRootTotalGauge.WithLabelValues(meta.Namespace).Set(float64(value))
m.deprecatedProxyRootTotalGauge.WithLabelValues(meta.Namespace).Set(float64(value))
delete(m.proxyMetricCache.Root, meta)
}

// All metrics processed, now remove what's left as they are not needed
for meta := range m.proxyMetricCache.Total {
m.proxyTotalGauge.DeleteLabelValues(meta.Namespace)
m.deprecatedProxyTotalGauge.DeleteLabelValues(meta.Namespace)
}
for meta := range m.proxyMetricCache.Invalid {
m.proxyInvalidGauge.DeleteLabelValues(meta.Namespace, meta.VHost)
m.deprecatedProxyInvalidGauge.DeleteLabelValues(meta.Namespace, meta.VHost)
}
for meta := range m.proxyMetricCache.Orphaned {
m.proxyOrphanedGauge.DeleteLabelValues(meta.Namespace)
m.deprecatedProxyOrphanedGauge.DeleteLabelValues(meta.Namespace)
}
for meta := range m.proxyMetricCache.Valid {
m.proxyValidGauge.DeleteLabelValues(meta.Namespace, meta.VHost)
m.deprecatedProxyValidGauge.DeleteLabelValues(meta.Namespace, meta.VHost)
}
for meta := range m.proxyMetricCache.Root {
m.proxyRootTotalGauge.DeleteLabelValues(meta.Namespace)
m.deprecatedProxyRootTotalGauge.DeleteLabelValues(meta.Namespace)
}

m.proxyMetricCache = &RouteMetric{
Expand Down
115 changes: 0 additions & 115 deletions internal/metrics/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,10 @@ func TestWriteProxyMetric(t *testing.T) {
tests := map[string]struct {
proxyMetrics RouteMetric
total testMetric
deprecatedTotal testMetric
valid testMetric
deprecatedValid testMetric
invalid testMetric
deprecatedInvalid testMetric
orphaned testMetric
deprecatedOrphaned testMetric
root testMetric
deprecatedRoot testMetric
}{
"simple": {
proxyMetrics: RouteMetric{
Expand Down Expand Up @@ -131,29 +126,6 @@ func TestWriteProxyMetric(t *testing.T) {
},
},
},
deprecatedTotal: testMetric{
metric: DeprecatedHTTPProxyTotalGauge,
want: []*io_prometheus_client.Metric{
{
Label: []*io_prometheus_client.LabelPair{{
Name: func() *string { i := "namespace"; return &i }(),
Value: func() *string { i := "foons"; return &i }(),
}},
Gauge: &io_prometheus_client.Gauge{
Value: func() *float64 { i := float64(3); return &i }(),
},
},
{
Label: []*io_prometheus_client.LabelPair{{
Name: func() *string { i := "namespace"; return &i }(),
Value: func() *string { i := "testns"; return &i }(),
}},
Gauge: &io_prometheus_client.Gauge{
Value: func() *float64 { i := float64(6); return &i }(),
},
},
},
},
orphaned: testMetric{
metric: HTTPProxyOrphanedGauge,
want: []*io_prometheus_client.Metric{
Expand All @@ -168,20 +140,6 @@ func TestWriteProxyMetric(t *testing.T) {
},
},
},
deprecatedOrphaned: testMetric{
metric: DeprecatedHTTPProxyOrphanedGauge,
want: []*io_prometheus_client.Metric{
{
Label: []*io_prometheus_client.LabelPair{{
Name: func() *string { i := "namespace"; return &i }(),
Value: func() *string { i := "testns"; return &i }(),
}},
Gauge: &io_prometheus_client.Gauge{
Value: func() *float64 { i := float64(1); return &i }(),
},
},
},
},
valid: testMetric{
metric: HTTPProxyValidGauge,
want: []*io_prometheus_client.Metric{
Expand All @@ -199,23 +157,6 @@ func TestWriteProxyMetric(t *testing.T) {
},
},
},
deprecatedValid: testMetric{
metric: DeprecatedHTTPProxyValidGauge,
want: []*io_prometheus_client.Metric{
{
Label: []*io_prometheus_client.LabelPair{{
Name: func() *string { i := "namespace"; return &i }(),
Value: func() *string { i := "testns"; return &i }(),
}, {
Name: func() *string { i := "vhost"; return &i }(),
Value: func() *string { i := "foo.com"; return &i }(),
}},
Gauge: &io_prometheus_client.Gauge{
Value: func() *float64 { i := float64(3); return &i }(),
},
},
},
},
invalid: testMetric{
metric: HTTPProxyInvalidGauge,
want: []*io_prometheus_client.Metric{
Expand All @@ -233,23 +174,6 @@ func TestWriteProxyMetric(t *testing.T) {
},
},
},
deprecatedInvalid: testMetric{
metric: DeprecatedHTTPProxyInvalidGauge,
want: []*io_prometheus_client.Metric{
{
Label: []*io_prometheus_client.LabelPair{{
Name: func() *string { i := "namespace"; return &i }(),
Value: func() *string { i := "testns"; return &i }(),
}, {
Name: func() *string { i := "vhost"; return &i }(),
Value: func() *string { i := "foo.com"; return &i }(),
}},
Gauge: &io_prometheus_client.Gauge{
Value: func() *float64 { i := float64(2); return &i }(),
},
},
},
},
root: testMetric{
metric: HTTPProxyRootTotalGauge,
want: []*io_prometheus_client.Metric{
Expand All @@ -264,20 +188,6 @@ func TestWriteProxyMetric(t *testing.T) {
},
},
},
deprecatedRoot: testMetric{
metric: DeprecatedHTTPProxyRootTotalGauge,
want: []*io_prometheus_client.Metric{
{
Label: []*io_prometheus_client.LabelPair{{
Name: func() *string { i := "namespace"; return &i }(),
Value: func() *string { i := "testns"; return &i }(),
}},
Gauge: &io_prometheus_client.Gauge{
Value: func() *float64 { i := float64(4); return &i }(),
},
},
},
},
},
}

Expand All @@ -298,55 +208,30 @@ func TestWriteProxyMetric(t *testing.T) {
}

gotTotal := []*io_prometheus_client.Metric{}
gotDeprecatedTotal := []*io_prometheus_client.Metric{}
gotValid := []*io_prometheus_client.Metric{}
gotDeprecatedValid := []*io_prometheus_client.Metric{}
gotInvalid := []*io_prometheus_client.Metric{}
gotDeprecatedInvalid := []*io_prometheus_client.Metric{}
gotOrphaned := []*io_prometheus_client.Metric{}
gotDeprecatedOrphaned := []*io_prometheus_client.Metric{}
gotRoot := []*io_prometheus_client.Metric{}
gotDeprecatedRoot := []*io_prometheus_client.Metric{}
for _, mf := range gathering {
switch mf.GetName() {
case tc.total.metric:
gotTotal = mf.Metric
case tc.deprecatedTotal.metric:
gotDeprecatedTotal = mf.Metric
case tc.valid.metric:
gotValid = mf.Metric
case tc.deprecatedValid.metric:
gotDeprecatedValid = mf.Metric
case tc.invalid.metric:
gotInvalid = mf.Metric
case tc.deprecatedInvalid.metric:
gotDeprecatedInvalid = mf.Metric
case tc.orphaned.metric:
gotOrphaned = mf.Metric
case tc.deprecatedOrphaned.metric:
gotDeprecatedOrphaned = mf.Metric
case tc.root.metric:
gotRoot = mf.Metric
case tc.deprecatedRoot.metric:
gotDeprecatedRoot = mf.Metric
}
}

assert.Equal(t, tc.total.want, gotTotal)
assert.Equal(t, tc.deprecatedTotal.want, gotDeprecatedTotal)
assert.Equal(t, gotTotal, gotDeprecatedTotal)
assert.Equal(t, tc.valid.want, gotValid)
assert.Equal(t, tc.deprecatedValid.want, gotDeprecatedValid)
assert.Equal(t, gotValid, gotDeprecatedValid)
assert.Equal(t, tc.invalid.want, gotInvalid)
assert.Equal(t, tc.deprecatedInvalid.want, gotDeprecatedInvalid)
assert.Equal(t, gotInvalid, gotDeprecatedInvalid)
assert.Equal(t, tc.orphaned.want, gotOrphaned)
assert.Equal(t, tc.deprecatedOrphaned.want, gotDeprecatedOrphaned)
assert.Equal(t, gotOrphaned, gotDeprecatedOrphaned)
assert.Equal(t, tc.root.want, gotRoot)
assert.Equal(t, tc.deprecatedRoot.want, gotDeprecatedRoot)
assert.Equal(t, gotRoot, gotDeprecatedRoot)
})
}
}
Expand Down
7 changes: 0 additions & 7 deletions site/_metrics/contour_httpproxy_invalid_total.md
View file

This file was deleted.

7 changes: 0 additions & 7 deletions site/_metrics/contour_httpproxy_orphaned_total.md
View file

This file was deleted.

Loading

0 comments on commit 50f17a3

Please sign in to comment.