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.

Sign up for GitHub

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

Jump to bottom

Refactor aggregations #2308

Merged
merged 36 commits into from
Jan 11, 2022
Merged

Refactor aggregations #2308

merged 36 commits into from
Jan 11, 2022

Conversation

ocelotl
Copy link
Contributor

@ocelotl ocelotl commented Dec 2, 2021 •
edited by codeboten
Loading

Fixes #2305

@ocelotl ocelotl requested a review from a team December 2, 2021 02:41
@ocelotl ocelotl mentioned this pull request Dec 2, 2021
Closed
@ocelotl ocelotl added Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary Skip Changelog PRs that do not require a CHANGELOG.md entry labels Dec 2, 2021
codeboten
codeboten reviewed Dec 2, 2021
View reviewed changes
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

This PR says to review the other PR which then points to another PR to review first, but both of those are still in draft. Are they ready for review?

@ocelotl ocelotl marked this pull request as draft December 2, 2021 17:16
@ocelotl ocelotl mentioned this pull request Dec 3, 2021
Merged
@ocelotl ocelotl marked this pull request as ready for review December 6, 2021 19:45
@ocelotl
Copy link
Contributor Author

ocelotl commented Dec 6, 2021

This PR says to review the other PR which then points to another PR to review first, but both of those are still in draft. Are they ready for review?

They are now ✌️

@ocelotl ocelotl added the metrics label Dec 7, 2021
@ocelotl ocelotl marked this pull request as draft December 8, 2021 21:14
@ocelotl
Copy link
Contributor Author

ocelotl commented Dec 8, 2021

Marking this one as draft while #2297 gets updated and reviewed

@ocelotl ocelotl force-pushed the issue_2305 branch 2 times, most recently from a283e65 to 6786462 Compare December 15, 2021 00:17
@ocelotl ocelotl marked this pull request as ready for review December 15, 2021 00:17
@ocelotl ocelotl requested a review from codeboten December 15, 2021 00:18
@ocelotl
Copy link
Contributor Author

ocelotl commented Dec 15, 2021

Ready for review ✌️

@ocelotl ocelotl force-pushed the issue_2305 branch 2 times, most recently from 621fbe8 to 264ab5d Compare December 15, 2021 00:26
This was referenced Dec 15, 2021
Closed
Closed
aabmass
aabmass reviewed Dec 15, 2021
View reviewed changes
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Outdated Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Outdated Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Outdated Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py Outdated Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py Outdated Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py Outdated Show resolved Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Outdated Show resolved Hide resolved
@ocelotl ocelotl requested a review from aabmass December 17, 2021 02:24
ocelotl and others added 21 commits January 10, 2022 18:19
@ocelotl
Adding AggregationTemporality enum
628a067
@ocelotl
Removing Generic
d0787db
@ocelotl
Remove types from aggregations
6965cad
@ocelotl
Fix lint
b1fd1fa
@ocelotl
Fix histogram test cases
e2c4bad
@ocelotl
Revert "Removing Generic"
c313eb4 
This reverts commit 132a0f7.
@ocelotl
Revert "Remove types from aggregations"
3712d38 
This reverts commit 49b3d2a.
@ocelotl
Fix self._value
c0d7f56
@ocelotl @aabmass
Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
74d6c8d 
Co-authored-by: Aaron Abbott <aaronabbott@google.com>
@ocelotl @aabmass
Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
59bd4aa 
Co-authored-by: Aaron Abbott <aaronabbott@google.com>
@ocelotl @aabmass
Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
e0a78db 
Co-authored-by: Aaron Abbott <aaronabbott@google.com>
@ocelotl @aabmass
Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
a347bb4 
Co-authored-by: Aaron Abbott <aaronabbott@google.com>
@ocelotl @srikanthccv
Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
8595ad8 
Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
@ocelotl
Use self._boundaries
eb67095
@ocelotl
Remove aggregation temporality prefix
38ebe4a
@ocelotl
Add MonotonicitySensitiveAggregation
66ce908
@ocelotl
Add collect test cases
6c84c66
@ocelotl
Use assertEqual instead
8ab680d
@ocelotl
Add delay
0ee91c4
@ocelotl
Use Aware instead of Sensitive
3c68beb
@ocelotl
Rename is_monotonic
c40bfd6
@lzchen lzchen merged commit 6b655ce into open-telemetry:main Jan 11, 2022
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

@srikanthccv srikanthccv srikanthccv approved these changes

@aabmass aabmass aabmass approved these changes

@codeboten codeboten codeboten approved these changes

Assignees

@ocelotl ocelotl

Labels
Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary metrics 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.

Refactor aggregations
5 participants
@ocelotl @codeboten @aabmass @lzchen @srikanthccv