Skip to content

Commit

Permalink
prometheus: Refactor getAttrs (#5937)
Browse files Browse the repository at this point in the history 
Addresses
#5932 (comment)

I feel that `getAttr` was doing too much and having:

```go
keys, values := getAttrs(dp.Attributes)
keys = append(keys, kv.keys...)
values = append(values, kv.vals...)
// ...
keys, values := getAttrs(*res.Set())
```

is more readable than:
```go
keys, values := getAttrs(dp.Attributes, ks, vs, resourceKV)
// ...
keys, values := getAttrs(*res.Set(), [2]string{}, [2]string{}, keyVals{})
```

Benchmarks results just in case to minimize the possibility of
accidental introduction of a performance overhead:

```
$ benchstat old.txt new.txt 
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/exporters/prometheus
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
                │   old.txt    │               new.txt               │
                │    sec/op    │    sec/op     vs base               │
Collect1-16       29.77µ ± 11%   30.33µ ± 10%       ~ (p=0.529 n=10)
Collect10-16      80.58µ ±  7%   77.93µ ± 15%       ~ (p=0.315 n=10)
Collect100-16     528.5µ ±  7%   511.2µ ±  2%  -3.28% (p=0.015 n=10)
Collect1000-16    3.179m ±  6%   3.344m ± 15%  +5.19% (p=0.003 n=10)
Collect10000-16   31.77m ±  2%   33.14m ±  7%  +4.34% (p=0.004 n=10)
geomean           662.9µ         668.9µ        +0.90%

                │   old.txt    │               new.txt               │
                │     B/op     │     B/op      vs base               │
Collect1-16       36.52Ki ± 0%   36.59Ki ± 0%  +0.17% (p=0.000 n=10)
Collect10-16      64.58Ki ± 0%   64.64Ki ± 0%  +0.09% (p=0.000 n=10)
Collect100-16     349.3Ki ± 0%   349.4Ki ± 0%  +0.03% (p=0.000 n=10)
Collect1000-16    3.163Mi ± 0%   3.163Mi ± 0%       ~ (p=0.247 n=10)
Collect10000-16   31.05Mi ± 0%   31.06Mi ± 0%  +0.02% (p=0.009 n=10)
geomean           610.6Ki        611.0Ki       +0.06%

                │   old.txt   │              new.txt               │
                │  allocs/op  │  allocs/op   vs base               │
Collect1-16        70.00 ± 0%    72.00 ± 0%  +2.86% (p=0.000 n=10)
Collect10-16       396.0 ± 0%    398.0 ± 0%  +0.51% (p=0.000 n=10)
Collect100-16     3.661k ± 0%   3.663k ± 0%  +0.05% (p=0.000 n=10)
Collect1000-16    36.15k ± 0%   36.15k ± 0%  +0.01% (p=0.000 n=10)
Collect10000-16   361.4k ± 0%   361.5k ± 0%  +0.03% (p=0.009 n=10)
geomean           4.212k        4.241k       +0.68%
```
  • Loading branch information
@pellared
pellared authored Nov 4, 2024
1 parent 7fd5942 commit 6e4c922
Showing 1 changed file with 37 additions and 37 deletions.
74 changes: 37 additions & 37 deletions exporters/prometheus/exporter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ const (
scopeInfoMetricName = "otel_scope_info"
scopeInfoDescription = "Instrumentation Scope metadata"

scopeNameLabel = "otel_scope_name"
scopeVersionLabel = "otel_scope_version"

traceIDExemplarKey = "trace_id"
spanIDExemplarKey = "span_id"
)

var (
scopeInfoKeys = [2]string{"otel_scope_name", "otel_scope_version"}

errScopeInvalid = errors.New("invalid scope")
)
var errScopeInvalid = errors.New("invalid scope")

// Exporter is a Prometheus Exporter that embeds the OTel metric.Reader
// interface for easy instantiation with a MeterProvider.
Expand Down Expand Up @@ -187,7 +186,11 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
}

