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.

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

A66 and A78 update: Add value type (integer/floating-point) to metrics #432

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yashykt
Copy link
Member

@yashykt yashykt commented May 12, 2024

No description provided.

@yashykt yashykt requested a review from markdroth May 12, 2024 19:37
@markdroth markdroth changed the title Add value type (integer/floating-point) to metrics A66 and A78 update: Add value type (integer/floating-point) to metrics Jun 5, 2024
* Implemented in: <language, ...>
* Last updated: 2024-04-08
* Discussion at: https://groups.google.com/g/grpc-io/c/A2Mqz8OMDys
## A78: gRPC OTel Metrics for WRR, Pick First, and XdsClient
Copy link
Member

Choose a reason for hiding this comment

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

Can you please undo the reformatting here? This makes it really hard to see what's actually changed.

@@ -406,22 +407,22 @@ the data.
* **grpc.client.attempt.started** <br>
The total number of RPC attempts started, including those that have not completed. <br>
*Attributes*: grpc.method, grpc.target <br>
*Type*: Counter <br>
*Type*: Counter (integer) <br>
Copy link
Member

Choose a reason for hiding this comment

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

I think we might want to say something more precise than just "integer" vs. "floating point". For example, would it be okay for a gRPC implementation to use a 32-bit integer here? I assume not, in which case we need language here saying so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants