Enable metric collection from MetricReader #1241

lalitb · 2022-03-03T07:52:24Z

Changes

This PR enables MetricReader to invoke metric collection. The flow is drawn (with red) in block diagram.

MetricReader : Acts as an interface to start the metric collection.
MetricCollector - This is an internal class to perform actual metric collection, and abstracts MetricReader from any implementation changes.

Flow

MetricReader::Collect() -> MetricCollector::Collect() -> Meter::Collect() -> AggregationStorage::Collect()

MetricReader::Collect(): Invokes MetricCollector::Collect()
MetricCollector::Collect() - Fetches all the configured Meters from MeterContext, and invoke Meter::Collect() on each of them.
Meter::Collect() - Fetches the configured Aggregation storages for each created instrument ( based on View configuration ), and invoke Collect() on them.

MetricReader and CollectorHandle are base classes for MetricCollector and are meant to be opaque handles over MetricCollector.

CHANGELOG.md updated for non-trivial changes
Unit tests have been added
Changes in public API reviewed

codecov · 2022-03-03T08:00:43Z

Codecov Report

Merging #1241 (1b6af65) into main (6bd2f96) will decrease coverage by 0.39%.
The diff coverage is 41.06%.

@@            Coverage Diff             @@
##             main    #1241      +/-   ##
==========================================
- Coverage   92.37%   91.99%   -0.38%     
==========================================
  Files         203      205       +2     
  Lines        7338     7395      +57     
==========================================
+ Hits         6778     6802      +24     
- Misses        560      593      +33

Impacted Files	Coverage Δ
...nclude/opentelemetry/sdk/metrics/metric_exporter.h	`0.00% <ø> (-50.00%)`	⬇️
...e/opentelemetry/sdk/metrics/state/metric_storage.h	`100.00% <ø> (ø)`
...ntelemetry/sdk/metrics/state/sync_metric_storage.h	`67.75% <0.00%> (ø)`
...clude/opentelemetry/sdk/metrics/sync_instruments.h	`100.00% <ø> (ø)`
sdk/src/metrics/meter.cc	`7.90% <0.00%> (-0.92%)`	⬇️
sdk/src/metrics/sync_instruments.cc	`100.00% <ø> (ø)`
sdk/src/metrics/metric_reader.cc	`22.86% <22.86%> (ø)`
sdk/test/metrics/meter_provider_sdk_test.cc	`75.68% <25.00%> (-2.70%)`	⬇️
sdk/src/metrics/meter_context.cc	`37.50% <27.28%> (+1.14%)`	⬆️
.../include/opentelemetry/sdk/metrics/metric_reader.h	`50.00% <50.00%> (-50.00%)`	⬇️
... and 10 more

…p into metric-collector

sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h

sdk/src/metrics/metric_reader.cc

sdk/include/opentelemetry/sdk/metrics/metric_reader.h

sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h

sdk/src/metrics/meter_context.cc

sdk/include/opentelemetry/sdk/metrics/meter_context.h

sdk/src/metrics/state/metric_collector.cc

sdk/test/metrics/metric_reader_test.cc

sdk/include/opentelemetry/sdk/metrics/meter_context.h

esigo

LGTM
Thanks for the PR

lalitb added 5 commits March 1, 2022 23:53

collector and reader

c08cf38

fix format, and memory corruption

c729c6a

changes for metric flow

f692e15

fix bazel build

ffb54ee

fix ostream exporter

8ecf729

lalitb requested a review from a team March 3, 2022 07:52

Merge branch 'main' into metric-collector

18e4f27

lalitb added 2 commits March 3, 2022 10:35

build error on windows

ad471d0

Merge branch 'metric-collector' of github.com:lalitb/opentelemetry-cp…

9cc53a4

…p into metric-collector

esigo reviewed Mar 4, 2022

View reviewed changes

sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h Show resolved Hide resolved