Skip to content

Commit

Permalink
Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
Browse files Browse the repository at this point in the history 
prevent conflict metric info
  • Loading branch information
@fatsheep9146
fatsheep9146 committed Nov 25, 2022
1 parent 1f5e6ad commit efd6eb0
Show file tree
Hide file tree
Showing 11 changed files with 360 additions and 18 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -52,6 +52,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Asynchronous callbacks are only called if they are registered with at least one instrument that does not use drop aggragation. (#3408)
- Do not report empty partial-success responses in the `go.opentelemetry.io/otel/exporters/otlp` exporters. (#3438, #3432)
- Handle partial success responses in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric` exporters. (#3162, #3440)
- Prevent duplicate Prometheus description, unit, and type. (#3469)
- Prevents panic when using incorrect `attribute.Value.As[Type]Slice()`. (#3489)

## Removed

Expand Down
48 changes: 48 additions & 0 deletions attribute/kv_test.go
View file
Expand Up @@ -18,6 +18,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/attribute"
)
Expand Down Expand Up @@ -132,3 +133,50 @@ func TestKeyValueValid(t *testing.T) {
}
}
}

func TestIncorrectCast(t *testing.T) {
testCases := []struct {
name string
val attribute.Value
}{
{
name: "Float64",
val: attribute.Float64Value(1.0),
},
{
name: "Int64",
val: attribute.Int64Value(2),
},
{
name: "String",
val: attribute.BoolValue(true),
},
{
name: "Float64Slice",
val: attribute.Float64SliceValue([]float64{1.0}),
},
{
name: "Int64Slice",
val: attribute.Int64SliceValue([]int64{2}),
},
{
name: "StringSlice",
val: attribute.BoolSliceValue([]bool{true}),
},
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
assert.NotPanics(t, func() {
tt.val.AsBool()
tt.val.AsBoolSlice()
tt.val.AsFloat64()
tt.val.AsFloat64Slice()
tt.val.AsInt64()
tt.val.AsInt64Slice()
tt.val.AsInterface()
tt.val.AsString()
tt.val.AsStringSlice()
})
})
}
}
44 changes: 36 additions & 8 deletions attribute/value.go
View file
Expand Up @@ -142,6 +142,13 @@ func (v Value) AsBool() bool {
// AsBoolSlice returns the []bool value. Make sure that the Value's type is
// BOOLSLICE.
func (v Value) AsBoolSlice() []bool {
if v.vtype != BOOLSLICE {
return nil
}
return v.asBoolSlice()
}

func (v Value) asBoolSlice() []bool {
return attribute.AsSlice[bool](v.slice)
}

Expand All @@ -154,6 +161,13 @@ func (v Value) AsInt64() int64 {
// AsInt64Slice returns the []int64 value. Make sure that the Value's type is
// INT64SLICE.
func (v Value) AsInt64Slice() []int64 {
if v.vtype != INT64SLICE {
return nil
}
return v.asInt64Slice()
}

func (v Value) asInt64Slice() []int64 {
return attribute.AsSlice[int64](v.slice)
}

Expand All @@ -166,6 +180,13 @@ func (v Value) AsFloat64() float64 {
// AsFloat64Slice returns the []float64 value. Make sure that the Value's type is
// FLOAT64SLICE.
func (v Value) AsFloat64Slice() []float64 {
if v.vtype != FLOAT64SLICE {
return nil
}
return v.asFloat64Slice()
}

func (v Value) asFloat64Slice() []float64 {
return attribute.AsSlice[float64](v.slice)
}

Expand All @@ -178,6 +199,13 @@ func (v Value) AsString() string {
// AsStringSlice returns the []string value. Make sure that the Value's type is
// STRINGSLICE.
func (v Value) AsStringSlice() []string {
if v.vtype != STRINGSLICE {
return nil
}
return v.asStringSlice()
}

func (v Value) asStringSlice() []string {
return attribute.AsSlice[string](v.slice)
}

Expand All @@ -189,19 +217,19 @@ func (v Value) AsInterface() interface{} {
case BOOL:
return v.AsBool()
case BOOLSLICE:
return v.AsBoolSlice()
return v.asBoolSlice()
case INT64:
return v.AsInt64()
case INT64SLICE:
return v.AsInt64Slice()
return v.asInt64Slice()
case FLOAT64:
return v.AsFloat64()
case FLOAT64SLICE:
return v.AsFloat64Slice()
return v.asFloat64Slice()
case STRING:
return v.stringly
case STRINGSLICE:
return v.AsStringSlice()
return v.asStringSlice()
}
return unknownValueType{}
}
Expand All @@ -210,19 +238,19 @@ func (v Value) AsInterface() interface{} {
func (v Value) Emit() string {
switch v.Type() {
case BOOLSLICE:
return fmt.Sprint(v.AsBoolSlice())
return fmt.Sprint(v.asBoolSlice())
case BOOL:
return strconv.FormatBool(v.AsBool())
case INT64SLICE:
return fmt.Sprint(v.AsInt64Slice())
return fmt.Sprint(v.asInt64Slice())
case INT64:
return strconv.FormatInt(v.AsInt64(), 10)
case FLOAT64SLICE:
return fmt.Sprint(v.AsFloat64Slice())
return fmt.Sprint(v.asFloat64Slice())
case FLOAT64:
return fmt.Sprint(v.AsFloat64())
case STRINGSLICE:
return fmt.Sprint(v.AsStringSlice())
return fmt.Sprint(v.asStringSlice())
case STRING:
return v.stringly
default:
Expand Down
71 changes: 63 additions & 8 deletions exporters/prometheus/exporter.go
View file
Expand Up @@ -24,6 +24,8 @@ import (
"unicode/utf8"

"github.com/prometheus/client_golang/prometheus"
dto "github.com/prometheus/client_model/go"
"google.golang.org/protobuf/proto"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -62,6 +64,7 @@ type collector struct {
disableScopeInfo bool
createTargetInfoOnce sync.Once
scopeInfos map[instrumentation.Scope]prometheus.Metric
metricFamilies map[string]*dto.MetricFamily
}

// prometheus counters MUST have a _total suffix:
Expand All @@ -83,6 +86,7 @@ func New(opts ...Option) (*Exporter, error) {
withoutUnits: cfg.withoutUnits,
disableScopeInfo: cfg.disableScopeInfo,
scopeInfos: make(map[instrumentation.Scope]prometheus.Metric),
metricFamilies: make(map[string]*dto.MetricFamily),
}

if err := cfg.registerer.Register(collector); err != nil {
Expand Down Expand Up @@ -149,22 +153,30 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
for _, m := range scopeMetrics.Metrics {
switch v := m.Data.(type) {
case metricdata.Histogram:
addHistogramMetric(ch, v, m, keys, values, c.getName(m))
addHistogramMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
case metricdata.Sum[int64]:
addSumMetric(ch, v, m, keys, values, c.getName(m))
addSumMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
case metricdata.Sum[float64]:
addSumMetric(ch, v, m, keys, values, c.getName(m))
addSumMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
case metricdata.Gauge[int64]:
addGaugeMetric(ch, v, m, keys, values, c.getName(m))
addGaugeMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
case metricdata.Gauge[float64]:
addGaugeMetric(ch, v, m, keys, values, c.getName(m))
addGaugeMetric(ch, v, m, keys, values, c.getName(m), c.metricFamilies)
}
}
}
}

func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histogram, m metricdata.Metrics, ks, vs [2]string, name string) {
func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histogram, m metricdata.Metrics, ks, vs [2]string, name string, mfs map[string]*dto.MetricFamily) {
// TODO(https://github.com/open-telemetry/opentelemetry-go/issues/3163): support exemplars
drop, help := validateMetrics(name, m.Description, dto.MetricType_HISTOGRAM.Enum(), mfs)
if drop {
return
}
if help != "" {
m.Description = help
}

for _, dp := range histogram.DataPoints {
keys, values := getAttrs(dp.Attributes, ks, vs)

Expand All @@ -185,15 +197,26 @@ func addHistogramMetric(ch chan<- prometheus.Metric, histogram metricdata.Histog
}
}

func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata.Sum[N], m metricdata.Metrics, ks, vs [2]string, name string) {
func addSumMetric[N int64 | float64](ch chan<- prometheus.Metric, sum metricdata.Sum[N], m metricdata.Metrics, ks, vs [2]string, name string, mfs map[string]*dto.MetricFamily) {
valueType := prometheus.CounterValue
metricType := dto.MetricType_COUNTER
if !sum.IsMonotonic {
valueType = prometheus.GaugeValue
metricType = dto.MetricType_GAUGE
}
if sum.IsMonotonic {
// Add _total suffix for counters
name += counterSuffix
}

drop, help := validateMetrics(name, m.Description, metricType.Enum(), mfs)
if drop {
return
}
if help != "" {
m.Description = help
}

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

Expand All @@ -207,7 +230,15 @@ 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) {
func addGaugeMetric[N int64 | float64](ch chan<- prometheus.Metric, gauge metricdata.Gauge[N], m metricdata.Metrics, ks, vs [2]string, name string, mfs map[string]*dto.MetricFamily) {
drop, help := validateMetrics(name, m.Description, dto.MetricType_GAUGE.Enum(), mfs)
if drop {
return
}
if help != "" {
m.Description = help
}

for _, dp := range gauge.DataPoints {
keys, values := getAttrs(dp.Attributes, ks, vs)

Expand Down Expand Up @@ -344,3 +375,27 @@ func sanitizeName(n string) string {

return b.String()
}

func validateMetrics(name, description string, metricType *dto.MetricType, mfs map[string]*dto.MetricFamily) (drop bool, help string) {
emf, exist := mfs[name]
if !exist {
mfs[name] = &dto.MetricFamily{
Name: proto.String(name),
Help: proto.String(description),
Type: metricType,
}
return false, ""
}
if emf.GetHelp() != description {
if emf.GetType() == dto.MetricType_HISTOGRAM || *metricType == dto.MetricType_HISTOGRAM {
return false, ""
}
otel.Handle(fmt.Errorf("the description '%s' of instrument '%s' conflicts with existing description '%s', override with existing one", description, name, emf.GetHelp()))
return false, emf.GetHelp()
}
if emf.GetType() != *metricType {
otel.Handle(fmt.Errorf("the type '%s' of instrument '%s' conflicts with existing type '%s', drop the new one. Please use a view that selects this instrument, and rename to a different name", *metricType, name, emf.GetType()))
return true, ""
}
return false, ""
}

0 comments on commit efd6eb0

Please sign in to comment.