There are design and usability issues with the current view implementation. This issue introduces a proposal to replace the current view implementation with one that addresses these issues.
Issues
View package is used as a namespace for a sprawl of options
It seems the main motivation for the view
package is to encapsulate the large number of configuration options that exist, and those that could exist. This choice was to help prevent the pollution of the sdk/metric
code API and its documentation. However, it only splits the issue to another package.
Instead of having a sprawl of options, ideally, view definitions should be defined concisely.
View package includes unrelated types.
The included Instrument
and InstrumentKind
types refer to metric instrument concepts, not view concepts. They are only included in the package to avoid an import cycle.
Ideally, these types would live in the sdk/metric
package.
Creation of a view returns an error
The view.New
function is declared as follows.
func New(opts ...Option) (View, error) {
Because this function is declared to return not only a View
, but also an error it cannot be used in-line by users during the SDK setup. For example:
view, err := view.New(/* user options */)
if err != nil {
// Error handling.
}
NewMeterProvider(/* use view here */)
Ideally, the view could be defined in-line with the MeterProvider
. E.g.
NewMeterProvider(WithView(NewView(/* user options */)))
This would be possible if a design can be determined that would remove the two explicit errors the New
function returns.
The implicit errors, those sent to the logging system by options, are not impediments to resolving this issue.
Multiple user options of the same kind are dropped
The view.New
function docs read:
// New returns a new configured View. If there are any duplicate Options passed,
// the last one passed will take precedence. The unique, de-duplicated,
// Options are all applied to the View.
This seems like appropriate behavior, but is also means that a user needs to read this documentation to understand how multiple options are handled. If they want to match instruments with name foo
and bar
so they pass both as options to the view it will cause frustration when both are not matched.
If it were not possible for a user to pass multiple values for a match or replacement criteria by design they would understand without reading docs or frustration that they need to use multiple views.
The current view is not extendable by a user
If a user wants to do something unique with a View
outside of the provided options they are not enabled. This may seem ideal as it restricts users to only create views defined by OTel. However, this project should not be about restricting users. It should be about providing useful packages that help them achieve what they want.
It would be a benefit to the project if View
s could be defined in a way that allows users to implement any idea they have, but also provide users with a way to create views directly based on what OTel prescribes.
Proposal
A complete example of this proposal can be found here
Move Instrument
and InstrumentKind
to the sdk/metric
package
Both types can be moved to the sdk/metric
package. The import cycle that prevented this is resolved in the rest of the proposal.
The Instrument
type can be extended to include all properties of a new instrument. That way users can match with complete context of the instrument being created:
// Instrument describes properties an instrument is created with.
type Instrument struct {
Name string
Description string
Kind InstrumentKind
Unit unit.Unit
Scope instrumentation.Scope
}
An added benefit of this complete definition is that the function parameters of the pipeline can be unified with this type. See the example for how this is done.
Add a Stream
type to the sdk/metric
package to describe the metric data stream
// Stream describes the stream of data an instrument produces.
type Stream struct {
// Instrument describes the instrument that created the stream.
Instrument
Temporality metricdata.Temporality
Aggregation aggregation.Aggregation
AttributeFilter attribute.Filter
}
This type is a representation of the data stream from the OTel specification.
Replace the view.View
with a View
func
in sdk/metric
package
Currently a View
is defined as a struct
type that only conveys the configuration output of view.New
. It is not configurable outside of the view
package.
If instead we define a view as a function translating an instrument definition into a data stream definition, a user will be enabled to define view configuration as they see fit and we are still able to correctly create pipelines. For example:
// View is an override to the default behavior of the SDK. It defines how data
// should be collected for certain instruments. It returns true and the exact
// Stream to use for matching Instruments. Otherwise, if the view does not
// match, false is returned.
type View func(Instrument) (Stream, bool)
With this definition a user is able to add whatever matching conditions they need, even if they exist outside what OTel specifies, and create data streams in the context of the instrument that is being created.
Alone, however, this would require users to recreate common views (i.e. renames, aggregation setting, description updates), and it doesn't provide the functional niceties specified by OTel (wildcard name matching). These issues are addressed by including a NewView
function to create a View
with these niceties.
Define NewView
to create a View
based on OTel specification
// NewView returns a View that applies the Stream mask for all instruments that
// match criteria. The returned View will only apply mask if all non-zero-value
// fields of criteria match the corresponding Instrument passed to the view. If
// no criteria are provided, all field of criteria are their zero-values, a
// view that matches no instruments is returned.
//
// The Name field of criteria supports wildcard pattern matching. The wildcard
// "*" is recognised as matching zero or more characters, and "?" is recognised
// as matching exactly one character. For example, a pattern of "*" will match
// all instrument names.
//
// The Stream mask only applies updates for non-zero-value fields. By default,
// the Instrument the View matches against will be use for the returned Stream
// and no Aggregation or AttributeFilter are set. If mask has a non-zero-value
// value for any of the Aggregation or AttributeFilter fields, or any of the
// Instrument fields, that value is used instead of the default. If you need to
// zero out an Stream field returned from a View, create a View directly.
func NewView(criteria Instrument, mask Stream) View
Adding this function allows for the matching of instrument properties, including wildcard name matching, and the replacement functionality defined by the OTel specification. It also facilitates common matching/replacement views similar to the existing view.New
.
Why not define this function to accept Options? Accepting options makes sense as a way to allow forward compatibility, when new options need to be added new functions returning new options can be added in a backwards compatible way. They also prevent the user from having to pass empty arguments as parameters when no options are desired.
To the latter point, when a user is using this convenience function to create a View
they should never be passing "no options". If they do not pass a match criteria or mask, the view is effectively a disablement or no-op. Neither of these is likely what the user wants to do. It is expected that the common use of this function will be by users that want to match something and set something. Given this expected use, designing for the no-option possibility here is non-ergonomic.
As for the extensibility of future options, both the Instrument
and Stream
are struct
that can be extended. The zero-value of fields for these struct
s are ignored so adding new fields will behave the same as the previous non-included-field version.
As an added benefit of fully specifying each allowed parameter, there is no possibility for a user to provide duplicate options. It becomes clear by design that only one name, description, instrument kind, etc. is matches per created View
. This is something that only was understood in documentation with the option approach.
This added benefit of single fully specified match criteria also removes the existing error of view.New
where an exact and wildcard name match is asked for.
As for the other existing error, where a user does not provide any matching criteria, it is translated into a returning a view that never matches. As mentioned above, the inherent design of the function discourages this use.
Having both possible explicit errors removed, the function no longer needs to return an error. It is now able to be included in-line with MeterProvider
creation options.
The existing view.New
also allows for logged errors when it is passed aggregations that are misconfigured. This new function can use this approach to error handling for that error type. Additionally, it can use that approach for any additions to the parameters it receives that result in errors.
Deprecate the view package in favor of implementation in sdk/metric
- Deprecate
sdk/metric/view.Instrument
in favor of sdk/metric.Instrument
- Deprecate
sdk/metric/view.InstrumentKind
in favor of sdk/metric.InstrumentKind
- Deprecate
sdk/metric/view.View
in favor of sdk/metric.View
- Deprecate
sdk/metric/view.New
in favor of sdk/metric.NewView
- Update
sdk/metric
to stop using all deprecated types
Tasks
How to split the proposal into reviewable PRs.
- [x] Add
View
, NewView
, Instrument
, Stream
, and InstrumentKind
to sdk/metric
with unit tests
- https://github.com/open-telemetry/opentelemetry-go/pull/3459
- [x] Update
sdk/metric
to use View
, NewView
, Instrument
, Stream
, and InstrumentKind
from sdk/metric
- https://github.com/open-telemetry/opentelemetry-go/pull/3461
- [x] Add example tests for
NewView
and View
.
- https://github.com/open-telemetry/opentelemetry-go/pull/3460
- [x] Deprecate the
view
package
- https://github.com/open-telemetry/opentelemetry-go/pull/3476
pkg:SDK area:metrics proposal