ostream metrics example by esigo · Pull Request #1312 · open-telemetry/opentelemetry-cpp

esigo · 2022-04-02T18:58:15Z

related to #1092 (issue)

Changes

For testing end-end metrics
Adds ostream metrics example and fixes Export().

For significant contributions please make sure you have completed the following items:

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

codecov · 2022-04-02T19:06:17Z

Codecov Report

Merging #1312 (454a2c9) into main (d72dcb6) will decrease coverage by 0.35%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1312      +/-   ##
==========================================
- Coverage   91.70%   91.36%   -0.34%     
==========================================
  Files         217      217              
  Lines        7842     7842              
==========================================
- Hits         7191     7164      -27     
- Misses        651      678      +27

Impacted Files	Coverage Δ
...nclude/opentelemetry/sdk/metrics/metric_exporter.h	`50.00% <ø> (ø)`
...entelemetry/sdk/metrics/view/instrument_selector.h	`100.00% <ø> (ø)`
...metrics/export/periodic_exporting_metric_reader.cc	`43.19% <0.00%> (-34.09%)`	⬇️
...t/metrics/periodic_exporting_metric_reader_test.cc	`64.52% <0.00%> (-29.03%)`	⬇️
sdk/src/metrics/metric_reader.cc	`55.56% <0.00%> (-8.33%)`	⬇️

lalitb · 2022-04-03T04:58:36Z


 sdk::common::ExportResult OStreamMetricExporter::Export(
-    const nostd::span<std::unique_ptr<opentelemetry::sdk::metrics::MetricData>> &records) noexcept
+    const sdk::metrics::MetricData &data) noexcept


This may need further changes, based on the review for #1299, as it adds Resources and Instrumentation Info to Metric Data to be exported.

#1299 is merged. you can modify the PR to use ResourceMetrics instead of MetricData.

@esigo - Just want to check if you want this to be reviewed now, as I was thinking to review it once it incorporates changes for ResourceMetrics as mentioned above :)

Hi @lalitb, I'm rewriting the unit tests, should be done by tomorrow. I'll ping you when I'm done.
Thanks

Thanks, @esigo. Just wanted to ensure that I am not delaying the review comments in case you are expecting it to be reviewed :)

@lalitb this should be ready for review, thanks :)

lalitb · 2022-04-11T12:09:51Z

+  double_counter->Add(3.14);
+  std::this_thread::sleep_for(std::chrono::milliseconds(500));
+  double_counter->Add(23.5);
+  std::this_thread::sleep_for(std::chrono::milliseconds(5000));


nit - should we add small metrics instrumentation under (example/common) similar to how we do for traces and logs?

thanks, added.

lalitb · 2022-04-11T12:13:15Z

+  auto duration = time_stamp.time_since_epoch().count();
+
+  std::chrono::nanoseconds dur(duration);
+  std::chrono::time_point<std::chrono::system_clock> dt(dur);


nit - need to check, but I think this can be directly obtained by calling time_stamp()?

I try to print time in human readable format here. I didn't find any function similar.

Not sure if something wrong with GitHub. I pasted a comment here, it was displayed multiple times and then when I removed one occurrence all of them removed :) But it was not that important, was thinking something to avoid couple of lines of code (so can be ignored):

std::string timeToString(opentelemetry::common::SystemTimestamp time_stamp) { std::time_t epoch_time = std::chrono::system_clock::to_time_t(time_stamp()); return std::ctime(&epoch_time); }

thanks, added your version.

lalitb · 2022-04-12T02:12:20Z

+#include "opentelemetry/sdk/metrics/meter.h"
+#include "opentelemetry/sdk/metrics/meter_provider.h"
+#include "opentelemetry/sdk/version/version.h"
+


instrumentation library shouldn't include any sdk component. Can we remove these includes ?

thanks, removed.

lalitb

Thanks. LGTM.

