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 1/4] Metric api prototype #2557

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
67 changes: 67 additions & 0 deletions metric/config.go
@@ -0,0 +1,67 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package metric // import "go.opentelemetry.io/otel/metric"

// MeterConfig contains options for Meters.
type MeterConfig struct {
instrumentationVersion string
schemaURL string
}

// InstrumentationVersion is the version of the library providing instrumentation.
func (cfg MeterConfig) InstrumentationVersion() string {
return cfg.instrumentationVersion
}

// SchemaURL is the schema_url of the library providing instrumentation.
func (cfg MeterConfig) SchemaURL() string {
return cfg.schemaURL
}

// MeterOption is an interface for applying Meter options.
type MeterOption interface {
// ApplyMeter is used to set a MeterOption value of a MeterConfig.
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
applyMeter(*MeterConfig)
}

// NewMeterConfig creates a new MeterConfig and applies
// all the given options.
func NewMeterConfig(opts ...MeterOption) MeterConfig {
var config MeterConfig
for _, o := range opts {
o.applyMeter(&config)
}
return config
}

type meterOptionFunc func(*MeterConfig)

func (fn meterOptionFunc) applyMeter(cfg *MeterConfig) {
fn(cfg)
}

// WithInstrumentationVersion sets the instrumentation version.
func WithInstrumentationVersion(version string) MeterOption {
return meterOptionFunc(func(config *MeterConfig) {
config.instrumentationVersion = version
})
}

// WithSchemaURL sets the schema URL.
func WithSchemaURL(schemaURL string) MeterOption {
return meterOptionFunc(func(config *MeterConfig) {
config.schemaURL = schemaURL
})
}
23 changes: 23 additions & 0 deletions metric/doc.go
@@ -0,0 +1,23 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/*
Package metric provides an implementation of the metrics part of the
OpenTelemetry API.

This package is currently in a pre-GA phase. Backwards incompatible changes
may be introduced in subsequent minor version releases as we work to track the
evolving OpenTelemetry specification and user feedback.
*/
package metric // import "go.opentelemetry.io/otel/metric"
69 changes: 69 additions & 0 deletions metric/go.mod
@@ -0,0 +1,69 @@
module go.opentelemetry.io/otel/metric

go 1.16

require go.opentelemetry.io/otel v1.3.0

replace go.opentelemetry.io/otel => ../

replace go.opentelemetry.io/otel/bridge/opencensus => ../bridge/opencensus

replace go.opentelemetry.io/otel/bridge/opentracing => ../bridge/opentracing

replace go.opentelemetry.io/otel/example/jaeger => ../example/jaeger

replace go.opentelemetry.io/otel/example/namedtracer => ../example/namedtracer

replace go.opentelemetry.io/otel/example/opencensus => ../example/opencensus

replace go.opentelemetry.io/otel/example/otel-collector => ../example/otel-collector

replace go.opentelemetry.io/otel/example/prom-collector => ../example/prom-collector

replace go.opentelemetry.io/otel/example/prometheus => ../example/prometheus

replace go.opentelemetry.io/otel/example/zipkin => ../example/zipkin

replace go.opentelemetry.io/otel/exporters/prometheus => ../exporters/prometheus

replace go.opentelemetry.io/otel/exporters/jaeger => ../exporters/jaeger

replace go.opentelemetry.io/otel/exporters/zipkin => ../exporters/zipkin

replace go.opentelemetry.io/otel/internal/tools => ../internal/tools

replace go.opentelemetry.io/otel/metric => ./

replace go.opentelemetry.io/otel/sdk => ../sdk

replace go.opentelemetry.io/otel/sdk/export/metric => ../sdk/export/metric

replace go.opentelemetry.io/otel/sdk/metric => ../sdk/metric

replace go.opentelemetry.io/otel/trace => ../trace

replace go.opentelemetry.io/otel/example/passthrough => ../example/passthrough

replace go.opentelemetry.io/otel/exporters/otlp/otlptrace => ../exporters/otlp/otlptrace

replace go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc => ../exporters/otlp/otlptrace/otlptracegrpc

replace go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp => ../exporters/otlp/otlptrace/otlptracehttp

replace go.opentelemetry.io/otel/exporters/otlp/otlpmetric => ../exporters/otlp/otlpmetric

replace go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc => ../exporters/otlp/otlpmetric/otlpmetricgrpc

replace go.opentelemetry.io/otel/exporters/stdout/stdoutmetric => ../exporters/stdout/stdoutmetric

replace go.opentelemetry.io/otel/exporters/stdout/stdouttrace => ../exporters/stdout/stdouttrace

