Skip to content

Commit

Permalink
[exporter/prometheus] Make the exporter doesn't mutate metrics
Browse files Browse the repository at this point in the history
by removing unnecessary Sort method
  • Loading branch information
dmitryax committed Dec 1, 2022
1 parent cb48eba commit 23f9d9f
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 9 deletions.
4 changes: 2 additions & 2 deletions .chloggen/fixprommuta.yaml
Expand Up @@ -5,8 +5,8 @@ change_type: bug_fix
component: prometheusexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Mark prometheus exporter as mutable since it sorts attributes
note: Make sure the exporter doesn't mutate metrics

# One or more tracking issues related to the change
issues: [16499]
issues: [16499, 16572]

9 changes: 6 additions & 3 deletions exporter/prometheusexporter/accumulator.go
Expand Up @@ -16,6 +16,7 @@ package prometheusexporter // import "github.com/open-telemetry/opentelemetry-co

import (
"fmt"
"sort"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -302,11 +303,13 @@ func timeseriesSignature(ilmName string, metric pmetric.Metric, attributes pcomm
b.WriteString(metric.Type().String())
b.WriteString("*" + ilmName)
b.WriteString("*" + metric.Name())
attributes.Sort().Range(func(k string, v pcommon.Value) bool {
b.WriteString("*" + k + "*" + v.AsString())
attrs := make([]string, 0, attributes.Len())
attributes.Range(func(k string, v pcommon.Value) bool {
attrs = append(attrs, "*"+k+"*"+v.AsString())
return true
})

sort.Strings(attrs)
b.WriteString(strings.Join(attrs, ""))
if job, ok := extractJob(resourceAttrs); ok {
b.WriteString("*" + model.JobLabel + "*" + job)
}
Expand Down
10 changes: 10 additions & 0 deletions exporter/prometheusexporter/accumulator_test.go
Expand Up @@ -509,6 +509,16 @@ func TestAccumulateDroppedMetrics(t *testing.T) {
}
}

func TestTimeseriesSignatureNotMutating(t *testing.T) {
attrs := pcommon.NewMap()
attrs.PutStr("label_2", "2")
attrs.PutStr("label_1", "1")
origAttrs := pcommon.NewMap()
attrs.CopyTo(origAttrs)
timeseriesSignature("test_il", pmetric.NewMetric(), attrs, attrs)
require.Equal(t, origAttrs, attrs) // make sure attrs are not mutated
}

func getMetricProperties(metric pmetric.Metric) (
attributes pcommon.Map,
ts time.Time,
Expand Down
3 changes: 0 additions & 3 deletions exporter/prometheusexporter/factory.go
Expand Up @@ -20,7 +20,6 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/exporter/exporterhelper"

"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/resourcetotelemetry"
Expand Down Expand Up @@ -68,8 +67,6 @@ func createMetricsExporter(
set,
cfg,
prometheus.ConsumeMetrics,
// TODO: Consider to revert to non mutable data, need to not modify the incoming data, like sorting when calculating the signature.
exporterhelper.WithCapabilities(consumer.Capabilities{MutatesData: true}),
exporterhelper.WithStart(prometheus.Start),
exporterhelper.WithShutdown(prometheus.Shutdown),
)
Expand Down
2 changes: 1 addition & 1 deletion exporter/prometheusexporter/go.mod
Expand Up @@ -14,7 +14,6 @@ require (
github.com/stretchr/testify v1.8.1
go.opentelemetry.io/collector v0.66.1-0.20221128222955-4ff1ff379b90
go.opentelemetry.io/collector/component v0.66.1-0.20221128222955-4ff1ff379b90
go.opentelemetry.io/collector/consumer v0.66.1-0.20221128222955-4ff1ff379b90
go.opentelemetry.io/collector/pdata v0.66.1-0.20221128222955-4ff1ff379b90
go.opentelemetry.io/collector/semconv v0.66.1-0.20221128222955-4ff1ff379b90
go.uber.org/zap v1.23.0
Expand Down Expand Up @@ -127,6 +126,7 @@ require (
github.com/spf13/pflag v1.0.5 // indirect
github.com/vultr/govultr/v2 v2.17.2 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/collector/consumer v0.66.1-0.20221128222955-4ff1ff379b90 // indirect
go.opentelemetry.io/collector/featuregate v0.66.1-0.20221128222955-4ff1ff379b90 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.36.4 // indirect
go.opentelemetry.io/otel v1.11.1 // indirect
Expand Down

0 comments on commit 23f9d9f

Please sign in to comment.