-
Notifications
You must be signed in to change notification settings - Fork 174
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
Link to proposal for gRPC metrics #627
Conversation
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
I think there's a few meta-conversations happening over this PR.
Let's try to tone down this PR to just addressing one thing, and we can follow up on the rest. My comments on each. Is this PR a breaking change?As written, yes. I would separate the PR that attempts to remove gRPC from generic RPC metrics from the one which adds links to gRPC's own o11y specification. Can gRPC link out to its new o11y specification form the OTEL semconv?We allow .NET, e.g. to define their client-library semconv here for OpenTelemetry users. I think having gRPC define its own metrics is in line with that. We can/should accept a link from our semconv on gRPC to the gRPC-owned o11y specification. Generally, OpenTelemetry is more than happy when projects take their own self-o11y into hand and own it / drive it forward themselves. Should gRPC participate in generic OpenTelemetry RPC conventions?This is not something OpenTelemetry can force. I think @markdroth presents a lot of rationale and reasons why gRPC thinks it will be more user-friendly for RPC-implementation-specific semantic conventions. I think folks in our community disagree. My own opinion is nuanced. I don't see the same kind of general specificaiton/abstraction that keeps databases more tightly coupled (e.g. SQL). However, specifically for gRPC spans, I think providing consistent I suggest for the purpose of this PR we leave this discussion out of it, and have that as a sidebar w/ My suggestionI think is true today:
Let's focus this PR on non-breaking changes to point at Let's follow up discussions between our community and @yashykt @markdroth et. al. on gRPC's participation in generic RPC conventions. |
👍 We worked heavily with the .NET team when stabilizing the OTel HTTP semantic conventions. They helped influence our HTTP semantic conventions, and we helped influence their .NET specific semantic conventions, and we ended up with a nicely aligned story across both. I would ❤️ to see the gRPC and OTel communities work together, I believe we can have similar success in the RPC semantic conventions (even more so since we are both under the CNCF umbrella! 🏖️). @jsuereth and I are both excited to kick off an RPC semantic convention stability working group this year. If there's a time constraint on the gRPC side to get this done, let us know and we can look at kicking it off sooner than later. |
As suggested, I've removed the breaking changes parts of this PR. I don't think there's a time constraint but I think we should try to resolve the concerns earlier rather than later. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
ping |
@yashykt could you please add a changelog entry? |
@lmolkova Done |
Changes
Link to specification for gRPC metrics - https://github.com/grpc/proposal/blob/master/A66-otel-stats.md
Merge requirement checklist
On running
make markdown-link-check
, I got the following errors, but I'm ignoring them since I'm not modifying these files -