replace go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp => ../exporters/otlp/otlpmetric/otlpmetrichttp

replace go.opentelemetry.io/otel/bridge/opencensus/test => ../bridge/opencensus/test

replace go.opentelemetry.io/otel/example/fib => ../example/fib

replace go.opentelemetry.io/otel/schema => ../schema

replace go.opentelemetry.io/otel/exporters/otlp/internal/retry => ../exporters/otlp/internal/retry
15 changes: 15 additions & 0 deletions metric/go.sum
@@ -0,0 +1,15 @@
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=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o=
github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
70 changes: 70 additions & 0 deletions metric/instrument/asyncfloat64/asyncfloat64.go
@@ -0,0 +1,70 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package asyncfloat64 // import "go.opentelemetry.io/otel/metric/instrument/asyncfloat64"
Copy link
Member

Choose a reason for hiding this comment

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

I think this is not a good package name for the functionality. It does not reflect any "metric" functionality. @rakyll would like your input here, since I know you have good input for these cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bogdan, do you think splitting/organizing packages into type based units like here is a problem? Even though the package names don't represent what's in them, I don't have a good suggestion. asyncfloat64instrument would be too verbose and putting all instruments in the same package might be too big. I think the choice here is a fair compromise if we need to split instruments into multiple packages. Disclaimer: I didn't have much time to think to come up with a better structure.

Copy link
Member

Choose a reason for hiding this comment

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

In golang the type name is not very important since majority of the times you don't use that in the assignments or declaration. That is one of the main reason I believe that the split in the package base on the input type does not make sense.

I think something like 'instrument.Float64Counter' and 'instrument.Float64ObserverCounter' is better because of that

Copy link
Contributor

Choose a reason for hiding this comment

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

@bogdandrutu There are 12 instruments in total, (int64, float64) x (6 instruments). This prototype has them in 4 packages, 3 instruments each. How many packages would you propose having?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I created a change that explored this approach. Here.

There are pros and cons to both approaches. There is also a takeaway that I will be incorporating into this patch.

The pros for grouping everything into the instrument package is that when looking for the kinds of instruments they are all documented together. This is a nice feature if we can have minimal other things in the package.
The downside of that approach is that the names become very unwieldy.
What is now a

asyncfloat64.UpDownCounter

Becomes a

insturment.AsyncFloat64UpDownCounter

The other pro is that we could get rid of the namespacing we do, the Meter Interface would then become the 12 Create functions + register. This was discussed in #2526, and this API in the PR is made this way because of that discussion.

The one takeaway I have is that the nonrecording implementations can be extracted into a separate package. That I will incorporate regardless of this discussion.

Copy link
Member

Choose a reason for hiding this comment

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

Personally I prefer the one package approach, and that package can either be "metric" where Meter and MeterProvider are defined.

I also believe that "Sync" can be removed from the names to shorter the common case:

+ Meter
  - ObservableInt64Counter(string, ...InstrumentOption) ObservableInt64Counter
  - ObservableInt64UpDownCounter(string, ...InstrumentOption) ObservableInt64UpDownCounter
  - ObservableInt64Gauge(string, ...InstrumentOption) ObservableInt64Gauge

  - ObservableFloat64Counter(string, ...InstrumentOption) ObservableFloat64Counter
  - ObservableFloat64UpDownCounter(string, ...InstrumentOption) ObservableFloat64UpDownCounter
  - ObservableFloat64Gauge(string, ...InstrumentOption) ObservableFloat64Gauge

  - Int64Counter(string, ...InstrumentOption) Int64Counter
  - Int64UpDownCounter(string, ...InstrumentOption) Int64UpDownCounter
  - Int64Histogram(string, ...InstrumentOption) Int64Histogram

  - Float64Counter(string, ...InstrumentOption) Float64Counter
  - Float64UpDownCounter(string, ...InstrumentOption) Float64UpDownCounter
  - Float64Histogram(string, ...InstrumentOption) Float64Histogram

Copy link
Member

@XSAM XSAM Mar 2, 2022

Choose a reason for hiding this comment

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

I slightly prefer the one package approach. End-users can find all instruments in the same place. When the generics feature is ready, we can have intuitive naming for generics functions without creating a new package:

I have no opinion for Async/Observable naming.

+ Meter
  - AsyncCounter(T, ...InstrumentOption) AsyncCounter
  - AsyncUpDownCounter(T, ...InstrumentOption) AsyncUpDownCounter
  - AsyncGauge(T, ...InstrumentOption) AsyncGauge

  - Counter(T, ...InstrumentOption) Counter
  - UpDownCounter(T, ...InstrumentOption) UpDownCounter
  - Histogram(T, ...InstrumentOption) Histogram