* install sdk config (#1273) * Bump actions/cache from 2 to 3 (#1277) * Add owent as an Approver (#1276) * add owent as reviewer * fix order * Disable benchmark action failure (#1284) * metrics exemplar round 1 (#1264) * [Metrics SDK] - fix spelling (AggregationTemporarily to AggregationTemporality) (#1288) * fix compilation error with protobuf 3.5 (#1289) * Fix span SetAttribute crash (#1283) * Synchronous Metric collection (Delta , Cumulative) (#1265) * Rename `http_client_curl` to `opentelemetry_http_client_curl` (#1301) Signed-off-by: owent <admin@owent.net> * Don't show coverage annotation for pull requests (#1304) * Implement periodic exporting metric reader (#1286) * Add `async-changes` branch to pull_request of github action (#1309) Signed-off-by: owent <admin@owent.net> * Add InstrumentationInfo and Resource to the metrics data to be exported. (#1299) * Excempt should be applied on issue instead of PR (#1316) * Bump codecov/codecov-action from 2.1.0 to 3 (#1318) * Move public definitions into `opentelemetry_api`. (#1314) Signed-off-by: owent <admin@owent.net> * Add building test without RTTI (#1294) * Remove implicitly deleted default constructor (#1267) Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * [ETW Exporter] - ETW provider handle cleanup (#1322) * Bump actions/stale from 4 to 5 (#1323) * ostream metrics example (#1312) * Prepare v1.3.0 release (#1324) * Update yield logic for ARM processor (#1325) * Fix for #1292 (#1326) * Implement Merge and Diff operation for Histogram Aggregation (#1303) * fix metrics compiler warnings (#1328) * Replace deprecated googletest API (#1327) * Remove redundant tail / in CMake install (#1329) * dependencies image as artifact (#1333) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * metrics histogram example (#1330) * Link `opentelemetry_ext` with `opentelemetry_api` (#1336) * ostream metrics cmake (#1344) * Fix conflicts Signed-off-by: owent <admin@owent.net> * Using clang-format-10 to format codes(clang-format-14 has a different output) Signed-off-by: owent <admin@owent.net> Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Ben Landrum <71329856+benlandrum@users.noreply.github.com> Co-authored-by: jmanjon <67091862+juandemanjon@users.noreply.github.com>

* install sdk config (#1273) * Bump actions/cache from 2 to 3 (#1277) * Add owent as an Approver (#1276) * add owent as reviewer * fix order * Disable benchmark action failure (#1284) * metrics exemplar round 1 (#1264) * [Metrics SDK] - fix spelling (AggregationTemporarily to AggregationTemporality) (#1288) * fix compilation error with protobuf 3.5 (#1289) * Fix span SetAttribute crash (#1283) * Synchronous Metric collection (Delta , Cumulative) (#1265) * Rename `http_client_curl` to `opentelemetry_http_client_curl` (#1301) Signed-off-by: owent <admin@owent.net> * Don't show coverage annotation for pull requests (#1304) * Implement periodic exporting metric reader (#1286) * Add `async-changes` branch to pull_request of github action (#1309) Signed-off-by: owent <admin@owent.net> * Add InstrumentationInfo and Resource to the metrics data to be exported. (#1299) * Excempt should be applied on issue instead of PR (#1316) * Bump codecov/codecov-action from 2.1.0 to 3 (#1318) * Move public definitions into `opentelemetry_api`. (#1314) Signed-off-by: owent <admin@owent.net> * Add building test without RTTI (#1294) * Remove implicitly deleted default constructor (#1267) Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * [ETW Exporter] - ETW provider handle cleanup (#1322) * Bump actions/stale from 4 to 5 (#1323) * ostream metrics example (#1312) * Prepare v1.3.0 release (#1324) * Update yield logic for ARM processor (#1325) * Fix for #1292 (#1326) * Implement Merge and Diff operation for Histogram Aggregation (#1303) * fix metrics compiler warnings (#1328) * Replace deprecated googletest API (#1327) * Remove redundant tail / in CMake install (#1329) * dependencies image as artifact (#1333) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * metrics histogram example (#1330) * Link `opentelemetry_ext` with `opentelemetry_api` (#1336) * ostream metrics cmake (#1344) * prometheus exporter (#1331) * remove exporter registration to meter provider (#1350) * Bump github/codeql-action from 1 to 2 (#1351) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add explicit type cast in baggage UrlDecode (#1353) * Fix scalar delete against array (#1356) * conditional include for codecvt header (#1355) * Add missing include guard (#1357) * Use latest TraceLoggingDynamic.h (#1354) * prometheus example (#1332) * Fix output time in metrics OStream exporter (#1346) * Simplify SDK Configuration: Use View with default aggregation if no matching View is configured (#1358) * Fix class member initialization order (#1360) * codecov ignore (#1364) * export opentelemetry_otlp_recordable (#1365) * Disable test on prometheus-cpp which not need (#1363) * Enable metric collection for Async Instruments - Delta and Cumulative (#1334) * fix baggage propagation for empty/invalid baggage context (#1367) * Fix empty tracestate header propagation (#1373) * Bump docker/setup-qemu-action from 1 to 2 (#1375) * Add noexcept/const qualifier at missing places for Trace API. (#1374) * fix noxcept * fix etw * Bump docker/build-push-action from 2 to 3 (#1377) * Bump docker/setup-buildx-action from 1 to 2 (#1376) * [Metrics SDK] Remove un-necessary files. (#1379) * reuse temporal metric storage for sync storage (#1369) * Prometheus exporter meters and instrument name (#1378) * Fix sharing resource in batched exported spans (#1386) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix: missing link to nlohmann_json (#1390) * Getting started document using ostream exporter (#1394) Co-authored-by: Reiley Yang <reyang@microsoft.com> * Connect async storage with async instruments (#1388) * get span_id from context when Logger::Log received invalid span_id (#1398) * Alpine image (#1382) * Upgrade proto to v0.17.0, update log data model (#1383) * Prepare v1.4.0 release (#1404) * Fix vcpkg package name in doc (#1392) * Document Getting Started with Prometheus and Grafana (#1396) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix OTEL_INTERNAL_LOG_INFO (#1407) * fix: WaitOnSocket select error when sockfd above FD_SETSIZE (#1410) * [BUILD] fix nlohmann_json's (third party) include dir (#1415) * [Metrics API/SDK] - Pass state to async callback function. (#1408) * Copy string_view passed to ETW exporter in PropertyVariant (#1425) * Fix ETW log exporter header inclusion (#1426) * [Metrics SDK] Only record non-negative / finite / Non-NAN histogram values (#1427) * validate histogram value * handle Nan * add changelog * divide by 0 error on windows * fix markdown lint * Fix global log handle symbols when using dlopen (#1420) * Add attributes/dimensions to metrics ostream exporter (#1400) * Log current timestamp instead of epoch time (#1434) * install sdk-config.h (#1419) * Fix GettingStarted documentation for Jaeger HTTP exporter (#1347) (#1439) * fix histogram (#1440) * Upgrade nlohmann_json to 3.10.5 (#1438) (#1441) * Fixed broken link to OpenTelemetry.io (#1445) (#1446) Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Ben Landrum <71329856+benlandrum@users.noreply.github.com> Co-authored-by: jmanjon <67091862+juandemanjon@users.noreply.github.com> Co-authored-by: Zsolt Bölöny <bolony.zsolt@gmail.com> Co-authored-by: Leo Di Donato <leodidonato@gmail.com> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <labhas@microsoft.com> Co-authored-by: Hamed Mansouri <hamed0381@gmail.com> Co-authored-by: ztao <t@taozj.org> Co-authored-by: Flier Lu <flier@users.noreply.github.com> Co-authored-by: Marc Alff <marc.alff@free.fr> Co-authored-by: Marc Alff <marc.alff@oracle.com>

* install sdk config (#1273) * Bump actions/cache from 2 to 3 (#1277) * Add owent as an Approver (#1276) * add owent as reviewer * fix order * Disable benchmark action failure (#1284) * metrics exemplar round 1 (#1264) * [Metrics SDK] - fix spelling (AggregationTemporarily to AggregationTemporality) (#1288) * fix compilation error with protobuf 3.5 (#1289) * Fix span SetAttribute crash (#1283) * Synchronous Metric collection (Delta , Cumulative) (#1265) * Rename `http_client_curl` to `opentelemetry_http_client_curl` (#1301) Signed-off-by: owent <admin@owent.net> * Don't show coverage annotation for pull requests (#1304) * Implement periodic exporting metric reader (#1286) * Add `async-changes` branch to pull_request of github action (#1309) Signed-off-by: owent <admin@owent.net> * Add InstrumentationInfo and Resource to the metrics data to be exported. (#1299) * Excempt should be applied on issue instead of PR (#1316) * Bump codecov/codecov-action from 2.1.0 to 3 (#1318) * Move public definitions into `opentelemetry_api`. (#1314) Signed-off-by: owent <admin@owent.net> * Add building test without RTTI (#1294) * Remove implicitly deleted default constructor (#1267) Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * [ETW Exporter] - ETW provider handle cleanup (#1322) * Bump actions/stale from 4 to 5 (#1323) * ostream metrics example (#1312) * Prepare v1.3.0 release (#1324) * Update yield logic for ARM processor (#1325) * Fix for #1292 (#1326) * Implement Merge and Diff operation for Histogram Aggregation (#1303) * fix metrics compiler warnings (#1328) * Replace deprecated googletest API (#1327) * Remove redundant tail / in CMake install (#1329) * dependencies image as artifact (#1333) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * metrics histogram example (#1330) * Link `opentelemetry_ext` with `opentelemetry_api` (#1336) * ostream metrics cmake (#1344) * prometheus exporter (#1331) * remove exporter registration to meter provider (#1350) * Bump github/codeql-action from 1 to 2 (#1351) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add explicit type cast in baggage UrlDecode (#1353) * Fix scalar delete against array (#1356) * conditional include for codecvt header (#1355) * Add missing include guard (#1357) * Use latest TraceLoggingDynamic.h (#1354) * prometheus example (#1332) * Fix output time in metrics OStream exporter (#1346) * Simplify SDK Configuration: Use View with default aggregation if no matching View is configured (#1358) * Fix class member initialization order (#1360) * codecov ignore (#1364) * export opentelemetry_otlp_recordable (#1365) * Disable test on prometheus-cpp which not need (#1363) * Enable metric collection for Async Instruments - Delta and Cumulative (#1334) * fix baggage propagation for empty/invalid baggage context (#1367) * Fix empty tracestate header propagation (#1373) * Bump docker/setup-qemu-action from 1 to 2 (#1375) * Add noexcept/const qualifier at missing places for Trace API. (#1374) * fix noxcept * fix etw * Bump docker/build-push-action from 2 to 3 (#1377) * Bump docker/setup-buildx-action from 1 to 2 (#1376) * [Metrics SDK] Remove un-necessary files. (#1379) * reuse temporal metric storage for sync storage (#1369) * Prometheus exporter meters and instrument name (#1378) * Fix sharing resource in batched exported spans (#1386) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix: missing link to nlohmann_json (#1390) * Getting started document using ostream exporter (#1394) Co-authored-by: Reiley Yang <reyang@microsoft.com> * Connect async storage with async instruments (#1388) * get span_id from context when Logger::Log received invalid span_id (#1398) * Alpine image (#1382) * Upgrade proto to v0.17.0, update log data model (#1383) * Prepare v1.4.0 release (#1404) * Fix vcpkg package name in doc (#1392) * Document Getting Started with Prometheus and Grafana (#1396) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix OTEL_INTERNAL_LOG_INFO (#1407) * fix: WaitOnSocket select error when sockfd above FD_SETSIZE (#1410) * [BUILD] fix nlohmann_json's (third party) include dir (#1415) * [Metrics API/SDK] - Pass state to async callback function. (#1408) * Copy string_view passed to ETW exporter in PropertyVariant (#1425) * Fix ETW log exporter header inclusion (#1426) * [Metrics SDK] Only record non-negative / finite / Non-NAN histogram values (#1427) * validate histogram value * handle Nan * add changelog * divide by 0 error on windows * fix markdown lint * Fix global log handle symbols when using dlopen (#1420) * Add attributes/dimensions to metrics ostream exporter (#1400) * Log current timestamp instead of epoch time (#1434) * install sdk-config.h (#1419) * Fix GettingStarted documentation for Jaeger HTTP exporter (#1347) (#1439) * fix histogram (#1440) * Upgrade nlohmann_json to 3.10.5 (#1438) (#1441) * Fixed broken link to OpenTelemetry.io (#1445) (#1446) * Fix variables inizialization (#1430) * Remove `AsyncBatch*Processor`, implement [opentelemetry-specification#2452](open-telemetry/opentelemetry-specification#2452) Signed-off-by: WenTao Ou <admin@owent.net> * Remove invalid changelog Signed-off-by: WenTao Ou <admin@owent.net> * Fix compiling problems without `ENABLE_ASYNC_EXPORT` Signed-off-by: WenTao Ou <admin@owent.net> * Always return `ExportResult::kSuccess` Signed-off-by: WenTao Ou <admin@owent.net> Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Ben Landrum <71329856+benlandrum@users.noreply.github.com> Co-authored-by: jmanjon <67091862+juandemanjon@users.noreply.github.com> Co-authored-by: Zsolt Bölöny <bolony.zsolt@gmail.com> Co-authored-by: Leo Di Donato <leodidonato@gmail.com> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <labhas@microsoft.com> Co-authored-by: Hamed Mansouri <hamed0381@gmail.com> Co-authored-by: ztao <t@taozj.org> Co-authored-by: Flier Lu <flier@users.noreply.github.com> Co-authored-by: Marc Alff <marc.alff@free.fr> Co-authored-by: Marc Alff <marc.alff@oracle.com> Co-authored-by: univisionsrl <31103417+univisionsrl@users.noreply.github.com>

ostream metrics example

8d1bf2c

esigo requested a review from a team April 2, 2022 18:58

lalitb reviewed Apr 3, 2022

View reviewed changes

esigo and others added 7 commits April 5, 2022 21:11

Merge branch 'main' into ostream-metrics-example

8c6ea1d

Merge branch 'main' into ostream-metrics-example

04afa7f

Merge branch 'main' into ostream-metrics-example

dcb1c5d

Merge branch 'main' into ostream-metrics-example

e51b14b

use ResourceMetrics

55aa2d9

fix CI

7aa6a14

RTTI CI

7b89ff9

lalitb reviewed Apr 11, 2022

View reviewed changes

Comment thread exporters/ostream/test/ostream_metric_test.cc

lalitb reviewed Apr 11, 2022

View reviewed changes

esigo and others added 3 commits April 11, 2022 20:02

comments

f9e42c6

fix CI

35cb208

Merge branch 'main' into ostream-metrics-example

34e8b8e

lalitb reviewed Apr 12, 2022

View reviewed changes

comments

454a2c9

lalitb approved these changes Apr 12, 2022

View reviewed changes

ThomsonTan reviewed Apr 12, 2022

View reviewed changes

Comment thread sdk/include/opentelemetry/sdk/metrics/view/instrument_selector.h

ThomsonTan approved these changes Apr 12, 2022

View reviewed changes

ThomsonTan merged commit 755f109 into open-telemetry:main Apr 12, 2022

esigo deleted the ostream-metrics-example branch April 12, 2022 18:14

lalitb mentioned this pull request Apr 20, 2022

Metrics SDK: Add Metrics Exporter #1092

Closed

Uh oh!

Conversation

esigo commented Apr 2, 2022

Changes

Uh oh!

codecov Bot commented Apr 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb Apr 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Apr 2, 2022 •

edited

Loading

lalitb Apr 8, 2022 •

edited

Loading

lalitb Apr 11, 2022 •

edited

Loading

lalitb Apr 12, 2022 •

edited

Loading