Skip to content

Commit

Permalink
Add additional labels if present (flyteorg#63)
Browse files Browse the repository at this point in the history
# TL;DR
Some of the labeled metrics forgot to add the additional labels.

## Type
 - [x] Bug Fix
 - [ ] Feature
 - [ ] Plugin

## Are all requirements met?

 - [x] Code completed
 - [x] Smoke tested
 - [x] Unit tests added
 - [x] Code documentation added
 - [x] Any pending items have an associated Issue

## Complete description
In all the places where we call GetMetricWith, we now pass the additional labels.

## Tracking Issue
NA

## Follow-up issue
NA
  • Loading branch information
wild-endeavor authored May 28, 2020
1 parent 176da91 commit c811acb
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 7 deletions.
2 changes: 1 addition & 1 deletion promutils/labeled/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Counter struct {
// Inc increments the counter by 1. Use Add to increment it by arbitrary non-negative values. The data point will be
// labeled with values from context. See labeled.SetMetricsKeys for information about to configure that.
func (c Counter) Inc(ctx context.Context) {
counter, err := c.CounterVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
counter, err := c.CounterVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, c.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand Down
10 changes: 5 additions & 5 deletions promutils/labeled/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type Gauge struct {
// Inc increments the gauge by 1. Use Add to increment by arbitrary values. The data point will be
// labeled with values from context. See labeled.SetMetricsKeys for information about to configure that.
func (g Gauge) Inc(ctx context.Context) {
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, g.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand All @@ -33,7 +33,7 @@ func (g Gauge) Inc(ctx context.Context) {
// Add adds the given value to the Gauge. (The value can be negative, resulting in a decrease of the Gauge.)
// The data point will be labeled with values from context. See labeled.SetMetricsKeys for information about to configure that.
func (g Gauge) Add(ctx context.Context, v float64) {
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, g.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand All @@ -47,7 +47,7 @@ func (g Gauge) Add(ctx context.Context, v float64) {
// Set sets the Gauge to an arbitrary value.
// The data point will be labeled with values from context. See labeled.SetMetricsKeys for information about to configure that.
func (g Gauge) Set(ctx context.Context, v float64) {
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, g.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand All @@ -61,7 +61,7 @@ func (g Gauge) Set(ctx context.Context, v float64) {
// Dec decrements the level by 1. Use Sub to decrement by arbitrary values. The data point will be
// labeled with values from context. See labeled.SetMetricsKeys for information about to configure that.
func (g Gauge) Dec(ctx context.Context) {
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, g.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand All @@ -75,7 +75,7 @@ func (g Gauge) Dec(ctx context.Context) {
// Sub adds the given value to the Gauge. The value can be negative, resulting in an increase of the Gauge.
// The data point will be labeled with values from context. See labeled.SetMetricsKeys for information about to configure that.
func (g Gauge) Sub(ctx context.Context, v float64) {
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
gauge, err := g.GaugeVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, g.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand Down
59 changes: 59 additions & 0 deletions promutils/labeled/gauge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,62 @@ func TestLabeledGauge(t *testing.T) {

g.SetToCurrentTime(ctx)
}

func TestWithAdditionalLabels(t *testing.T) {
UnsetMetricKeys()
assert.NotPanics(t, func() {
SetMetricKeys(contextutils.ProjectKey, contextutils.DomainKey, contextutils.WorkflowIDKey, contextutils.TaskIDKey, contextutils.LaunchPlanIDKey)
})

scope := promutils.NewScope("testscope")
ctx := context.Background()
ctx = contextutils.WithProjectDomain(ctx, "flyte", "dev")
g := NewGauge("unittestlabeled", "some desc", scope, AdditionalLabelsOption{Labels: []string{"bearing"}})
assert.NotNil(t, g)

const header = `
# HELP testscope:unittestlabeled some desc
# TYPE testscope:unittestlabeled gauge
`

g.Inc(ctx)
var expected = `
testscope:unittestlabeled{bearing="", domain="dev",lp="",project="flyte",task="",wf=""} 1
`
err := testutil.CollectAndCompare(g.GaugeVec, strings.NewReader(header+expected))
assert.NoError(t, err)

bearingKey := contextutils.Key("bearing")
ctx = context.WithValue(ctx, bearingKey, "123")
g.Set(ctx, 42)
expected = `
testscope:unittestlabeled{bearing="", domain="dev",lp="",project="flyte",task="",wf=""} 1
testscope:unittestlabeled{bearing="123", domain="dev",lp="",project="flyte",task="",wf=""} 42
`
err = testutil.CollectAndCompare(g.GaugeVec, strings.NewReader(header+expected))
assert.NoError(t, err)

g.Add(ctx, 1)
expected = `
testscope:unittestlabeled{bearing="", domain="dev",lp="",project="flyte",task="",wf=""} 1
testscope:unittestlabeled{bearing="123", domain="dev",lp="",project="flyte",task="",wf=""} 43
`
err = testutil.CollectAndCompare(g.GaugeVec, strings.NewReader(header+expected))
assert.NoError(t, err)

g.Dec(ctx)
expected = `
testscope:unittestlabeled{bearing="", domain="dev",lp="",project="flyte",task="",wf=""} 1
testscope:unittestlabeled{bearing="123", domain="dev",lp="",project="flyte",task="",wf=""} 42
`
err = testutil.CollectAndCompare(g.GaugeVec, strings.NewReader(header+expected))
assert.NoError(t, err)

g.Sub(ctx, 42)
expected = `
testscope:unittestlabeled{bearing="", domain="dev",lp="",project="flyte",task="",wf=""} 1
testscope:unittestlabeled{bearing="123", domain="dev",lp="",project="flyte",task="",wf=""} 0
`
err = testutil.CollectAndCompare(g.GaugeVec, strings.NewReader(header+expected))
assert.NoError(t, err)
}
2 changes: 1 addition & 1 deletion promutils/labeled/stopwatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type StopWatch struct {
// ....
// }
func (c StopWatch) Start(ctx context.Context) Timer {
w, err := c.StopWatchVec.GetMetricWith(contextutils.Values(ctx, metricKeys...))
w, err := c.StopWatchVec.GetMetricWith(contextutils.Values(ctx, append(metricKeys, c.additionalLabels...)...))
if err != nil {
panic(err.Error())
}
Expand Down

0 comments on commit c811acb

Please sign in to comment.