Skip to content

Commit

Permalink
Prepare to move metrics code to separate subpackage (#1316)
Browse files Browse the repository at this point in the history
* Move registry package under metric

* Move Number type to the metric/number subpackage

This also renames NumberKind type to Kind.

* Update changelog

* Drop outdated comment
  • Loading branch information
krnowak committed Nov 11, 2020
1 parent f9984f2 commit 386331a
Show file tree
Hide file tree
Showing 51 changed files with 531 additions and 493 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
This matches the returned type and fixes misuse of the term metric. (#1240)
- Move test harness from the `go.opentelemetry.io/otel/api/apitest` package into `go.opentelemetry.io/otel/oteltest`. (#1241)
- Rename `MergeItererator` to `MergeIterator` in the `go.opentelemetry.io/otel/label` package. (#1244)
- Move the `go.opentelemetry.io/otel/api/metric`, `go.opentelemetry.io/otel/api/metric/metrictest`, and `go.opentelemetry.io/otel/api/metric/registry` packages into `go.opentelemetry.io/otel` as part of #964. (#1252)
- Move the `go.opentelemetry.io/otel/api/metric` and `go.opentelemetry.io/otel/api/metric/metrictest` packages into `go.opentelemetry.io/otel` as part of #964. (#1252)
- Move the `go.opentelemetry.io/otel/api/metric/registry` package into `go.opentelemetry.io/otel/metric/registry as a part of #1303. (#1316)
- Move the `Number` type (together with related functions) from `go.opentelemetry.io/otel/api/metric` package into `go.opentelemetry.io/otel/metric/number` as a part of #1303. (#1316)
- The function signature of the Span `AddEvent` method in `go.opentelemetry.io/otel` is updated to no longer take an unused context and instead take a required name and a variable number of `EventOption`s. (#1254)
- The function signature of the Span `RecordError` method in `go.opentelemetry.io/otel` is updated to no longer take an unused context and instead take a required error value and a variable number of `EventOption`s. (#1254)
- Move the `go.opentelemetry.io/otel/api/global` package to `go.opentelemetry.io/otel/global`. (#1262)
Expand Down
2 changes: 1 addition & 1 deletion exporters/metric/prometheus/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
// This test demonstrates that it is relatively difficult to setup a
// Prometheus export pipeline:
//
// 1. The default boundaries are difficult to pass, should be []float instead of []otel.Number
// 1. The default boundaries are difficult to pass, should be []float instead of []number.Number
//
// TODO: Address this issue.

Expand Down
13 changes: 7 additions & 6 deletions exporters/metric/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/global"
"go.opentelemetry.io/otel/label"
"go.opentelemetry.io/otel/metric/number"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/controller/pull"
Expand Down Expand Up @@ -257,7 +258,7 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
}
}

func (c *collector) exportLastValue(ch chan<- prometheus.Metric, lvagg aggregation.LastValue, kind otel.NumberKind, desc *prometheus.Desc, labels []string) error {
func (c *collector) exportLastValue(ch chan<- prometheus.Metric, lvagg aggregation.LastValue, kind number.Kind, desc *prometheus.Desc, labels []string) error {
lv, _, err := lvagg.LastValue()
if err != nil {
return fmt.Errorf("error retrieving last value: %w", err)
Expand All @@ -272,7 +273,7 @@ func (c *collector) exportLastValue(ch chan<- prometheus.Metric, lvagg aggregati
return nil
}

func (c *collector) exportNonMonotonicCounter(ch chan<- prometheus.Metric, sum aggregation.Sum, kind otel.NumberKind, desc *prometheus.Desc, labels []string) error {
func (c *collector) exportNonMonotonicCounter(ch chan<- prometheus.Metric, sum aggregation.Sum, kind number.Kind, desc *prometheus.Desc, labels []string) error {
v, err := sum.Sum()
if err != nil {
return fmt.Errorf("error retrieving counter: %w", err)
Expand All @@ -287,7 +288,7 @@ func (c *collector) exportNonMonotonicCounter(ch chan<- prometheus.Metric, sum a
return nil
}

func (c *collector) exportMonotonicCounter(ch chan<- prometheus.Metric, sum aggregation.Sum, kind otel.NumberKind, desc *prometheus.Desc, labels []string) error {
func (c *collector) exportMonotonicCounter(ch chan<- prometheus.Metric, sum aggregation.Sum, kind number.Kind, desc *prometheus.Desc, labels []string) error {
v, err := sum.Sum()
if err != nil {
return fmt.Errorf("error retrieving counter: %w", err)
Expand All @@ -302,13 +303,13 @@ func (c *collector) exportMonotonicCounter(ch chan<- prometheus.Metric, sum aggr
return nil
}

func (c *collector) exportSummary(ch chan<- prometheus.Metric, dist aggregation.Distribution, kind otel.NumberKind, desc *prometheus.Desc, labels []string) error {
func (c *collector) exportSummary(ch chan<- prometheus.Metric, dist aggregation.Distribution, kind number.Kind, desc *prometheus.Desc, labels []string) error {
count, err := dist.Count()
if err != nil {
return fmt.Errorf("error retrieving count: %w", err)
}

var sum otel.Number
var sum number.Number
sum, err = dist.Sum()
if err != nil {
return fmt.Errorf("error retrieving distribution sum: %w", err)
Expand All @@ -329,7 +330,7 @@ func (c *collector) exportSummary(ch chan<- prometheus.Metric, dist aggregation.
return nil
}

func (c *collector) exportHistogram(ch chan<- prometheus.Metric, hist aggregation.Histogram, kind otel.NumberKind, desc *prometheus.Desc, labels []string) error {
func (c *collector) exportHistogram(ch chan<- prometheus.Metric, hist aggregation.Histogram, kind number.Kind, desc *prometheus.Desc, labels []string) error {
buckets, err := hist.Histogram()
if err != nil {
return fmt.Errorf("error retrieving histogram: %w", err)
Expand Down
24 changes: 12 additions & 12 deletions exporters/otlp/internal/transform/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ import (
"sync"
"time"

"go.opentelemetry.io/otel"
commonpb "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen/common/v1"
metricpb "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen/metrics/v1"
resourcepb "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen/resource/v1"

"go.opentelemetry.io/otel/label"
"go.opentelemetry.io/otel/metric/number"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/instrumentation"
Expand Down Expand Up @@ -294,7 +294,7 @@ func Record(exportSelector export.ExportKindSelector, r export.Record) (*metricp
}
}

func gaugePoint(record export.Record, num otel.Number, start, end time.Time) (*metricpb.Metric, error) {
func gaugePoint(record export.Record, num number.Number, start, end time.Time) (*metricpb.Metric, error) {
desc := record.Descriptor()
labels := record.Labels()

Expand All @@ -305,7 +305,7 @@ func gaugePoint(record export.Record, num otel.Number, start, end time.Time) (*m
}

switch n := desc.NumberKind(); n {
case otel.Int64NumberKind:
case number.Int64Kind:
m.Data = &metricpb.Metric_IntGauge{
IntGauge: &metricpb.IntGauge{
DataPoints: []*metricpb.IntDataPoint{
Expand All @@ -318,7 +318,7 @@ func gaugePoint(record export.Record, num otel.Number, start, end time.Time) (*m
},
},
}
case otel.Float64NumberKind:
case number.Float64Kind:
m.Data = &metricpb.Metric_DoubleGauge{
DoubleGauge: &metricpb.DoubleGauge{
DataPoints: []*metricpb.DoubleDataPoint{
Expand Down Expand Up @@ -348,7 +348,7 @@ func exportKindToTemporality(ek export.ExportKind) metricpb.AggregationTemporali
return metricpb.AggregationTemporality_AGGREGATION_TEMPORALITY_UNSPECIFIED
}

func sumPoint(record export.Record, num otel.Number, start, end time.Time, ek export.ExportKind, monotonic bool) (*metricpb.Metric, error) {
func sumPoint(record export.Record, num number.Number, start, end time.Time, ek export.ExportKind, monotonic bool) (*metricpb.Metric, error) {
desc := record.Descriptor()
labels := record.Labels()

Expand All @@ -359,7 +359,7 @@ func sumPoint(record export.Record, num otel.Number, start, end time.Time, ek ex
}

switch n := desc.NumberKind(); n {
case otel.Int64NumberKind:
case number.Int64Kind:
m.Data = &metricpb.Metric_IntSum{
IntSum: &metricpb.IntSum{
IsMonotonic: monotonic,
Expand All @@ -374,7 +374,7 @@ func sumPoint(record export.Record, num otel.Number, start, end time.Time, ek ex
},
},
}
case otel.Float64NumberKind:
case number.Float64Kind:
m.Data = &metricpb.Metric_DoubleSum{
DoubleSum: &metricpb.DoubleSum{
IsMonotonic: monotonic,
Expand All @@ -398,7 +398,7 @@ func sumPoint(record export.Record, num otel.Number, start, end time.Time, ek ex

// minMaxSumCountValue returns the values of the MinMaxSumCount Aggregator
// as discrete values.
func minMaxSumCountValues(a aggregation.MinMaxSumCount) (min, max, sum otel.Number, count int64, err error) {
func minMaxSumCountValues(a aggregation.MinMaxSumCount) (min, max, sum number.Number, count int64, err error) {
if min, err = a.Min(); err != nil {
return
}
Expand Down Expand Up @@ -433,7 +433,7 @@ func minMaxSumCount(record export.Record, a aggregation.MinMaxSumCount) (*metric
bounds := []float64{0.0, 100.0}

switch n := desc.NumberKind(); n {
case otel.Int64NumberKind:
case number.Int64Kind:
m.Data = &metricpb.Metric_IntHistogram{
IntHistogram: &metricpb.IntHistogram{
DataPoints: []*metricpb.IntHistogramDataPoint{
Expand All @@ -449,7 +449,7 @@ func minMaxSumCount(record export.Record, a aggregation.MinMaxSumCount) (*metric
},
},
}
case otel.Float64NumberKind:
case number.Float64Kind:
m.Data = &metricpb.Metric_DoubleHistogram{
DoubleHistogram: &metricpb.DoubleHistogram{
DataPoints: []*metricpb.DoubleHistogramDataPoint{
Expand Down Expand Up @@ -513,7 +513,7 @@ func histogramPoint(record export.Record, ek export.ExportKind, a aggregation.Hi
Unit: string(desc.Unit()),
}
switch n := desc.NumberKind(); n {
case otel.Int64NumberKind:
case number.Int64Kind:
m.Data = &metricpb.Metric_IntHistogram{
IntHistogram: &metricpb.IntHistogram{
AggregationTemporality: exportKindToTemporality(ek),
Expand All @@ -530,7 +530,7 @@ func histogramPoint(record export.Record, ek export.ExportKind, a aggregation.Hi
},
},
}
case otel.Float64NumberKind:
case number.Float64Kind:
m.Data = &metricpb.Metric_DoubleHistogram{
DoubleHistogram: &metricpb.DoubleHistogram{
AggregationTemporality: exportKindToTemporality(ek),
Expand Down
39 changes: 20 additions & 19 deletions exporters/otlp/internal/transform/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel"
commonpb "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen/common/v1"
metricpb "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen/metrics/v1"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/label"
"go.opentelemetry.io/otel/metric/number"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/export/metric/metrictest"
Expand Down Expand Up @@ -114,15 +115,15 @@ func TestMinMaxSumCountValue(t *testing.T) {
require.NoError(t, mmsc.SynchronizedMove(ckpt, &otel.Descriptor{}))
min, max, sum, count, err := minMaxSumCountValues(ckpt.(aggregation.MinMaxSumCount))
if assert.NoError(t, err) {
assert.Equal(t, min, otel.NewInt64Number(1))
assert.Equal(t, max, otel.NewInt64Number(10))
assert.Equal(t, sum, otel.NewInt64Number(11))
assert.Equal(t, min, number.NewInt64Number(1))
assert.Equal(t, max, number.NewInt64Number(10))
assert.Equal(t, sum, number.NewInt64Number(11))
assert.Equal(t, count, int64(2))
}
}

func TestMinMaxSumCountDatapoints(t *testing.T) {
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Int64NumberKind)
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Int64Kind)
labels := label.NewSet()
mmsc, ckpt := metrictest.Unslice2(minmaxsumcount.New(2, &desc))

Expand Down Expand Up @@ -161,10 +162,10 @@ func TestMinMaxSumCountPropagatesErrors(t *testing.T) {
}

func TestSumIntDataPoints(t *testing.T) {
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Int64NumberKind)
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Int64Kind)
labels := label.NewSet()
s, ckpt := metrictest.Unslice2(sumAgg.New(2))
assert.NoError(t, s.Update(context.Background(), otel.Number(1), &desc))
assert.NoError(t, s.Update(context.Background(), number.Number(1), &desc))
require.NoError(t, s.SynchronizedMove(ckpt, &desc))
record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd)
sum, ok := ckpt.(aggregation.Sum)
Expand All @@ -189,10 +190,10 @@ func TestSumIntDataPoints(t *testing.T) {
}

func TestSumFloatDataPoints(t *testing.T) {
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Float64NumberKind)
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Float64Kind)
labels := label.NewSet()
s, ckpt := metrictest.Unslice2(sumAgg.New(2))
assert.NoError(t, s.Update(context.Background(), otel.NewFloat64Number(1), &desc))
assert.NoError(t, s.Update(context.Background(), number.NewFloat64Number(1), &desc))
require.NoError(t, s.SynchronizedMove(ckpt, &desc))
record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd)
sum, ok := ckpt.(aggregation.Sum)
Expand All @@ -218,10 +219,10 @@ func TestSumFloatDataPoints(t *testing.T) {
}

func TestLastValueIntDataPoints(t *testing.T) {
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Int64NumberKind)
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Int64Kind)
labels := label.NewSet()
s, ckpt := metrictest.Unslice2(lvAgg.New(2))
assert.NoError(t, s.Update(context.Background(), otel.Number(100), &desc))
assert.NoError(t, s.Update(context.Background(), number.Number(100), &desc))
require.NoError(t, s.SynchronizedMove(ckpt, &desc))
record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd)
sum, ok := ckpt.(aggregation.LastValue)
Expand All @@ -244,7 +245,7 @@ func TestLastValueIntDataPoints(t *testing.T) {
}

func TestSumErrUnknownValueType(t *testing.T) {
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.NumberKind(-1))
desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Kind(-1))
labels := label.NewSet()
s := &sumAgg.New(1)[0]
record := export.NewRecord(&desc, &labels, nil, s, intervalStart, intervalEnd)
Expand Down Expand Up @@ -273,7 +274,7 @@ func (t *testAgg) Aggregation() aggregation.Aggregation {

// None of these three are used:

func (t *testAgg) Update(ctx context.Context, number otel.Number, descriptor *otel.Descriptor) error {
func (t *testAgg) Update(ctx context.Context, number number.Number, descriptor *otel.Descriptor) error {
return nil
}
func (t *testAgg) SynchronizedMove(destination export.Aggregator, descriptor *otel.Descriptor) error {
Expand All @@ -295,25 +296,25 @@ type testErrMinMaxSumCount struct {
testErrSum
}

func (te *testErrLastValue) LastValue() (otel.Number, time.Time, error) {
func (te *testErrLastValue) LastValue() (number.Number, time.Time, error) {
return 0, time.Time{}, te.err
}
func (te *testErrLastValue) Kind() aggregation.Kind {
return aggregation.LastValueKind
}

func (te *testErrSum) Sum() (otel.Number, error) {
func (te *testErrSum) Sum() (number.Number, error) {
return 0, te.err
}
func (te *testErrSum) Kind() aggregation.Kind {
return aggregation.SumKind
}

func (te *testErrMinMaxSumCount) Min() (otel.Number, error) {
func (te *testErrMinMaxSumCount) Min() (number.Number, error) {
return 0, te.err
}

func (te *testErrMinMaxSumCount) Max() (otel.Number, error) {
func (te *testErrMinMaxSumCount) Max() (number.Number, error) {
return 0, te.err
}

Expand All @@ -329,7 +330,7 @@ var _ aggregation.MinMaxSumCount = &testErrMinMaxSumCount{}

func TestRecordAggregatorIncompatibleErrors(t *testing.T) {
makeMpb := func(kind aggregation.Kind, agg aggregation.Aggregation) (*metricpb.Metric, error) {
desc := otel.NewDescriptor("things", otel.CounterInstrumentKind, otel.Int64NumberKind)
desc := otel.NewDescriptor("things", otel.CounterInstrumentKind, number.Int64Kind)
labels := label.NewSet()
res := resource.Empty()
test := &testAgg{
Expand Down Expand Up @@ -366,7 +367,7 @@ func TestRecordAggregatorIncompatibleErrors(t *testing.T) {

func TestRecordAggregatorUnexpectedErrors(t *testing.T) {
makeMpb := func(kind aggregation.Kind, agg aggregation.Aggregation) (*metricpb.Metric, error) {
desc := otel.NewDescriptor("things", otel.CounterInstrumentKind, otel.Int64NumberKind)
desc := otel.NewDescriptor("things", otel.CounterInstrumentKind, number.Int64Kind)
labels := label.NewSet()
res := resource.Empty()
return Record(export.CumulativeExportKindSelector(), export.NewRecord(&desc, &labels, res, agg, intervalStart, intervalEnd))
Expand Down
Loading

0 comments on commit 386331a

Please sign in to comment.