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

Add more details to SDK MeterProvider and View #1730

Merged
merged 29 commits into from
Jul 27, 2021
Merged
Changes from 19 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4b866a8
Add View API
reyang May 28, 2021
b3259fc
fix link
reyang May 28, 2021
be9beb5
address comments
reyang Jun 24, 2021
c5c73aa
provide one example
reyang Jun 24, 2021
f82475e
update wording
reyang Jun 24, 2021
7359e9a
polish wording
reyang Jun 24, 2021
2df446b
update wording
reyang Jun 24, 2021
09501aa
mention histogram bucket in the view aggregation
reyang Jun 24, 2021
87e3bfe
resolve comments based on discussion during the SIG meeting
reyang Jun 27, 2021
0c32806
fix typo
reyang Jun 27, 2021
142a345
update the spec based on discussion during the SIG meeting
reyang Jun 29, 2021
8b9a7f0
add more diagrams
reyang Jul 10, 2021
d14d005
update the spec based on the discussion during Metrics SIG meeting
reyang Jul 10, 2021
bbe5b40
minor tweak diagram
reyang Jul 11, 2021
49bf1b1
address review comment
reyang Jul 13, 2021
7d8336d
update the default histogram buckets
reyang Jul 13, 2021
083c51c
remove pipeline from this PR
reyang Jul 20, 2021
4f00ac7
address comments regarding error handling
reyang Jul 20, 2021
3340239
Merge branch 'main' into reyang/metrics-sdk-view
reyang Jul 21, 2021
2d5a888
Update specification/metrics/sdk.md
reyang Jul 21, 2021
f1621e7
adjust wording
reyang Jul 22, 2021
e606fe5
Merge branch 'main' into reyang/metrics-sdk-view
reyang Jul 22, 2021
4d0913a
add instrument type and clarify the selection criteria
reyang Jul 26, 2021
6261507
update the view selection logic
reyang Jul 26, 2021
954102f
update the view selection logic
reyang Jul 26, 2021
5fc0500
fix broken link
reyang Jul 26, 2021
971e770
Merge branch 'main' into reyang/metrics-sdk-view
reyang Jul 26, 2021
4ab3465
update the TODO part by removing the details
reyang Jul 27, 2021
f9f47ed
Merge branch 'main' into reyang/metrics-sdk-view
reyang Jul 27, 2021
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
178 changes: 167 additions & 11 deletions specification/metrics/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,168 @@ Table of Contents

## MeterProvider

TODO:
### Meter Creation

