Skip to content

[Metrics SDK] Add support for Pull Metric Reader#1701

Merged
esigo merged 5 commits intoopen-telemetry:mainfrom
esigo:1561-Pull-Metric-Exporter
Oct 21, 2022
Merged

[Metrics SDK] Add support for Pull Metric Reader#1701
esigo merged 5 commits intoopen-telemetry:mainfrom
esigo:1561-Pull-Metric-Exporter

Conversation

@esigo
Copy link
Copy Markdown
Member

@esigo esigo commented Oct 20, 2022 •
edited
Loading

Fixes #1561 (issue)

Changes

Please provide a brief description of the changes here.

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

@esigo esigo requested a review from a team October 20, 2022 19:14
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 20, 2022 •
edited
Loading

Codecov Report

Merging #1701 (c45f30e) into main (15a148e) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1701   +/-   ##
=======================================
  Coverage   86.37%   86.37%           
=======================================
  Files         169      169           
  Lines        5156     5156           
=======================================
  Hits         4453     4453           
  Misses        703      703
Impacted Files Coverage Δ
...opentelemetry/sdk/metrics/state/metric_collector.h 100.00% <ø> (ø)
...metrics/export/periodic_exporting_metric_reader.cc 73.92% <ø> (ø)
exporters/ostream/test/ostream_metric_test.cc 100.00% <100.00%> (ø)
...e/opentelemetry/sdk/metrics/push_metric_exporter.h 100.00% <100.00%> (ø)
sdk/src/metrics/meter_context.cc 89.66% <100.00%> (ø)
sdk/src/metrics/meter_provider.cc 90.91% <100.00%> (ø)
sdk/src/metrics/state/metric_collector.cc 92.00% <100.00%> (ø)

@lalitb
Copy link
Copy Markdown
Member

lalitb commented Oct 20, 2022 •
edited
Loading

This looks good. Just to add

If the pull exporter is modeled as MetricReader, implementors MAY name the MetricExporter interface as PushMetricExporter to prevent naming confusion.

  • Later we can add some doc guidelines/examples on how the users can create custom pull exporter using MetricReader.

Thanks for working on it.

@esigo esigo changed the title FOR DISCUSSION [Metrics SDK] Add support for Pull Metric Exporter [Metrics SDK] Add support for Pull Metric Reader Oct 21, 2022
lalitb
lalitb approved these changes Oct 21, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks

@esigo esigo added the metrics label Oct 21, 2022
@esigo esigo merged commit 70777ef into open-telemetry:main Oct 21, 2022
@esigo esigo deleted the 1561-Pull-Metric-Exporter branch October 21, 2022 21:21
@esigo esigo mentioned this pull request Oct 28, 2022
Merged
ays7 added a commit to ays7/opentelemetry-cpp that referenced this pull request Oct 29, 2022
@ays7
Merge commit '9acde87b08b225ce511fa8a20c6cba14f2921518' into ays/thre…
456a0e7 
…ad-local-stack

* commit '9acde87b08b225ce511fa8a20c6cba14f2921518': (36 commits)
  Prepare v1.7.0 release with Metrics API/SDK GA. (open-telemetry#1721)
  Fix: 1712 -  Validate Instrument meta data (name, unit, description) (open-telemetry#1713)
  Document libthrift 0.12.0 doesn't work with Jaeger exporter (open-telemetry#1714)
  Fix:1674, Add Monotonic Property to Sum Aggregation, and unit tests for Up Down Counter (open-telemetry#1675)
  [Metrics SDK] Move Metrics Exemplar processing behind feature flag (open-telemetry#1710)
  [Metrics API/SDK] Change Meter API/SDK to return nostd::unique_ptr for Sync Instruments (open-telemetry#1707)
  [Metrics] Switch to explicit 64 bit integers (open-telemetry#1686)
  Add metrics e2e test to asan & tsan CI (open-telemetry#1670)
  Add otlp-grpc example bazel (open-telemetry#1708)
  [Metrics SDK] Add support for Pull Metric Reader (open-telemetry#1701)
  Fix debug log of OTLP HTTP exporter and ES log exporter (open-telemetry#1703)
  [SEMANTIC CONVENTIONS] Upgrade to version 1.14.0 (open-telemetry#1697)
  Fix a potential precision loss on integer in ReservoirCellIndexFor (open-telemetry#1696)
  fix Histogram crash (open-telemetry#1685)
  Fix:1676 Segfault when short export period is used for metrics  (open-telemetry#1682)
  Add timeout support to MeterContext::ForceFlush (open-telemetry#1673)
  Add CMake OTELCPP_MAINTAINER_MODE (open-telemetry#1650)
  [DOCS] - Minor updates to OStream Metrics exporter documentation (open-telemetry#1679)
  Fix:open-telemetry#1575 API Documentation for Metrics SDK and API (open-telemetry#1678)
  Fixes open-telemetry#249 (open-telemetry#1677)
  ...
yxue pushed a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022
@esigo @yxue
[Metrics SDK] Add support for Pull Metric Reader (open-telemetry#1701)
c6552dc
yxue pushed a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022
@esigo @yxue
[Metrics SDK] Add support for Pull Metric Reader (open-telemetry#1701)
e640b89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lalitb lalitb lalitb approved these changes

Assignees

No one assigned

Labels

metrics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Metrics SDK] Add support for Pull Metric Exporter

2 participants

@esigo @lalitb
⚡