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

[API EPIC 4/4] Fix tests and examples #2587

Merged
merged 21 commits into from Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
9 changes: 0 additions & 9 deletions .github/dependabot.yml
Expand Up @@ -227,15 +227,6 @@ updates:
schedule:
day: sunday
interval: weekly
- package-ecosystem: gomod
directory: /internal/metric
labels:
- dependencies
- go
- "Skip Changelog"
schedule:
day: sunday
interval: weekly
- package-ecosystem: gomod
directory: /internal/tools
labels:
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### ⚠️ Notice ⚠️

This update is a breaking change of the unstable Metrics API. Code instrumented with the `go.opentelemetry.io/otel/metric` <= v0.27.0 will need to be modified.

### Added

- Added support to configure the span limits with environment variables.
Expand All @@ -23,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- For tracestate's members, prepend the new element and remove the oldest one, which is over capacity (#2592)
- Add event and link drop counts to the exported data from the `oltptrace` exporter. (#2601)
- The metrics API has been significantly changed. (#2587)

### Fixed

Expand Down
2 changes: 1 addition & 1 deletion bridge/opencensus/aggregation.go
Expand Up @@ -21,8 +21,8 @@ import (

"go.opencensus.io/metric/metricdata"

"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/number"
)

var (
Expand Down
18 changes: 9 additions & 9 deletions bridge/opencensus/exporter.go
Expand Up @@ -27,13 +27,13 @@ import (

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
"go.opentelemetry.io/otel/metric/instrument"
"go.opentelemetry.io/otel/metric/unit"
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/number"
"go.opentelemetry.io/otel/sdk/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/resource"
)

Expand Down Expand Up @@ -170,17 +170,17 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (sdkapi.Descriptor, e
// Includes TypeGaugeDistribution, TypeCumulativeDistribution, TypeSummary
return sdkapi.Descriptor{}, fmt.Errorf("%w; descriptor type: %v", errConversion, ocDescriptor.Type)
}
opts := []metric.InstrumentOption{
metric.WithDescription(ocDescriptor.Description),
opts := []instrument.Option{
instrument.WithDescription(ocDescriptor.Description),
}
switch ocDescriptor.Unit {
case metricdata.UnitDimensionless:
opts = append(opts, metric.WithUnit(unit.Dimensionless))
opts = append(opts, instrument.WithUnit(unit.Dimensionless))
case metricdata.UnitBytes:
opts = append(opts, metric.WithUnit(unit.Bytes))
opts = append(opts, instrument.WithUnit(unit.Bytes))
case metricdata.UnitMilliseconds:
opts = append(opts, metric.WithUnit(unit.Milliseconds))
opts = append(opts, instrument.WithUnit(unit.Milliseconds))
}
cfg := metric.NewInstrumentConfig(opts...)
cfg := instrument.NewConfig(opts...)
return sdkapi.NewDescriptor(ocDescriptor.Name, ikind, nkind, cfg.Description(), cfg.Unit()), nil
}
24 changes: 12 additions & 12 deletions bridge/opencensus/exporter_test.go
Expand Up @@ -26,15 +26,15 @@ import (

"go.opentelemetry.io/otel"
"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/instrument"
"go.opentelemetry.io/otel/metric/unit"
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric/controller/controllertest"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metrictest"
"go.opentelemetry.io/otel/sdk/metric/number"
"go.opentelemetry.io/otel/sdk/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/resource"
)

Expand Down Expand Up @@ -400,8 +400,8 @@ func TestConvertDescriptor(t *testing.T) {
"foo",
sdkapi.GaugeObserverInstrumentKind,
number.Int64Kind,
metric.WithDescription("bar"),
metric.WithUnit(unit.Bytes),
instrument.WithDescription("bar"),
instrument.WithUnit(unit.Bytes),
),
},
{
Expand All @@ -416,8 +416,8 @@ func TestConvertDescriptor(t *testing.T) {
"foo",
sdkapi.GaugeObserverInstrumentKind,
number.Float64Kind,
metric.WithDescription("bar"),
metric.WithUnit(unit.Milliseconds),
instrument.WithDescription("bar"),
instrument.WithUnit(unit.Milliseconds),
),
},
{
Expand All @@ -432,8 +432,8 @@ func TestConvertDescriptor(t *testing.T) {
"foo",
sdkapi.CounterObserverInstrumentKind,
number.Int64Kind,
metric.WithDescription("bar"),
metric.WithUnit(unit.Dimensionless),
instrument.WithDescription("bar"),
instrument.WithUnit(unit.Dimensionless),
),
},
{
Expand All @@ -448,8 +448,8 @@ func TestConvertDescriptor(t *testing.T) {
"foo",
sdkapi.CounterObserverInstrumentKind,
number.Float64Kind,
metric.WithDescription("bar"),
metric.WithUnit(unit.Dimensionless),
instrument.WithDescription("bar"),
instrument.WithUnit(unit.Dimensionless),
),
},
{
Expand Down
62 changes: 33 additions & 29 deletions example/prometheus/main.go
Expand Up @@ -25,7 +25,7 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/exporters/prometheus"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/global"
"go.opentelemetry.io/otel/metric/instrument"
"go.opentelemetry.io/otel/sdk/metric/aggregator/histogram"
controller "go.opentelemetry.io/otel/sdk/metric/controller/basic"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
Expand All @@ -35,6 +35,9 @@ import (

var (
lemonsKey = attribute.Key("ex.com/lemons")

// TODO Bring back Global package
meterProvider metric.MeterProvider
)

func initMeter() {
Expand All @@ -54,7 +57,9 @@ func initMeter() {
if err != nil {
log.Panicf("failed to initialize prometheus exporter %v", err)
}
global.SetMeterProvider(exporter.MeterProvider())
// TODO Bring back Global package
// global.SetMeterProvider(exporter.MeterProvider())
meterProvider = exporter.MeterProvider()

http.HandleFunc("/", exporter.ServeHTTP)
go func() {
Expand All @@ -67,23 +72,33 @@ func initMeter() {
func main() {
initMeter()

meter := global.Meter("ex.com/basic")
// TODO Bring back Global package
// meter := global.Meter("ex.com/basic")
meter := meterProvider.Meter("ex.com/basic")
observerLock := new(sync.RWMutex)
observerValueToReport := new(float64)
observerLabelsToReport := new([]attribute.KeyValue)
cb := func(_ context.Context, result metric.Float64ObserverResult) {

gaugeObserver, err := meter.AsyncFloat64().Gauge("ex.com.one")
if err != nil {
log.Panicf("failed to initialize instrument: %v", err)
}
_ = meter.RegisterCallback([]instrument.Asynchronous{gaugeObserver}, func(ctx context.Context) {
(*observerLock).RLock()
value := *observerValueToReport
labels := *observerLabelsToReport
(*observerLock).RUnlock()
result.Observe(value, labels...)
}
_ = metric.Must(meter).NewFloat64GaugeObserver("ex.com.one", cb,
metric.WithDescription("A GaugeObserver set to 1.0"),
)
gaugeObserver.Observe(ctx, value, labels...)
})

histogram := metric.Must(meter).NewFloat64Histogram("ex.com.two")
counter := metric.Must(meter).NewFloat64Counter("ex.com.three")
histogram, err := meter.SyncFloat64().Histogram("ex.com.two")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do really miss Must() for instrument initialization

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the Must() forms tend to make the code read better, but they really don't make sense in this world. Both because this would either mean extra API surface, but also because our meter <-> instrument relation is different from both Prometheus and opencensus. Both had a static implementation of our meter, and it could create new instruments at init. That model doesn't work very well when you have to initialize your meter before you register instruments.

if err != nil {
log.Panicf("failed to initialize instrument: %v", err)
}
counter, err := meter.SyncFloat64().Counter("ex.com.three")
if err != nil {
log.Panicf("failed to initialize instrument: %v", err)
}

commonLabels := []attribute.KeyValue{lemonsKey.Int(10), attribute.String("A", "1"), attribute.String("B", "2"), attribute.String("C", "3")}
notSoCommonLabels := []attribute.KeyValue{lemonsKey.Int(13)}
Expand All @@ -94,38 +109,27 @@ func main() {
*observerValueToReport = 1.0
*observerLabelsToReport = commonLabels
(*observerLock).Unlock()
meter.RecordBatch(
ctx,
commonLabels,
histogram.Measurement(2.0),
counter.Measurement(12.0),
)

histogram.Record(ctx, 2.0, commonLabels...)
counter.Add(ctx, 12.0, commonLabels...)

time.Sleep(5 * time.Second)

(*observerLock).Lock()
*observerValueToReport = 1.0
*observerLabelsToReport = notSoCommonLabels
(*observerLock).Unlock()
meter.RecordBatch(
ctx,
notSoCommonLabels,
histogram.Measurement(2.0),
counter.Measurement(22.0),
)
histogram.Record(ctx, 2.0, notSoCommonLabels...)
counter.Add(ctx, 22.0, notSoCommonLabels...)

time.Sleep(5 * time.Second)

(*observerLock).Lock()
*observerValueToReport = 13.0
*observerLabelsToReport = commonLabels
(*observerLock).Unlock()
meter.RecordBatch(
ctx,
commonLabels,
histogram.Measurement(12.0),
counter.Measurement(13.0),
)
histogram.Record(ctx, 12.0, commonLabels...)
counter.Add(ctx, 13.0, commonLabels...)

fmt.Println("Example finished updating, please visit :2222")

Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlpmetric/exporter.go
Expand Up @@ -20,9 +20,9 @@ import (
"sync"

"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal/metrictransform"
"go.opentelemetry.io/otel/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/resource"
)

Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/otlpmetric/exporter_test.go
Expand Up @@ -29,16 +29,16 @@ import (
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric"
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal/metrictransform"
"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/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric/aggregator"
"go.opentelemetry.io/otel/sdk/metric/aggregator/histogram"
"go.opentelemetry.io/otel/sdk/metric/aggregator/sum"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metrictest"
"go.opentelemetry.io/otel/sdk/metric/number"
"go.opentelemetry.io/otel/sdk/metric/processor/processortest"
"go.opentelemetry.io/otel/sdk/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/resource"
commonpb "go.opentelemetry.io/proto/otlp/common/v1"
metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
Expand Down
Expand Up @@ -24,10 +24,10 @@ import (
"sync"
"time"

"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/number"
"go.opentelemetry.io/otel/sdk/resource"
commonpb "go.opentelemetry.io/proto/otlp/common/v1"
metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
Expand Down
Expand Up @@ -25,14 +25,14 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/metrictest"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/metric/aggregator"
"go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue"
"go.opentelemetry.io/otel/sdk/metric/aggregator/sum"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/export/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metrictest"
"go.opentelemetry.io/otel/sdk/metric/number"
"go.opentelemetry.io/otel/sdk/metric/sdkapi"
commonpb "go.opentelemetry.io/proto/otlp/common/v1"
metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)
Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/otlpmetric/internal/otlpmetrictest/data.go
Expand Up @@ -20,13 +20,13 @@ import (
"time"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/metrictest"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric/aggregator/sum"
"go.opentelemetry.io/otel/sdk/metric/export"
"go.opentelemetry.io/otel/sdk/metric/metrictest"
"go.opentelemetry.io/otel/sdk/metric/number"
"go.opentelemetry.io/otel/sdk/metric/processor/processortest"
"go.opentelemetry.io/otel/sdk/metric/sdkapi"
)

// OneRecordReader is a Reader that returns just one
Expand Down