Skip to content

Commit

Permalink
metrics-generator: truncate label names and values exceeding a config…
Browse files Browse the repository at this point in the history
…urable length (#1897)
  • Loading branch information
kvrhdn authored Nov 17, 2022
1 parent eac504a commit ee97e3f
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 48 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Old config will still work but will be removed in a future release. [#1735](http
* [ENHANCEMENT] Add cli command an existing file to tempodb's current parquet schema. [#1706](https:/grafana/tempo/pull/1707) (@joe-elliott)
* [ENHANCEMENT] Add query parameter to search API for traceQL queries [#1729](https:/grafana/tempo/pull/1729) (@kvrhdn)
* [ENHANCEMENT] metrics-generator: filter out older spans before metrics are aggregated [#1612](https:/grafana/tempo/pull/1612) (@ie-pham)
* * [ENHANCEMENT] Add hedging to trace by ID lookups created by the frontend. [#1735](https:/grafana/tempo/pull/1735) (@mapno)
* [ENHANCEMENT] Add hedging to trace by ID lookups created by the frontend. [#1735](https:/grafana/tempo/pull/1735) (@mapno)
New config options and defaults:
```
query_frontend:
Expand All @@ -88,6 +88,7 @@ Internal types are updated to use `scope` instead of `instrumentation_library`.
* [ENHANCEMENT] metrics-generator: extract `status_message` field from spans [#1786](https:/grafana/tempo/pull/1786), [#1794](https:/grafana/tempo/pull/1794) (@stoewer)
* [ENHANCEMENT] metrics-generator: handle collisions between user defined and default dimensions [#1794](https:/grafana/tempo/pull/1794) (@stoewer)
* [ENHANCEMENT] distributor: Log span names when `distributor.log_received_spans.include_all_attributes` is on [#1790](https:/grafana/tempo/pull/1790) (@suraciii)
* [ENHANCEMENT] metrics-generator: truncate label names and values exceeding a configurable length [#1897](https:/grafana/tempo/pull/1897) (@kvrhdn)
* [BUGFIX] Honor caching and buffering settings when finding traces by id [#1697](https:/grafana/tempo/pull/1697) (@joe-elliott)
* [BUGFIX] Correctly propagate errors from the iterator layer up through the queriers [#1723](https:/grafana/tempo/pull/1723) (@joe-elliott)
* [BUGFIX] Make multitenancy work with HTTP [#1781](https:/grafana/tempo/pull/1781) (@gouthamve)
Expand Down
6 changes: 6 additions & 0 deletions docs/tempo/website/configuration/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ metrics_generator:
# A list of labels that will be added to all generated metrics.
[external_labels: <map>]
# The maximum length of label names. Label names exceeding this limit will be truncated.
[max_label_name_length: <int> | default = 1024]
# The maximum length of label values. Label values exceeding this limit will be truncated.
[max_label_value_length: <int> | default = 2048]
# Storage and remote write configuration
storage:
Expand Down
8 changes: 5 additions & 3 deletions modules/generator/processor/servicegraphs/servicegraphs.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ func (t tooManySpansError) Error() string {
type Processor struct {
Cfg Config

store store.Store
registry registry.Registry
store store.Store

closeCh chan struct{}

Expand All @@ -82,7 +83,8 @@ func New(cfg Config, tenant string, registry registry.Registry, logger log.Logge
}

p := &Processor{
Cfg: cfg,
Cfg: cfg,
registry: registry,

closeCh: make(chan struct{}, 1),

Expand Down Expand Up @@ -241,7 +243,7 @@ func (p *Processor) onComplete(e *store.Edge) {
labelValues = append(labelValues, e.Dimensions[dimension])
}

registryLabelValues := registry.NewLabelValues(labelValues)
registryLabelValues := p.registry.NewLabelValues(labelValues)

p.serviceGraphRequestTotal.Inc(registryLabelValues, 1)
if e.Failed {
Expand Down
5 changes: 4 additions & 1 deletion modules/generator/processor/spanmetrics/spanmetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ var (
type Processor struct {
Cfg Config

registry registry.Registry

spanMetricsCallsTotal registry.Counter
spanMetricsDurationSeconds registry.Histogram
spanMetricsSizeTotal registry.Counter
Expand All @@ -46,6 +48,7 @@ func New(cfg Config, registry registry.Registry) gen.Processor {

return &Processor{
Cfg: cfg,
registry: registry,
spanMetricsCallsTotal: registry.NewCounter(metricCallsTotal, labels),
spanMetricsDurationSeconds: registry.NewHistogram(metricDurationSeconds, labels, cfg.HistogramBuckets),
spanMetricsSizeTotal: registry.NewCounter(metricSizeTotal, labels),
Expand Down Expand Up @@ -98,7 +101,7 @@ func (p *Processor) aggregateMetricsForSpan(svcName string, rs *v1.Resource, spa
labelValues = append(labelValues, value)
}

registryLabelValues := registry.NewLabelValues(labelValues)
registryLabelValues := p.registry.NewLabelValues(labelValues)

p.spanMetricsCallsTotal.Inc(registryLabelValues, 1)
p.spanMetricsSizeTotal.Inc(registryLabelValues, float64(span.Size()))
Expand Down
10 changes: 10 additions & 0 deletions modules/generator/registry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,20 @@ type Config struct {

// ExternalLabels are added to any time series generated by this instance.
ExternalLabels map[string]string `yaml:"external_labels,omitempty"`

// MaxLabelNameLength configures the maximum length of label names. Label names exceeding
// this limit will be truncated.
MaxLabelNameLength int `yaml:"max_label_name_length"`

// MaxLabelValueLength configures the maximum length of label values. Label values exceeding
// this limit will be truncated.
MaxLabelValueLength int `yaml:"max_label_value_length"`
}

// RegisterFlagsAndApplyDefaults registers the flags.
func (cfg *Config) RegisterFlagsAndApplyDefaults(prefix string, f *flag.FlagSet) {
cfg.CollectionInterval = 15 * time.Second
cfg.StaleDuration = 15 * time.Minute
cfg.MaxLabelNameLength = 1024
cfg.MaxLabelValueLength = 2048
}
36 changes: 18 additions & 18 deletions modules/generator/registry/counter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ func Test_counter(t *testing.T) {

c := newCounter("my_counter", []string{"label"}, onAdd, nil)

c.Inc(NewLabelValues([]string{"value-1"}), 1.0)
c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-1"}), 1.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)

assert.Equal(t, 2, seriesAdded)

Expand All @@ -31,8 +31,8 @@ func Test_counter(t *testing.T) {
}
collectMetricAndAssert(t, c, collectionTimeMs, nil, 2, expectedSamples, nil)

c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(NewLabelValues([]string{"value-3"}), 3.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-3"}), 3.0)

assert.Equal(t, 3, seriesAdded)

Expand All @@ -52,7 +52,7 @@ func Test_counter_invalidLabelValues(t *testing.T) {
c.Inc(nil, 1.0)
})
assert.Panics(t, func() {
c.Inc(NewLabelValues([]string{"value-1", "value-2"}), 1.0)
c.Inc(newLabelValues([]string{"value-1", "value-2"}), 1.0)
})
}

Expand All @@ -68,8 +68,8 @@ func Test_counter_cantAdd(t *testing.T) {
// allow adding new series
canAdd = true

c.Inc(NewLabelValues([]string{"value-1"}), 1.0)
c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-1"}), 1.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)

collectionTimeMs := time.Now().UnixMilli()
expectedSamples := []sample{
Expand All @@ -81,8 +81,8 @@ func Test_counter_cantAdd(t *testing.T) {
// block new series - existing series can still be updated
canAdd = false

c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(NewLabelValues([]string{"value-3"}), 3.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-3"}), 3.0)

collectionTimeMs = time.Now().UnixMilli()
expectedSamples = []sample{
Expand All @@ -102,8 +102,8 @@ func Test_counter_removeStaleSeries(t *testing.T) {
c := newCounter("my_counter", []string{"label"}, nil, onRemove)

timeMs := time.Now().UnixMilli()
c.Inc(NewLabelValues([]string{"value-1"}), 1.0)
c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-1"}), 1.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)

c.removeStaleSeries(timeMs)

Expand All @@ -120,7 +120,7 @@ func Test_counter_removeStaleSeries(t *testing.T) {
timeMs = time.Now().UnixMilli()

// update value-2 series
c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)

c.removeStaleSeries(timeMs)

Expand All @@ -136,8 +136,8 @@ func Test_counter_removeStaleSeries(t *testing.T) {
func Test_counter_externalLabels(t *testing.T) {
c := newCounter("my_counter", []string{"label"}, nil, nil)

c.Inc(NewLabelValues([]string{"value-1"}), 1.0)
c.Inc(NewLabelValues([]string{"value-2"}), 2.0)
c.Inc(newLabelValues([]string{"value-1"}), 1.0)
c.Inc(newLabelValues([]string{"value-2"}), 2.0)

collectionTimeMs := time.Now().UnixMilli()
expectedSamples := []sample{
Expand Down Expand Up @@ -165,8 +165,8 @@ func Test_counter_concurrencyDataRace(t *testing.T) {

for i := 0; i < 4; i++ {
go accessor(func() {
c.Inc(NewLabelValues([]string{"value-1"}), 1.0)
c.Inc(NewLabelValues([]string{"value-2"}), 1.0)
c.Inc(newLabelValues([]string{"value-1"}), 1.0)
c.Inc(newLabelValues([]string{"value-2"}), 1.0)
})
}

Expand All @@ -177,7 +177,7 @@ func Test_counter_concurrencyDataRace(t *testing.T) {
for i := range s {
s[i] = letters[rand.Intn(len(letters))]
}
c.Inc(NewLabelValues([]string{string(s)}), 1.0)
c.Inc(newLabelValues([]string{string(s)}), 1.0)
})

go accessor(func() {
Expand Down Expand Up @@ -210,7 +210,7 @@ func Test_counter_concurrencyCorrectness(t *testing.T) {
case <-end:
return
default:
c.Inc(NewLabelValues([]string{"value-1"}), 1.0)
c.Inc(newLabelValues([]string{"value-1"}), 1.0)
totalCount.Inc()
}
}
Expand Down
36 changes: 18 additions & 18 deletions modules/generator/registry/histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func Test_histogram(t *testing.T) {

h := newHistogram("my_histogram", []string{"label"}, []float64{1.0, 2.0}, onAdd, nil)

h.ObserveWithExemplar(NewLabelValues([]string{"value-1"}), 1.0, "trace-1")
h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 1.5, "trace-2")
h.ObserveWithExemplar(newLabelValues([]string{"value-1"}), 1.0, "trace-1")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 1.5, "trace-2")

assert.Equal(t, 2, seriesAdded)

Expand Down Expand Up @@ -53,8 +53,8 @@ func Test_histogram(t *testing.T) {
}
collectMetricAndAssert(t, h, collectionTimeMs, nil, 10, expectedSamples, expectedExemplars)

h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 2.5, "trace-2.2")
h.ObserveWithExemplar(NewLabelValues([]string{"value-3"}), 3.0, "trace-3")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 2.5, "trace-2.2")
h.ObserveWithExemplar(newLabelValues([]string{"value-3"}), 3.0, "trace-3")

assert.Equal(t, 3, seriesAdded)

Expand Down Expand Up @@ -98,7 +98,7 @@ func Test_histogram_invalidLabelValues(t *testing.T) {
h.ObserveWithExemplar(nil, 1.0, "")
})
assert.Panics(t, func() {
h.ObserveWithExemplar(NewLabelValues([]string{"value-1", "value-2"}), 1.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-1", "value-2"}), 1.0, "")
})
}

Expand All @@ -114,8 +114,8 @@ func Test_histogram_cantAdd(t *testing.T) {
// allow adding new series
canAdd = true

h.ObserveWithExemplar(NewLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 1.5, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 1.5, "")

collectionTimeMs := time.Now().UnixMilli()
expectedSamples := []sample{
Expand All @@ -135,8 +135,8 @@ func Test_histogram_cantAdd(t *testing.T) {
// block new series - existing series can still be updated
canAdd = false

h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 2.5, "")
h.ObserveWithExemplar(NewLabelValues([]string{"value-3"}), 3.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 2.5, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-3"}), 3.0, "")

collectionTimeMs = time.Now().UnixMilli()
expectedSamples = []sample{
Expand Down Expand Up @@ -164,8 +164,8 @@ func Test_histogram_removeStaleSeries(t *testing.T) {
h := newHistogram("my_histogram", []string{"label"}, []float64{1.0, 2.0}, nil, onRemove)

timeMs := time.Now().UnixMilli()
h.ObserveWithExemplar(NewLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 1.5, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 1.5, "")

h.removeStaleSeries(timeMs)

Expand All @@ -190,7 +190,7 @@ func Test_histogram_removeStaleSeries(t *testing.T) {
timeMs = time.Now().UnixMilli()

// update value-2 series
h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 2.5, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 2.5, "")

h.removeStaleSeries(timeMs)

Expand All @@ -210,8 +210,8 @@ func Test_histogram_removeStaleSeries(t *testing.T) {
func Test_histogram_externalLabels(t *testing.T) {
h := newHistogram("my_histogram", []string{"label"}, []float64{1.0, 2.0}, nil, nil)

h.ObserveWithExemplar(NewLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 1.5, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 1.5, "")

collectionTimeMs := time.Now().UnixMilli()
expectedSamples := []sample{
Expand Down Expand Up @@ -247,8 +247,8 @@ func Test_histogram_concurrencyDataRace(t *testing.T) {

for i := 0; i < 4; i++ {
go accessor(func() {
h.ObserveWithExemplar(NewLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(NewLabelValues([]string{"value-2"}), 1.5, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-1"}), 1.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-2"}), 1.5, "")
})
}

Expand All @@ -259,7 +259,7 @@ func Test_histogram_concurrencyDataRace(t *testing.T) {
for i := range s {
s[i] = letters[rand.Intn(len(letters))]
}
h.ObserveWithExemplar(NewLabelValues([]string{string(s)}), 1.0, "")
h.ObserveWithExemplar(newLabelValues([]string{string(s)}), 1.0, "")
})

go accessor(func() {
Expand Down Expand Up @@ -292,7 +292,7 @@ func Test_histogram_concurrencyCorrectness(t *testing.T) {
case <-end:
return
default:
h.ObserveWithExemplar(NewLabelValues([]string{"value-1"}), 2.0, "")
h.ObserveWithExemplar(newLabelValues([]string{"value-1"}), 2.0, "")
totalCount.Inc()
}
}
Expand Down
8 changes: 7 additions & 1 deletion modules/generator/registry/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package registry

// Registry is a metrics store.
type Registry interface {
NewLabelValues(values []string) *LabelValues
NewCounter(name string, labels []string) Counter
NewHistogram(name string, labels []string, buckets []float64) Histogram
}
Expand All @@ -27,13 +28,18 @@ type LabelValues struct {
hash uint64
}

func NewLabelValues(values []string) *LabelValues {
func newLabelValues(values []string) *LabelValues {
return &LabelValues{
values: values,
hash: 0,
}
}

func newLabelValuesWithMax(values []string, maxLengthLabelValue int) *LabelValues {
truncateLength(values, maxLengthLabelValue)
return newLabelValues(values)
}

func (l *LabelValues) getValues() []string {
if l == nil {
return nil
Expand Down
19 changes: 19 additions & 0 deletions modules/generator/registry/interface_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package registry

import (
"testing"

"github.com/stretchr/testify/assert"
)

func Test_newLabelValuesWithMax(t *testing.T) {
labelValues := newLabelValuesWithMax([]string{"abc", "abcdef"}, 5)

assert.Equal(t, []string{"abc", "abcde"}, labelValues.getValues())
}

func Test_newLabelValuesWithMax_zeroLength(t *testing.T) {
labelValues := newLabelValuesWithMax([]string{"abc", "abcdef"}, 0)

assert.Equal(t, []string{"abc", "abcdef"}, labelValues.getValues())
}
Loading

0 comments on commit ee97e3f

Please sign in to comment.