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.

Sign up for GitHub

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

Add MetricProducer to allow sdk.MeterProviders to incorporate metric data from third-party sources #2722

Closed
wants to merge 13 commits into from
Closed

Add MetricProducer to allow sdk.MeterProviders to incorporate metric data from third-party sources #2722

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
013fb63
add MetricProducer to support third-party sources of metrics
dashpole Aug 16, 2022
280295e
clarify registration uniqueness
dashpole Aug 17, 2022
5d5c091
describe the produce function
dashpole Aug 17, 2022
e2e6a8a
add MetricReader.Register
dashpole Aug 17, 2022
79c859a
Apply suggestions from code review
dashpole Aug 18, 2022
203a048
MetricProducer is part of a MeterProvider
dashpole Aug 18, 2022
c7eb926
MetricProducers are passed to MeterProviders
dashpole Aug 19, 2022
763213c
address feedback from sig meeting
dashpole Aug 30, 2022
c2e7ef5
add instrumentation link
dashpole Sep 7, 2022
fd3c9c8
Update specification/metrics/sdk.md
dashpole Sep 9, 2022
4877d9d
emit warnings for duplicate scopes
dashpole Sep 13, 2022
79948c5
Update specification/metrics/sdk.md
dashpole Sep 14, 2022
739dd1a
Merge branch 'main' into metric_producer_bridge
dashpole Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 87 additions & 15 deletions specification/metrics/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ linkTitle: SDK
- [ForceFlush()](#forceflush)
- [Shutdown()](#shutdown)
* [Pull Metric Exporter](#pull-metric-exporter)
- [MetricProducer](#metricproducer)
* [Interface Definition](#interface-definition-1)
+ [Produce() metrics](#produce-metrics)
+ [InstrumentationScope() InstrumentationScope](#instrumentationscope-instrumentationscope)
- [Defaults and configuration](#defaults-and-configuration)
- [Numerical limits handling](#numerical-limits-handling)
- [Compatibility requirements](#compatibility-requirements)
Expand All @@ -62,7 +66,8 @@ linkTitle: SDK

A `MeterProvider` MUST provide a way to allow a [Resource](../resource/sdk.md) to
be specified. If a `Resource` is specified, it SHOULD be associated with all the
metrics produced by any `Meter` from the `MeterProvider`. The [tracing SDK
metrics produced by any `Meter` from the `MeterProvider` or produced by any
`MetricProducer`. The [tracing SDK
specification](../trace/sdk.md#additional-span-interfaces) has provided some
suggestions regarding how to implement this efficiently.

Expand All @@ -76,10 +81,16 @@ an [`InstrumentationScope`](../glossary.md#instrumentation-scope) instance which
is stored on the created `Meter`.

Configuration (i.e., [MetricExporters](#metricexporter),
[MetricReaders](#metricreader) and [Views](#view)) MUST be managed solely by the
`MeterProvider` and the SDK MUST provide a way to configure all options that are
implemented by the SDK. This MAY be done at the time of MeterProvider creation
if appropriate.
[MetricReaders](#metricreader), [MetricProducers](#metricproducer) and
[Views](#view)) MUST be managed solely by the `MeterProvider` and the SDK MUST
provide a way to configure all options that are implemented by the SDK. This
MAY be done at the time of MeterProvider creation if appropriate.

If a meter is created which produces an
[`InstrumentationScope`](../glossary.md#instrumentation-scope), which matches
the InstrumentationScope of a [MetricProducer](#metricproducer), or if multiple
[MetricProducers](#metricproducer) have the same InstrumentationScope the SDK
SHOULD emit a warning.
Comment on lines +89 to +93
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like 2 separate warnings/errors.

  1. If an instrument is created, after applying views, such that it's scope conflicts with a MetricProducer. If we create a meter that conflicts but never create an instrument that does, because of scope rename or not creating any instruments, is there an warning?
  2. When registering MetricProducers if they were to collide then there should be an error/warning. This should be a new norm forming sentence.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree this is two separate statements, but it doesn't say anything about per-instrument warnings. I believe we should emit warnings for the conflicting scopes, but allow the instruments to be correctly registered into each. IOW the presence of MetricProducers could produce scope warnings, not instrument warnings. The only way to get instrument warnings should be to have conflicts within a scope (IMO).


The `MeterProvider` MAY provide methods to update the configuration. If
configuration is updated (e.g., adding a `MetricReader`), the updated
Expand Down Expand Up @@ -136,7 +147,9 @@ instances.
### View

A `View` provides SDK users with the flexibility to customize the metrics that
are output by the SDK. Here are some examples when a `View` might be needed:
are output by the SDK from Measurements made by OpenTelemetry
[Instruments](./api.md#instrument). Here are some examples when a `View` might
be needed:

* Customize which [Instruments](./api.md#instrument) are to be
processed/ignored. For example, an [instrumented
Expand Down Expand Up @@ -769,15 +782,20 @@ used with pull-based metrics collection. A common sub-class of
`MetricReader`, the periodic exporting `MetricReader` SHOULD be provided
to be used typically with push-based metrics collection.

The `MetricReader` MUST ensure that data points are output in the
configured aggregation temporality for each instrument kind. For
synchronous instruments being output with Cumulative temporality, this
means converting [Delta to Cumulative](supplementary-guidelines.md#synchronous-example-cumulative-aggregation-temporality)
The `MetricReader` MUST ensure that data points from OpenTelemetry
[instruments](./api.md#instrument) are output in the configured aggregation
temporality for each instrument kind. For synchronous instruments being output
with Cumulative temporality, this means converting
[Delta to Cumulative](supplementary-guidelines.md#synchronous-example-cumulative-aggregation-temporality)
aggregation temporality. For asynchronous instruments being output
with Delta temporality, this means converting [Cumulative to
Delta](supplementary-guidelines.md#asynchronous-example-delta-temporality) aggregation
temporality.

The `MetricReader` is not required to ensure data points from a
dashpole marked this conversation as resolved.
Show resolved Hide resolved
dashpole marked this conversation as resolved.
Show resolved Hide resolved
[MetricProducer](#metricproducer) are output in the configured aggregation
temporality.

The SDK MUST support multiple `MetricReader` instances to be registered on the
same `MeterProvider`, and the [MetricReader.Collect](#collect) invocation on one
`MetricReader` instance SHOULD NOT introduce side-effects to other `MetricReader`
Expand Down Expand Up @@ -814,9 +832,10 @@ functions.

#### Collect

Collects the metrics from the SDK. If there are [asynchronous
Instruments](./api.md#asynchronous-instrument-api) involved, their callback
functions will be triggered.
Collects the metrics from the SDK and its [MetricProducers](#metricproducer).
It retrieves a batch of metric points from MetricProducers by invoking
`Produce()`. If there are [asynchronous SDK Instruments](./api.md#asynchronous-instrument-api)
involved, their callback functions will be triggered.

`Collect` SHOULD provide a way to let the caller know whether it succeeded,
failed or timed out. When the `Collect` operation fails or times out on
Expand All @@ -828,6 +847,11 @@ SDK](../overview.md#sdk) authors MAY choose to add parameters (e.g. callback,
filter, timeout). [OpenTelemetry SDK](../overview.md#sdk) authors MAY choose the
return value type, or do not return anything.

The `MetricReader` MUST ensure the batch of metric points from
`MetricProducers` are associated with the `MetricProducer`'s
`InstrumentationScope()`, and with the `MeterProvider`'s configured resource if
it is possible for those to conflict.
Comment on lines +852 to +853
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "if it is possible for those to conflict." mean?

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 meant it to mean: "If a batch of metric points from a MetricProducer can conflict with the MeterProvider's resource". This would be possible if a "batch of metric points" includes resource information. That wouldn't be the case in Go, but would be in Java given their current definition of "batch of metric points".

I could remove "if it is possible for those to conflict", or I could change it to "...configured resource. If a resource or scope is already present in a batch of metric points from a MetricProducer, that MUST be overridden with the MetricProducer's Instrumentation Scope and the MeterProvider's resource".

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I see this was also discussed in #2722 (comment). Now that I understand the Java SDK has Resource in its data object, I understand this point, which helps explain @bogdandrutu's remarks in #2722 (comment).


Note: it is expected that the `MetricReader.Collect` implementations will be
provided by the SDK, so it is RECOMMENDED to prevent the user from accidentally
overriding it, if possible (e.g. `final` in C++ and Java, `sealed` in C#).
Expand Down Expand Up @@ -898,8 +922,12 @@ primarily a simple telemetry data encoder and transmitter.
Metric Exporter has access to the [aggregated metrics
data](./data-model.md#timeseries-model). Metric Exporters SHOULD
report an error condition for data output by the `MetricReader` with
unsupported Aggregation or Aggregation Temporality, as this condition
can be corrected by a change of `MetricReader` configuration.
unsupported Aggregation or Aggregation Temporality. For data originating from
OpenTelemetry Instruments, this condition can be corrected by a change of
`MetricReader` configuration. For metric data from a `MetricProducer`, it is an
indication to the user that metric data is being dropped. The presence of data
dashpole marked this conversation as resolved.
Show resolved Hide resolved
with an unsupported Aggregation or Aggregation Temporality SHOULD NOT prevent
reyang marked this conversation as resolved.
Show resolved Hide resolved
the exporter from sending other metric data.

There could be multiple [Push Metric Exporters](#push-metric-exporter) or [Pull
Metric Exporters](#pull-metric-exporter) or even a mixture of both configured at
Expand Down Expand Up @@ -1086,6 +1114,50 @@ modeled to interact with other components in the SDK:
+-----------------------------+
```

## MetricProducer
dashpole marked this conversation as resolved.
Show resolved Hide resolved

**Status**: [Experimental](../document-status.md)

`MetricProducer` defines the interface which bridges to third-party metric
sources MUST implement so they can be plugged into an OpenTelemetry
[MeterProvider](#meterprovider) as a source of aggregated metric data.

```text
+-----------------+ +--------------+
| | Metrics... | |
| MeterProvider +------------> MetricReader |
| | | |
+---------------^^+ +--------------+
| MetricProducer |
+-----------------+
```

### Interface Definition

A `MetricProducer` MUST support the following functions:

#### Produce() metrics

`Produce` provides metrics from the MetricProducer to the caller. `Produce`
MUST return a batch of [Metrics](./data-model.md#metric-points). If the batch
of Metrics includes an Instrumentation Scope, it MUST be the same as the
InstrumentationScope returned by InstrumentationScope(). The batch of metrics
SHOULD NOT include resource. `Produce` does not have any required parameters,
however, [OpenTelemetry SDK](../overview.md#sdk) authors MAY choose to add
parameters (e.g. timeout).

`Produce` SHOULD provide a way to let the caller know whether it succeeded,
failed or timed out. When the `Produce` operation fails, the `MetricProducer`
MAY return successfully collected results and a failed reasons list to the
caller.

#### InstrumentationScope() InstrumentationScope

`InstrumentationScope` provides the
[InstrumentationScope](../glossary.md#instrumentation-scope) associated with
the MetricProducer. A MetricProducer instance MUST return the same
instrumentation scope for each invocation of InstrumentationScope().
dashpole marked this conversation as resolved.
Show resolved Hide resolved

## Defaults and configuration

The SDK MUST provide configuration according to the [SDK environment
Expand Down