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

Add Census resource API and implementation. #6774

Merged
merged 11 commits into from
Aug 2, 2016
Merged

Add Census resource API and implementation. #6774

merged 11 commits into from
Aug 2, 2016

Conversation

a-veitch
Copy link
Contributor

@a-veitch a-veitch commented Jun 3, 2016

Changes in this PR, in approximate order of significance:

  • Add core Resource API, implementation and tests.
  • Rename two of the Census proto messages to better reflect existing naming in other google systems.
  • Add base resource definitions for latency resources, and add them when census is initialized.
  • Remove the unimplemented/placeholder census stats API functions from census.h. Will re-add as they are implemented.
  • Extend the census initialization function to return features enabled, with corresponding cleanup in implementation.

@a-veitch
Copy link
Contributor Author

a-veitch commented Jun 3, 2016

Remaining test failures look unrelated (macos port server failure).

@therc therc mentioned this pull request Jun 5, 2016
Closed
bogdandrutu
bogdandrutu reviewed Jun 6, 2016
View reviewed changes
src/core/ext/census/resource.h
*
*/

// Census-internal resource definition and manipluation functions.
Copy link

Choose a reason for hiding this comment

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

C89 comments in the header?

@bogdandrutu
Copy link

Please fix tests.

@a-veitch
Copy link
Contributor Author

a-veitch commented Jun 7, 2016

AFAIKT, all the test failures are unrelated - macos still failing on port_server (basic tests) and some socket failure in one of the interop tests. PTAL.

bogdandrutu
bogdandrutu reviewed Jun 27, 2016
View reviewed changes
include/grpc/census.h
arbitrary number of views by which it will be broken down.
* Resource: Users record measurements for a single resource. Examples
include RPC latency, CPU seconds consumed, and bytes transmitted.
* Aggregation: An aggregation of a set of measurements. Census supports the
Copy link

Choose a reason for hiding this comment

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

Are we not going to support SUM?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we not going to support SUM?

Sums are part of the Distribution aggregation.

@kpayson64 kpayson64 merged commit 329d647 into grpc:master Aug 2, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@bogdandrutu bogdandrutu

Labels
area/Census cla: yes disposition/ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@a-veitch @bogdandrutu @googlebot @kpayson64