Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate InstrumentationLibrary from metric.Descriptor #2197

Merged
merged 46 commits into from Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
cf7e04c
factor instrumentation library out of the instrument descriptor
jmacd Aug 18, 2021
b016889
SDK tests pass
jmacd Aug 18, 2021
5ab49d1
checkpoint work
jmacd Aug 19, 2021
5b9625f
otlp and opencensus tests passing
jmacd Aug 20, 2021
72e8439
prometheus
jmacd Aug 20, 2021
198f350
tests pass, working on lint
jmacd Aug 20, 2021
e201f52
lint applied: MetricReader->Reader
jmacd Aug 20, 2021
fdbe7c3
comments
jmacd Aug 20, 2021
cb9e431
Changelog
jmacd Aug 20, 2021
8c17495
Merge branch 'main' into jmacd/move_descriptor
jmacd Aug 23, 2021
9db6a72
Merge branch 'main' into jmacd/move_descriptor
jmacd Aug 31, 2021
feca077
Merge remote-tracking branch 'origin/main' into jmacd/move_descriptor
jmacd Sep 2, 2021
c51c940
Merge branch 'jmacd/move_descriptor' of github.com:jmacd/opentelemetr…
jmacd Sep 2, 2021
93615ae
Merge branch 'main' into jmacd/move_descriptor
jmacd Sep 2, 2021
aab6115
Merge branch 'main' into jmacd/move_descriptor
jmacd Sep 8, 2021
4ae9be2
Merge branch 'jmacd/move_descriptor' of github.com:jmacd/opentelemetr…
jmacd Sep 8, 2021
e591fa5
Apply suggestions from code review
jmacd Sep 8, 2021
65b8f3e
remove an interdependency
jmacd Sep 8, 2021
7871e03
fix build
jmacd Sep 8, 2021
794ed0a
Merge branch 'main' into jmacd/move_descriptor
jmacd Sep 14, 2021
7f71842
re-indent one
jmacd Sep 14, 2021
823c957
Merge branch 'jmacd/move_descriptor' of github.com:jmacd/opentelemetr…
jmacd Sep 14, 2021
c18e5e7
Apply suggestions from code review
jmacd Sep 15, 2021
89b1fd1
Lint&feedback
jmacd Sep 15, 2021
7a3d9a3
Merge branch 'jmacd/move_descriptor' of github.com:jmacd/opentelemetr…
jmacd Sep 15, 2021
5032f63
update after rename
jmacd Sep 15, 2021
a00f5fb
Merge branch 'main' into jmacd/move_descriptor
jmacd Sep 16, 2021
472b113
comment fix
jmacd Sep 16, 2021
52b6ca9
style fix for meter options
jmacd Sep 17, 2021
2780b45
remove libraryReader, let Controller implement the reader API directly
jmacd Sep 17, 2021
3b7c3ca
rename 'impl' field to 'provider'
jmacd Sep 17, 2021
23d0585
remove a type assertion
jmacd Sep 17, 2021
a927758
move metric/registry into internal; move registry.MeterProvider into …
jmacd Sep 17, 2021
f1b51da
add test for controller registry function
jmacd Sep 17, 2021
d09e2e2
CheckpointSet->Reader everywhere
jmacd Sep 17, 2021
6ad398d
lint
jmacd Sep 17, 2021
9b990d2
Merge branch 'main' of github.com:open-telemetry/opentelemetry-go int…
jmacd Sep 21, 2021
989066b
remove two unnecessary accessor methods; Controller implements MeterP…
jmacd Sep 21, 2021
ea7bc59
use a sync.Map
jmacd Sep 21, 2021
3356eb5
ensure the initOnce is always called; handle multiple errors
jmacd Sep 21, 2021
b4636f3
Apply suggestions from code review
jmacd Sep 23, 2021
b418d3d
cleanup locking in metrictest
jmacd Sep 23, 2021
21f7401
Merge branch 'jmacd/move_descriptor' of github.com:jmacd/opentelemetr…
jmacd Sep 23, 2021
265232b
Revert "ensure the initOnce is always called; handle multiple errors"
jmacd Sep 24, 2021
2e183e9
Revert "use a sync.Map"
jmacd Sep 24, 2021
6e984a5
restore the TODO about sync.Map
jmacd Sep 24, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Metric SDK/API implementation type `InstrumentKind` moves into `sdkapi` sub-package. (#2091)
- The Metrics SDK export record no longer contains a Resource pointer, the SDK `"go.opentelemetry.io/otel/sdk/trace/export/metric".Exporter.Export()` function for push-based exporters now takes a single Resource argument, pull-based exporters use `"go.opentelemetry.io/otel/sdk/metric/controller/basic".Controller.Resource()`. (#2120)
- The JSON output of the `go.opentelemetry.io/otel/exporters/stdout/stdouttrace` is harmonized now such that the output is "plain" JSON objects after each other of the form `{ ... } { ... } { ... }`. Earlier the JSON objects describing a span were wrapped in a slice for each `Exporter.ExportSpans` call, like `[ { ... } ][ { ... } { ... } ]`. Outputting JSON object directly after each other is consistent with JSON loggers, and a bit easier to parse and read. (#2196)
- The Metric SDK `Export()` function takes a new two-level reader interface for iterating over results one instrumentation library at a time.
- The former `"go.opentelemetry.io/otel/sdk/export/metric".CheckpointSet` is renamed `Reader`.
- The new interface is named `"go.opentelemetry.io/otel/sdk/export/metric".InstrumentationLibraryReader`. (#2197)
jmacd marked this conversation as resolved.
Show resolved Hide resolved

### Deprecated

Expand Down
25 changes: 19 additions & 6 deletions bridge/opencensus/exporter.go
Expand Up @@ -32,6 +32,7 @@ import (
"go.opentelemetry.io/otel/metric/unit"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/resource"
)

Expand All @@ -55,18 +56,30 @@ func (e *exporter) ExportMetrics(ctx context.Context, metrics []*metricdata.Metr
if len(metrics) != 0 {
res = convertResource(metrics[0].Resource)
}
return e.base.Export(ctx, res, &checkpointSet{metrics: metrics})
return e.base.Export(ctx, res, &censusLibraryReader{metrics: metrics})
}

type checkpointSet struct {
// RWMutex implements locking for the `CheckpointSet` interface.
type censusLibraryReader struct {
metrics []*metricdata.Metric
}

func (r censusLibraryReader) ForEach(readerFunc func(instrumentation.Library, export.Reader) error) error {
return readerFunc(instrumentation.Library{
Name: "OpenCensus Bridge",
}, &metricReader{metrics: r.metrics})
}

type metricReader struct {
// RWMutex implements locking for the `Reader` interface.
sync.RWMutex
metrics []*metricdata.Metric
}

var _ export.Reader = &metricReader{}

// ForEach iterates through the CheckpointSet, passing an
// export.Record with the appropriate aggregation to an exporter.
func (d *checkpointSet) ForEach(exporter export.ExportKindSelector, f func(export.Record) error) error {
func (d *metricReader) ForEach(exporter export.ExportKindSelector, f func(export.Record) error) error {
for _, m := range d.metrics {
descriptor, err := convertDescriptor(m.Descriptor)
if err != nil {
Expand Down Expand Up @@ -158,7 +171,6 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, e
}
opts := []metric.InstrumentOption{
metric.WithDescription(ocDescriptor.Description),
metric.WithInstrumentationName("OpenCensus Bridge"),
}
switch ocDescriptor.Unit {
case metricdata.UnitDimensionless:
Expand All @@ -168,5 +180,6 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, e
case metricdata.UnitMilliseconds:
opts = append(opts, metric.WithUnit(unit.Milliseconds))
}
return metric.NewDescriptor(ocDescriptor.Name, ikind, nkind, opts...), nil
cfg := metric.NewInstrumentConfig(opts...)
return metric.NewDescriptor(ocDescriptor.Name, ikind, nkind, cfg.Description(), cfg.Unit()), nil
}
34 changes: 16 additions & 18 deletions bridge/opencensus/exporter_test.go
Expand Up @@ -28,12 +28,15 @@ import (

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/metrictest"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
"go.opentelemetry.io/otel/metric/unit"
export "go.opentelemetry.io/otel/sdk/export/metric"
exportmetric "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric/controller/controllertest"
"go.opentelemetry.io/otel/sdk/resource"
)

Expand All @@ -44,12 +47,13 @@ type fakeExporter struct {
err error
}

func (f *fakeExporter) Export(ctx context.Context, res *resource.Resource, cps exportmetric.CheckpointSet) error {
return cps.ForEach(f, func(record exportmetric.Record) error {
f.resource = res
f.records = append(f.records, record)
return f.err
})
func (f *fakeExporter) Export(ctx context.Context, res *resource.Resource, ilmr exportmetric.InstrumentationLibraryReader) error {
return controllertest.ReadAll(ilmr, export.StatelessExportKindSelector(),
jmacd marked this conversation as resolved.
Show resolved Hide resolved
func(_ instrumentation.Library, record exportmetric.Record) error {
f.resource = res
f.records = append(f.records, record)
return f.err
})
}

type fakeErrorHandler struct {
Expand All @@ -71,11 +75,10 @@ func (f *fakeErrorHandler) matches(err error) error {

func TestExportMetrics(t *testing.T) {
now := time.Now()
basicDesc := metric.NewDescriptor(
basicDesc := metrictest.NewDescriptor(
"",
sdkapi.ValueObserverInstrumentKind,
number.Int64Kind,
metric.WithInstrumentationName("OpenCensus Bridge"),
)
fakeErrorHandler := &fakeErrorHandler{}
otel.SetErrorHandler(fakeErrorHandler)
Expand Down Expand Up @@ -393,11 +396,10 @@ func TestConvertDescriptor(t *testing.T) {
}{
{
desc: "empty descriptor",
expected: metric.NewDescriptor(
expected: metrictest.NewDescriptor(
"",
sdkapi.ValueObserverInstrumentKind,
number.Int64Kind,
metric.WithInstrumentationName("OpenCensus Bridge"),
),
},
{
Expand All @@ -408,11 +410,10 @@ func TestConvertDescriptor(t *testing.T) {
Unit: metricdata.UnitBytes,
Type: metricdata.TypeGaugeInt64,
},
expected: metric.NewDescriptor(
expected: metrictest.NewDescriptor(
"foo",
sdkapi.ValueObserverInstrumentKind,
number.Int64Kind,
metric.WithInstrumentationName("OpenCensus Bridge"),
metric.WithDescription("bar"),
metric.WithUnit(unit.Bytes),
),
Expand All @@ -425,11 +426,10 @@ func TestConvertDescriptor(t *testing.T) {
Unit: metricdata.UnitMilliseconds,
Type: metricdata.TypeGaugeFloat64,
},
expected: metric.NewDescriptor(
expected: metrictest.NewDescriptor(
"foo",
sdkapi.ValueObserverInstrumentKind,
number.Float64Kind,
metric.WithInstrumentationName("OpenCensus Bridge"),
metric.WithDescription("bar"),
metric.WithUnit(unit.Milliseconds),
),
Expand All @@ -442,11 +442,10 @@ func TestConvertDescriptor(t *testing.T) {
Unit: metricdata.UnitDimensionless,
Type: metricdata.TypeCumulativeInt64,
},
expected: metric.NewDescriptor(
expected: metrictest.NewDescriptor(
"foo",
sdkapi.SumObserverInstrumentKind,
number.Int64Kind,
metric.WithInstrumentationName("OpenCensus Bridge"),
metric.WithDescription("bar"),
metric.WithUnit(unit.Dimensionless),
),
Expand All @@ -459,11 +458,10 @@ func TestConvertDescriptor(t *testing.T) {
Unit: metricdata.UnitDimensionless,
Type: metricdata.TypeCumulativeFloat64,
},
expected: metric.NewDescriptor(
expected: metrictest.NewDescriptor(
"foo",
sdkapi.SumObserverInstrumentKind,
number.Float64Kind,
metric.WithInstrumentationName("OpenCensus Bridge"),
metric.WithDescription("bar"),
metric.WithUnit(unit.Dimensionless),
),
Expand Down
1 change: 1 addition & 0 deletions bridge/opencensus/go.mod
Expand Up @@ -8,6 +8,7 @@ require (
go.opentelemetry.io/otel/metric v0.22.0
go.opentelemetry.io/otel/sdk v1.0.0-RC2
go.opentelemetry.io/otel/sdk/export/metric v0.22.0
go.opentelemetry.io/otel/sdk/metric v0.22.0
go.opentelemetry.io/otel/trace v1.0.0-RC2
)

Expand Down
2 changes: 2 additions & 0 deletions bridge/opencensus/go.sum
@@ -1,5 +1,7 @@
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
2 changes: 2 additions & 0 deletions bridge/opencensus/test/go.sum
@@ -1,5 +1,7 @@
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
Expand Down
2 changes: 1 addition & 1 deletion example/prometheus/main.go
Expand Up @@ -40,7 +40,7 @@ var (
func initMeter() {
config := prometheus.Config{}
c := controller.New(
processor.New(
processor.NewFactory(
selector.NewWithHistogramDistribution(
histogram.WithExplicitBoundaries(config.DefaultHistogramBoundaries),
),
Expand Down
13 changes: 9 additions & 4 deletions exporters/otlp/otlpmetric/exporter.go
Expand Up @@ -24,6 +24,7 @@ import (
metricsdk "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/resource"
metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)

var (
Expand All @@ -43,16 +44,20 @@ type Exporter struct {
}

// Export exports a batch of metrics.
func (e *Exporter) Export(ctx context.Context, res *resource.Resource, checkpointSet metricsdk.CheckpointSet) error {
rms, err := metrictransform.CheckpointSet(ctx, e, res, checkpointSet, 1)
func (e *Exporter) Export(ctx context.Context, res *resource.Resource, ilmr metricsdk.InstrumentationLibraryReader) error {
rm, err := metrictransform.InstrumentationLibraryReader(ctx, e, res, ilmr, 1)
jmacd marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
if len(rms) == 0 {
if rm == nil {
return nil
}

return e.client.UploadMetrics(ctx, rms)
// TODO: There is never more than one resource emitted by this
// call, as per the specification. We can change the
// signature of UploadMetrics correspondingly. Here create a
// singleton list to reduce the size of the current PR:
jmacd marked this conversation as resolved.
Show resolved Hide resolved
return e.client.UploadMetrics(ctx, []*metricpb.ResourceMetrics{rm})
}

// Start establishes a connection to the receiving endpoint.
Expand Down