New `Meter` instances are always created through a `MeterProvider` (see
[API](./api.md#meterprovider)). The `name`, `version` (optional), and
`schema_url` (optional) arguments supplied to the `MeterProvider` MUST be used
to create an
[`InstrumentationLibrary`](https://github.com/open-telemetry/oteps/blob/main/text/0083-component.md)
instance which is stored on the created `Meter`.

Configuration (i.e., [MeasurementProcessors](#measurementprocessor),
[MetricProcessors](#metricprocessor), [MetricExporters](#metricexporter) and
[`Views`](#view)) MUST be managed solely by the `MeterProvider` and it MUST
reyang marked this conversation as resolved.
Show resolved Hide resolved
provide some way to configure all of them that are implemented in the SDK, at
least when creating or initializing it.
reyang marked this conversation as resolved.
Show resolved Hide resolved

The `MeterProvider` MAY provide methods to update the configuration. If
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
configuration is updated (e.g., adding a `MetricProcessor`), the updated
configuration MUST also apply to all already returned `Meters` (i.e. it MUST NOT
matter whether a `Meter` was obtained from the `MeterProvider` before or after
the configuration change). Note: Implementation-wise, this could mean that
`Meter` instances have a reference to their `MeterProvider` and access
configuration only via this reference.

### Shutdown

TODO

### ForceFlush
reyang marked this conversation as resolved.
Show resolved Hide resolved

TODO

### View
reyang marked this conversation as resolved.
Show resolved Hide resolved

`View` gives the SDK users flexibility to customize the metrics they want. Here
are some examples when `View` is needed:
reyang marked this conversation as resolved.
Show resolved Hide resolved

* Customize which [Instrument](./api.md#instrument) to be processed/ignored. For
reyang marked this conversation as resolved.
Show resolved Hide resolved
example, an [instrumented library](../glossary.md#instrumented-library) can
provide both temperature and humidity, but the application developer only
wants temperature information.
reyang marked this conversation as resolved.
Show resolved Hide resolved
* Customize the aggregation - if the default aggregation associated with the
Instrument does not meet the expectation. For example, an HTTP client library
reyang marked this conversation as resolved.
Show resolved Hide resolved
might expose HTTP client request duration as [Histogram](./api.md#histogram)
by default, but the application developer only wants the total count of
reyang marked this conversation as resolved.
Show resolved Hide resolved
outgoing requests.
* Customize which attribute(s) to be reported as metrics dimension(s). For
reyang marked this conversation as resolved.
Show resolved Hide resolved
example, an HTTP server library might expose HTTP verb (e.g. GET, POST) and
HTTP status code (e.g. 200, 301, 404). The application developer might only
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this belongs, but adjusting the sampling of Exemplars is likely an aggregator problem. I can write up my thoughts here but I investigated what Prometheus has been up to in this space and I think likely exemplar sampling should be tied to aggregator. If not, we should call it out as a thing users may want to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jsuereth Agree. One simple aggregator for exemplars would use an equal-probability reservoir sampler such as Algorithm R. Then, we can apply an adjusted_count attribute to each exemplar, and a downstream system can combine exemplars into a data set, which can be used to estimate histograms of the full-cardinality event stream.

care about HTTP status code (e.g. reporting the total count of HTTP requests
for each HTTP status code). There are also extreme scenario that the
application developer does not need any dimension (e.g. just get the total
reyang marked this conversation as resolved.
Show resolved Hide resolved
count of all incoming requests).
* Add additional dimension(s) from the [Context](../context/context.md). For
example, a [Baggage](../baggage/api.md) value might be available indicating
whether an HTTP request is coming from a bot/crawler or not. The application
developer might want this to be converted to a dimension for HTTP server
metrics (e.g. the request/second from bots vs. real users).
jmacd marked this conversation as resolved.
Show resolved Hide resolved

The SDK MUST provide the means to register Views with a `MeterProvider`. Here
are the inputs:

* The Instrument selection criteria (required), which covers:
* The `name` of the Instrument(s), with wildcard support (required).
reyang marked this conversation as resolved.
Show resolved Hide resolved
* The `name` of the Meter (optional).
* The `version` of the Meter (optional).
* The `schema_url` of the Meter (optional).
* Individual language client MAY choose to support more criteria. For example,
a strong typed language MAY support point type (e.g. allow the users to
select Instruments based on whether the underlying type is integer or
double).
* The `name` of the View (optional). If not provided, the Instrument `name`
would be used by default. This will be used as the name of the [metrics
stream](./datamodel.md#events--data-stream--timeseries).
* The configuration for the resulting [metrics
stream](./datamodel.md#events--data-stream--timeseries):
* The `description`. If not provided, the Instrument `description` would be
jmacd marked this conversation as resolved.
Show resolved Hide resolved
used by default.
* A list of `attribute keys` (optional). If not provided, all the attribute
keys will be used by default (TODO: once the Hint API is available, the
default behavior should respect the Hint if it is available).
* The `extra dimensions` which come from Baggage/Context (optional). If not
provided, no extra dimension will be used. Please note that this only
applies to [synchronous Instruments](./api.md#synchronous-instrument).
Comment on lines +126 to +128
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should leave these out of the first pass. Otherwise, we need to figure out some standard way to figure out how to define the source of the information declaratively.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by "define the source of the information"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think having this would help us to move forward (once we have the skeleton of the spec, it is easier for others to come and contribute the "meat") 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean... how do you extract it from the context? Are you providing some sort of function on the context that would give you an attribute?

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that the SDK via its MeasurementProcessor has access to the context for this purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

sure...but how do you define a view to do that work? how do you specify how to extract data from the context?

* The `aggregation` (optional) to be used. If not provided, a default
jmacd marked this conversation as resolved.
Show resolved Hide resolved
aggregation will be applied by the SDK. The default aggregation is a TODO
(e.g. first see if there is an explicitly specified aggregation from the
View, if not, see if there is a Hint, if not, fallback to the default
aggregation that is associated with the Instrument type), and for histogram,
the default buckets would be (-∞, 0], (0, 5.0], (5.0, 10.0], (10.0,
25.0], (25.0, 50.0], (50.0, 75.0], (75.0, 100.0], (100.0, 250.0], (250.0,
500.0], (500.0, 1000.0], (1000.0, +∞).
reyang marked this conversation as resolved.
Show resolved Hide resolved

The SDK SHOULD use the following logic to determine how to process an
Instrument:
reyang marked this conversation as resolved.
Show resolved Hide resolved

* Determine the `MeterProvider` which "owns" the Instrument.
* If the `MeterProvider` has no `View` registered, take the Instrument and apply
the default configuration.
* If the `MeterProvider` has one or more `View`(s) registered, for each View:
* If the Instrument could match the instrument selection criteria:
* Try to apply the View configuration. If there is an error (e.g. the View
asks for extra dimensions from the Baggage, but the Instrument is
[asynchronous](./api.md#asynchronous-instrument) which doesn't have
Context) or a conflict (e.g. the View requires to export the metrics using
a certain name, but the name is already used by another View), provide a
way to let the user know (e.g. expose [self-diagnostics
logs](../error-handling.md#self-diagnostics)).
* END.
reyang marked this conversation as resolved.
Show resolved Hide resolved

Here are some examples:

```python
# Python
'''
+------------------+
| MeterProvider |
| Meter A |
| Counter X |
| Histogram Y |
| Meter B |
| Gauge Z |
+------------------+
'''

# metrics from X and Y (reported as Foo and Bar) will be exported
meter_provider
.add_view("X")
.add_view("Foo", instrument_name="Y")
.add_view(
"Bar",
instrument_name="Y",
aggregation=HistogramAggregation(buckets=[5.0, 10.0, 25.0, 50.0, 100.0]))
.set_exporter(PrometheusExporter())
```

```python
# all the metrics will be exported using the default configuration
meter_provider.set_exporter(ConsoleExporter())
```

* Allow multiple pipelines (processors, exporters).
* Configure "Views".
* Configure timing (related to [issue
1432](https://github.com/open-telemetry/opentelemetry-specification/issues/1432)).
```python
# all the metrics will be exported using the default configuration
meter_provider
.add_view("*") # a wildcard view that matches everything
reyang marked this conversation as resolved.
Show resolved Hide resolved
.set_exporter(ConsoleExporter())
```

```python
# Counter X will be exported as cumulative sum
meter_provider
.add_view("X", aggregation=SumAggregation(CUMULATIVE))
.set_exporter(ConsoleExporter())
```

```python
# Counter X will be exported as delta sum
# Histogram Y and Gauge Z will be exported with 2 dimensions (a and b)
meter_provider
.add_view("X", aggregation=SumAggregation(DELTA))
.add_view("*", attribute_keys=["a", "b"])
.set_exporter(ConsoleExporter())
```

## MeasurementProcessor

Expand Down Expand Up @@ -69,13 +225,13 @@ active span](../trace/api.md#context-interaction)).
+------------------+
| MeterProvider | +----------------------+ +-----------------+
| Meter A | Measurements... | | Metrics... | |
| Instrument X |-----------------> MeasurementProcessor +------------> In-memory state |
| Instrument Y + | | | |
| Instrument X +-----------------> MeasurementProcessor +------------> In-memory state |
| Instrument Y | | | | |
| Meter B | +----------------------+ +-----------------+
| Instrument Z |
| ... | +----------------------+ +-----------------+
| ... | Measurements... | | Metrics... | |
| ... |-----------------> MeasurementProcessor +------------> In-memory state |
| ... +-----------------> MeasurementProcessor +------------> In-memory state |
| ... | | | | |
| ... | +----------------------+ +-----------------+
+------------------+
Expand All @@ -95,20 +251,20 @@ in the SDK:
```text
+-----------------+ +-----------------+ +-----------------------+
| | Metrics... | | Metrics... | |
> In-memory state | -----------> MetricProcessor | -----------> MetricExporter (push) |--> Another process
| In-memory state +------------> MetricProcessor +------------> MetricExporter (push) +--> Another process
| | | | | |
+-----------------+ +-----------------+ +-----------------------+

+-----------------+ +-----------------+ +-----------------------+
| | Metrics... | | Metrics... | |
> In-memory state |------------> MetricProcessor |------------> MetricExporter (pull) |--> Another process (scraper)
| In-memory state +------------> MetricProcessor +------------> MetricExporter (pull) +--> Another process (scraper)
| | | | | |
+-----------------+ +-----------------+ +-----------------------+
```

## MetricExporter

`MetricExporter` defines the interface that protocol-specific exporters must
`MetricExporter` defines the interface that protocol-specific exporters MUST
implement so that they can be plugged into OpenTelemetry SDK and support sending
of telemetry data.

Expand Down