for _, scopeMetrics := range metrics.ScopeMetrics {
var keys, values [2]string
n := len(c.resourceKeyVals.keys) + 2 // resource attrs + scope name + scope version
kv := keyVals{
keys: make([]string, 0, n),
vals: make([]string, 0, n),
}

if !c.disableScopeInfo {
scopeInfo, err := c.scopeInfo(scopeMetrics.Scope)
Expand All @@ -202,10 +205,13 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {

ch <- scopeInfo

keys = scopeInfoKeys
values = [2]string{scopeMetrics.Scope.Name, scopeMetrics.Scope.Version}
kv.keys = append(kv.keys, scopeNameLabel, scopeVersionLabel)
kv.vals = append(kv.vals, scopeMetrics.Scope.Name, scopeMetrics.Scope.Version)
}

kv.keys = append(kv.keys, c.resourceKeyVals.keys...)
kv.vals = append(kv.vals, c.resourceKeyVals.vals...)

for _, m := range scopeMetrics.Metrics {
typ := c.metricType(m)
if typ == nil {
Expand All @@ -224,25 +230,27 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {

switch v := m.Data.(type) {
case metricdata.Histogram[int64]:
addHistogramMetric(ch, v, m, keys, values, name, c.resourceKeyVals)
addHistogramMetric(ch, v, m, name, kv)
case metricdata.Histogram[float64]:
addHistogramMetric(ch, v, m, keys, values, name, c.resourceKeyVals)
addHistogramMetric(ch, v, m, name, kv)
case metricdata.Sum[int64]:
addSumMetric(ch, v, m, keys, values, name, c.resourceKeyVals)
addSumMetric(ch, v, m, name, kv)
case metricdata.Sum[float64]:
addSumMetric(ch, v, m, keys, values, name, c.resourceKeyVals)
addSumMetric(ch, v, m, name, kv)
case metricdata.Gauge[int64]:
addGaugeMetric(ch, v, m, keys, values, name, c.resourceKeyVals)
addGaugeMetric(ch, v, m, name, kv)
case metricdata.Gauge[float64]:
addGaugeMetric(ch, v, m, keys, values, name, c.resourceKeyVals)
addGaugeMetric(ch, v, m, name, kv)
}
}
}
}

func addHistogramMetric[N int64 | float64](ch chan<- prometheus.Metric, histogram metricdata.Histogram[N], m metricdata.Metrics, ks, vs [2]string, name string, resourceKV keyVals) {
func addHistogramMetric[N int64 | float64](ch chan<- prometheus.Metric, histogram metricdata.Histogram[N], m metricdata.Metrics, name string, kv keyVals) {
for _, dp := range histogram.DataPoints {
keys, values := getAttrs(dp.Attributes, ks, vs, resourceKV)
keys, values := getAttrs(dp.Attributes)
keys = append(keys, kv.keys...)
values = append(values, kv.vals...)

desc := prometheus.NewDesc(name, m.Description, keys, nil)
buckets := make(map[float64]uint64, len(dp.Bounds))
Expand All @@ -262,14 +270,16 @@ func addHistogramMetric[N int64 | float64](ch chan<- prometheus.Metric, histogra
}
}

func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata.Sum[N], m metricdata.Metrics, ks, vs [2]string, name string, resourceKV keyVals) {
func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata.Sum[N], m metricdata.Metrics, name string, kv keyVals) {
valueType := prometheus.CounterValue
if !sum.IsMonotonic {
valueType = prometheus.GaugeValue
}

for _, dp := range sum.DataPoints {
keys, values := getAttrs(dp.Attributes, ks, vs, resourceKV)
keys, values := getAttrs(dp.Attributes)
keys = append(keys, kv.keys...)
values = append(values, kv.vals...)

desc := prometheus.NewDesc(name, m.Description, keys, nil)
m, err := prometheus.NewConstMetric(desc, valueType, float64(dp.Value), values...)
Expand All @@ -286,9 +296,11 @@ func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata
}
}

func addGaugeMetric[N int64 | float64](ch chan<- prometheus.Metric, gauge metricdata.Gauge[N], m metricdata.Metrics, ks, vs [2]string, name string, resourceKV keyVals) {
func addGaugeMetric[N int64 | float64](ch chan<- prometheus.Metric, gauge metricdata.Gauge[N], m metricdata.Metrics, name string, kv keyVals) {
for _, dp := range gauge.DataPoints {
keys, values := getAttrs(dp.Attributes, ks, vs, resourceKV)
keys, values := getAttrs(dp.Attributes)
keys = append(keys, kv.keys...)
values = append(values, kv.vals...)

desc := prometheus.NewDesc(name, m.Description, keys, nil)
m, err := prometheus.NewConstMetric(desc, prometheus.GaugeValue, float64(dp.Value), values...)
Expand All @@ -300,9 +312,9 @@ func addGaugeMetric[N int64 | float64](ch chan<- prometheus.Metric, gauge metric
}
}

// getAttrs parses the attribute.Set to two lists of matching Prometheus-style
// getAttrs converts the attribute.Set to two lists of matching Prometheus-style
// keys and values.
func getAttrs(attrs attribute.Set, ks, vs [2]string, resourceKV keyVals) ([]string, []string) {
func getAttrs(attrs attribute.Set) ([]string, []string) {
keys := make([]string, 0, attrs.Len())
values := make([]string, 0, attrs.Len())
itr := attrs.Iter()
Expand Down Expand Up @@ -334,29 +346,17 @@ func getAttrs(attrs attribute.Set, ks, vs [2]string, resourceKV keyVals) ([]stri
values = append(values, strings.Join(vals, ";"))
}
}

if ks[0] != "" {
keys = append(keys, ks[:]...)
values = append(values, vs[:]...)
}

for idx := range resourceKV.keys {
keys = append(keys, resourceKV.keys[idx])
values = append(values, resourceKV.vals[idx])
}

return keys, values
}

func createInfoMetric(name, description string, res *resource.Resource) (prometheus.Metric, error) {
keys, values := getAttrs(*res.Set(), [2]string{}, [2]string{}, keyVals{})
keys, values := getAttrs(*res.Set())
desc := prometheus.NewDesc(name, description, keys, nil)
return prometheus.NewConstMetric(desc, prometheus.GaugeValue, float64(1), values...)
}

func createScopeInfoMetric(scope instrumentation.Scope) (prometheus.Metric, error) {
keys := scopeInfoKeys[:]
desc := prometheus.NewDesc(scopeInfoMetricName, scopeInfoDescription, keys, nil)
desc := prometheus.NewDesc(scopeInfoMetricName, scopeInfoDescription, []string{scopeNameLabel, scopeVersionLabel}, nil)
return prometheus.NewConstMetric(desc, prometheus.GaugeValue, float64(1), scope.Name, scope.Version)
}

Expand Down Expand Up @@ -446,7 +446,7 @@ func (c *collector) createResourceAttributes(res *resource.Resource) {
defer c.mu.Unlock()

resourceAttrs, _ := res.Set().Filter(c.resourceAttributesFilter)
resourceKeys, resourceValues := getAttrs(resourceAttrs, [2]string{}, [2]string{}, keyVals{})
resourceKeys, resourceValues := getAttrs(resourceAttrs)
c.resourceKeyVals = keyVals{keys: resourceKeys, vals: resourceValues}
}

Expand Down

0 comments on commit 6e4c922

Please sign in to comment.