import (
"context"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/instrument"
)

// Instruments provides access to individual instruments.
type Instruments interface {
// Counter creates an instrument for recording increasing values.
Counter(name string, opts ...instrument.Option) (Counter, error)

// UpDownCounter creates an instrument for recording changes of a value.
UpDownCounter(name string, opts ...instrument.Option) (UpDownCounter, error)

// Gauge creates an instrument for recording the current value.
Gauge(name string, opts ...instrument.Option) (Gauge, error)
}

// Counter is an instrument that records increasing values.
type Counter interface {
// Observe records the state of the instrument.
//
// It is only valid to call this within a callback. If called outside of the
// registered callback it should have no effect on the instrument, and an
// error will be reported via the error handeling
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue)

instrument.Asynchronous
}

// UpDownCounter is an instrument that records increasing or decresing values.
type UpDownCounter interface {
// Observe records the state of the instrument.
//
// It is only valid to call this within a callback. If called outside of the
// registered callback it should have no effect on the instrument, and an
// error will be reported via the error handeling
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue)

instrument.Asynchronous
}

// Gauge is an instrument that records independent readings.
type Gauge interface {
// Observe records the state of the instrument.
//
// It is only valid to call this within a callback. If called outside of the
// registered callback it should have no effect on the instrument, and an
// error will be reported via the error handeling
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue)

instrument.Asynchronous
}
70 changes: 70 additions & 0 deletions metric/instrument/asyncint64/asyncint64.go
@@ -0,0 +1,70 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package asyncint64 // import "go.opentelemetry.io/otel/metric/instrument/asyncint64"

import (
"context"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/instrument"
)

// Instruments provides access to individual instruments.
type Instruments interface {
dashpole marked this conversation as resolved.
Show resolved Hide resolved
// Counter creates an instrument for recording increasing values.
Counter(name string, opts ...instrument.Option) (Counter, error)

// UpDownCounter creates an instrument for recording changes of a value.
UpDownCounter(name string, opts ...instrument.Option) (UpDownCounter, error)

// Gauge creates an instrument for recording the current value.
Gauge(name string, opts ...instrument.Option) (Gauge, error)
}

// Counter is an instrument that records increasing values.
type Counter interface {
// Observe records the state of the instrument.
//
// It is only valid to call this within a callback. If called outside of the
// registered callback it should have no effect on the instrument, and an
// error will be reported via the error handeling
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue)

instrument.Asynchronous
}

// UpDownCounter is an instrument that records increasing or decresing values.
type UpDownCounter interface {
// Observe records the state of the instrument.
//
// It is only valid to call this within a callback. If called outside of the
// registered callback it should have no effect on the instrument, and an
// error will be reported via the error handeling
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue)

instrument.Asynchronous
}

// Gauge is an instrument that records independent readings.
type Gauge interface {
// Observe records the state of the instrument.
//
// It is only valid to call this within a callback. If called outside of the
// registered callback it should have no effect on the instrument, and an
// error will be reported via the error handeling
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue)

instrument.Asynchronous
}
70 changes: 70 additions & 0 deletions metric/instrument/config.go
@@ -0,0 +1,70 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package instrument // import "go.opentelemetry.io/otel/metric/instrument"

import "go.opentelemetry.io/otel/metric/unit"

// Config contains options for metric instrument descriptors.
type Config struct {
description string
unit unit.Unit
}

// Description describes the instrument in human-readable terms.
func (cfg Config) Description() string {
return cfg.description
}

// Unit describes the measurement unit for a instrument.
func (cfg Config) Unit() unit.Unit {
return cfg.unit
}

// Option is an interface for applying metric instrument options.
type Option interface {
// ApplyMeter is used to set a Option value of a
// Config.
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
applyInstrument(*Config)
}

// NewConfig creates a new Config
// and applies all the given options.
func NewConfig(opts ...Option) Config {
var config Config
for _, o := range opts {
o.applyInstrument(&config)
}
return config
}

type optionFunc func(*Config)

func (fn optionFunc) applyInstrument(cfg *Config) {
fn(cfg)
}

// WithDescription applies provided description.
func WithDescription(desc string) Option {
return optionFunc(func(cfg *Config) {
cfg.description = desc
})
}

// WithUnit applies provided unit.
func WithUnit(unit unit.Unit) Option {
return optionFunc(func(cfg *Config) {
cfg.unit = unit
})
}