Skip to content

Commit

Permalink
Remove WithKeys() option, defaultkeys batcher (#639)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmacd authored Apr 14, 2020
1 parent bcb8b64 commit a8f7b32
Show file tree
Hide file tree
Showing 13 changed files with 13 additions and 415 deletions.
22 changes: 0 additions & 22 deletions api/metric/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ type Config struct {
Description string
// Unit is an optional field describing the metric instrument.
Unit unit.Unit
// Keys are recommended keys determined in the handles
// obtained for the metric.
Keys []core.Key
// Resource describes the entity for which measurements are made.
Resource resource.Resource
// LibraryName is the name given to the Meter that created
Expand Down Expand Up @@ -117,13 +114,6 @@ func (d Descriptor) MetricKind() Kind {
return d.kind
}

// Keys returns the recommended keys included in the metric
// definition. These keys may be used by a Batcher as a default set
// of grouping keys for the metric instrument.
func (d Descriptor) Keys() []core.Key {
return d.config.Keys
}

// Description provides a human-readable description of the metric
// instrument.
func (d Descriptor) Description() string {
Expand Down Expand Up @@ -210,18 +200,6 @@ func (u unitOption) Apply(config *Config) {
config.Unit = unit.Unit(u)
}

// WithKeys applies recommended label keys. Multiple `WithKeys`
// options accumulate.
func WithKeys(keys ...core.Key) Option {
return keysOption(keys)
}

type keysOption []core.Key

func (k keysOption) Apply(config *Config) {
config.Keys = append(config.Keys, k...)
}

// WithResource applies provided Resource.
//
// This will override any existing Resource.
Expand Down
24 changes: 0 additions & 24 deletions api/metric/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ func TestOptions(t *testing.T) {
type testcase struct {
name string
opts []metric.Option
keys []core.Key
desc string
unit unit.Unit
resource resource.Resource
Expand All @@ -47,23 +46,6 @@ func TestOptions(t *testing.T) {
{
name: "no opts",
opts: nil,
keys: nil,
desc: "",
unit: "",
resource: resource.Resource{},
},
{
name: "keys keys keys",
opts: []metric.Option{
metric.WithKeys(key.New("foo"), key.New("foo2")),
metric.WithKeys(key.New("bar"), key.New("bar2")),
metric.WithKeys(key.New("baz"), key.New("baz2")),
},
keys: []core.Key{
key.New("foo"), key.New("foo2"),
key.New("bar"), key.New("bar2"),
key.New("baz"), key.New("baz2"),
},
desc: "",
unit: "",
resource: resource.Resource{},
Expand All @@ -73,7 +55,6 @@ func TestOptions(t *testing.T) {
opts: []metric.Option{
metric.WithDescription("stuff"),
},
keys: nil,
desc: "stuff",
unit: "",
resource: resource.Resource{},
Expand All @@ -84,7 +65,6 @@ func TestOptions(t *testing.T) {
metric.WithDescription("stuff"),
metric.WithDescription("things"),
},
keys: nil,
desc: "things",
unit: "",
resource: resource.Resource{},
Expand All @@ -94,7 +74,6 @@ func TestOptions(t *testing.T) {
opts: []metric.Option{
metric.WithUnit("s"),
},
keys: nil,
desc: "",
unit: "s",
resource: resource.Resource{},
Expand All @@ -105,7 +84,6 @@ func TestOptions(t *testing.T) {
metric.WithUnit("s"),
metric.WithUnit("h"),
},
keys: nil,
desc: "",
unit: "h",
resource: resource.Resource{},
Expand All @@ -115,7 +93,6 @@ func TestOptions(t *testing.T) {
opts: []metric.Option{
metric.WithResource(*resource.New(key.New("name").String("test-name"))),
},
keys: nil,
desc: "",
unit: "",
resource: *resource.New(key.New("name").String("test-name")),
Expand All @@ -126,7 +103,6 @@ func TestOptions(t *testing.T) {
if diff := cmp.Diff(metric.Configure(tt.opts), metric.Config{
Description: tt.desc,
Unit: tt.unit,
Keys: tt.keys,
Resource: tt.resource,
}); diff != "" {
t.Errorf("Compare options: -got +want %s", diff)
Expand Down
1 change: 0 additions & 1 deletion example/basic/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ func main() {
result.Observe(1, commonLabels...)
}
_ = metric.Must(meter).RegisterFloat64Observer("ex.com.one", oneMetricCB,
metric.WithKeys(fooKey, barKey, lemonsKey),
metric.WithDescription("An observer set to 1.0"),
)

Expand Down
5 changes: 1 addition & 4 deletions example/prometheus/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ import (
)

var (
fooKey = key.New("ex.com/foo")
barKey = key.New("ex.com/bar")
lemonsKey = key.New("ex.com/lemons")
)

Expand Down Expand Up @@ -63,11 +61,10 @@ func main() {
result.Observe(value, labels...)
}
_ = metric.Must(meter).RegisterFloat64Observer("ex.com.one", cb,
metric.WithKeys(fooKey, barKey, lemonsKey),
metric.WithDescription("A measure set to 1.0"),
)

measureTwo := metric.Must(meter).NewFloat64Measure("ex.com.two", metric.WithKeys(key.New("A")))
measureTwo := metric.Must(meter).NewFloat64Measure("ex.com.two")
measureThree := metric.Must(meter).NewFloat64Counter("ex.com.three")

commonLabels := []core.KeyValue{lemonsKey.Int(10), key.String("A", "1"), key.String("B", "2"), key.String("C", "3")}
Expand Down
4 changes: 2 additions & 2 deletions exporters/metric/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"go.opentelemetry.io/otel/api/global"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregator"
"go.opentelemetry.io/otel/sdk/metric/batcher/defaultkeys"
"go.opentelemetry.io/otel/sdk/metric/batcher/ungrouped"
"go.opentelemetry.io/otel/sdk/metric/controller/push"
"go.opentelemetry.io/otel/sdk/metric/selector/simple"
)
Expand Down Expand Up @@ -158,7 +158,7 @@ func NewExportPipeline(config Config, period time.Duration) (*push.Controller, h
// it could try again on the next scrape and no data would be lost, only resolution.
//
// Gauges (or LastValues) and Summaries are an exception to this and have different behaviors.
batcher := defaultkeys.New(selector, export.NewDefaultLabelEncoder(), true)
batcher := ungrouped.New(selector, export.NewDefaultLabelEncoder(), true)
pusher := push.New(batcher, exporter, period)
pusher.Start()

Expand Down
2 changes: 1 addition & 1 deletion exporters/metric/stdout/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func ExampleNewExportPipeline() {
meter := pusher.Meter("example")

// Create and update a single counter:
counter := metric.Must(meter).NewInt64Counter("a.counter", metric.WithKeys(key))
counter := metric.Must(meter).NewInt64Counter("a.counter")
labels := []core.KeyValue{key.String("value")}

counter.Add(ctx, 100, labels...)
Expand Down
23 changes: 4 additions & 19 deletions exporters/metric/stdout/stdout.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"strings"
"time"

"go.opentelemetry.io/otel/api/core"
"go.opentelemetry.io/otel/api/global"

export "go.opentelemetry.io/otel/sdk/export/metric"
Expand Down Expand Up @@ -214,33 +213,19 @@ func (e *Exporter) Export(_ context.Context, checkpointSet export.CheckpointSet)
}
}

specifiedKeyMap := make(map[core.Key]core.Value)
var encodedLabels string
iter := record.Labels().Iter()
for iter.Next() {
kv := iter.Label()
specifiedKeyMap[kv.Key] = kv.Value
}

var materializedKeys []string

if iter.Len() > 0 {
encoded := record.Labels().Encoded(e.config.LabelEncoder)
materializedKeys = append(materializedKeys, encoded)
}

for _, k := range desc.Keys() {
if _, ok := specifiedKeyMap[k]; !ok {
materializedKeys = append(materializedKeys, string(k))
}
encodedLabels = record.Labels().Encoded(e.config.LabelEncoder)
}

var sb strings.Builder

sb.WriteString(desc.Name())

if len(materializedKeys) > 0 {
if len(encodedLabels) > 0 {
sb.WriteRune('{')
sb.WriteString(strings.Join(materializedKeys, ","))
sb.WriteString(encodedLabels)
sb.WriteRune('}')
}

Expand Down
19 changes: 0 additions & 19 deletions exporters/metric/stdout/stdout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,22 +276,3 @@ func TestStdoutLastValueNotSet(t *testing.T) {

require.Equal(t, `{"updates":null}`, fix.Output())
}

func TestStdoutCounterWithUnspecifiedKeys(t *testing.T) {
fix := newFixture(t, stdout.Config{})

checkpointSet := test.NewCheckpointSet(export.NewDefaultLabelEncoder())

keys := []core.Key{key.New("C"), key.New("D")}

desc := metric.NewDescriptor("test.name", metric.CounterKind, core.Int64NumberKind, metric.WithKeys(keys...))
cagg := sum.New()
aggtest.CheckedUpdate(fix.t, cagg, core.NewInt64Number(10), &desc)
cagg.Checkpoint(fix.ctx, &desc)

checkpointSet.Add(&desc, cagg, key.String("A", "B"))

fix.Export(checkpointSet)

require.Equal(t, `{"updates":[{"name":"test.name{A=B,C,D}","sum":10}]}`, fix.Output())
}
8 changes: 0 additions & 8 deletions exporters/otlp/internal/transform/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) {
tests := []struct {
name string
metricKind metric.Kind
keys []core.Key
description string
unit unit.Unit
numberKind core.NumberKind
Expand All @@ -112,7 +111,6 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) {
{
"mmsc-test-a",
metric.MeasureKind,
[]core.Key{},
"test-a-description",
unit.Dimensionless,
core.Int64NumberKind,
Expand All @@ -128,7 +126,6 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) {
{
"mmsc-test-b",
metric.CounterKind, // This shouldn't change anything.
[]core.Key{"test"}, // This shouldn't change anything.
"test-b-description",
unit.Bytes,
core.Float64NumberKind, // This shouldn't change anything.
Expand All @@ -151,7 +148,6 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) {
mmsc.Checkpoint(ctx, &metric.Descriptor{})
for _, test := range tests {
desc := metric.NewDescriptor(test.name, test.metricKind, test.numberKind,
metric.WithKeys(test.keys...),
metric.WithDescription(test.description),
metric.WithUnit(test.unit))
labels := export.NewSimpleLabels(export.NoopLabelEncoder{}, test.labels...)
Expand Down Expand Up @@ -208,7 +204,6 @@ func TestSumMetricDescriptor(t *testing.T) {
tests := []struct {
name string
metricKind metric.Kind
keys []core.Key
description string
unit unit.Unit
numberKind core.NumberKind
Expand All @@ -218,7 +213,6 @@ func TestSumMetricDescriptor(t *testing.T) {
{
"sum-test-a",
metric.CounterKind,
[]core.Key{},
"test-a-description",
unit.Dimensionless,
core.Int64NumberKind,
Expand All @@ -234,7 +228,6 @@ func TestSumMetricDescriptor(t *testing.T) {
{
"sum-test-b",
metric.MeasureKind, // This shouldn't change anything.
[]core.Key{"test"}, // This shouldn't change anything.
"test-b-description",
unit.Milliseconds,
core.Float64NumberKind,
Expand All @@ -251,7 +244,6 @@ func TestSumMetricDescriptor(t *testing.T) {

for _, test := range tests {
desc := metric.NewDescriptor(test.name, test.metricKind, test.numberKind,
metric.WithKeys(test.keys...),
metric.WithDescription(test.description),
metric.WithUnit(test.unit),
)
Expand Down
Loading

0 comments on commit a8f7b32

Please sign in to comment.