Skip to content

Implement SynchronousMeasurementConsumer#2388

Merged
lzchen merged 5 commits intoopen-telemetry:mainfrom
aabmass:2328-measurementconsumer
Jan 19, 2022
Merged

Implement SynchronousMeasurementConsumer#2388
lzchen merged 5 commits intoopen-telemetry:mainfrom
aabmass:2328-measurementconsumer

Conversation

@aabmass
Copy link
Copy Markdown
Member

@aabmass aabmass commented Jan 18, 2022

Description

  • Implemented the SynchronousMeasurementConsumer from the design.
  • Added a stub implementation and TODO for implementing the MetricReaderStorage (Implement MetricReaderStorage #2378)
  • Added a small (internal) SdkConfiguration dataclass for passing through SDK configuration from MeterProvider. This avoids having to access private methods/variables on the MeterProvider class.

Fixes #2328

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Added test case mocking out dependencies.

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added

@aabmass aabmass added sdk Affects the SDK package. metrics labels Jan 18, 2022
@aabmass aabmass force-pushed the 2328-measurementconsumer branch 2 times, most recently from 933c339 to 1b41260 Compare January 18, 2022 18:46
aabmass
aabmass commented Jan 18, 2022
View reviewed changes
Comment thread opentelemetry-sdk/tests/metrics/test_measurement_consumer.py
@aabmass aabmass force-pushed the 2328-measurementconsumer branch from 1b41260 to 4ba0f61 Compare January 18, 2022 18:54
@aabmass aabmass force-pushed the 2328-measurementconsumer branch from 4ba0f61 to 7609465 Compare January 18, 2022 18:54
@aabmass aabmass marked this pull request as ready for review January 18, 2022 18:54
@aabmass aabmass requested a review from a team January 18, 2022 18:55
@aabmass aabmass added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 18, 2022
ocelotl
ocelotl reviewed Jan 18, 2022
View reviewed changes
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/_metrics/sdk_configuration.py
ocelotl
ocelotl suggested changes Jan 18, 2022
View reviewed changes
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement_consumer.py Outdated
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py
lzchen
lzchen reviewed Jan 18, 2022
View reviewed changes
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement_consumer.py
lzchen
lzchen reviewed Jan 18, 2022
View reviewed changes
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement_consumer.py Outdated
@aabmass aabmass requested review from lzchen and ocelotl January 18, 2022 21:20
@ocelotl ocelotl mentioned this pull request Jan 18, 2022
Closed
ocelotl
ocelotl suggested changes Jan 18, 2022
View reviewed changes
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py Outdated
Comment thread opentelemetry-sdk/tests/metrics/test_measurement_consumer.py Outdated
@ocelotl ocelotl added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Jan 19, 2022
@lzchen lzchen merged commit fb60538 into open-telemetry:main Jan 19, 2022
@aabmass aabmass deleted the 2328-measurementconsumer branch January 19, 2022 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lzchen lzchen lzchen approved these changes

+1 more reviewer

@ocelotl ocelotl ocelotl approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary metrics sdk Affects the SDK package. Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement DefaultMeasurementConsumer

3 participants

@aabmass @ocelotl @lzchen