Open aabmass opened 4 years ago
I am happy to submit a PR for this 🙂
@aabmass Does it need add extra dependencies to io.grpc:grpc-census
? Currently we only depend on io.opencensus:opencensus-api
and io.opencensus:opencensus-contrib-grpc-metrics
@dapengzhang0 I believe it needs io.opencensus:opencensus-contrib-exemplar-util
which is pretty small
@dapengzhang0 would you be open to a PR for this?
would you be open to a PR for this?
@aabmass We would like to know why we/google cloud want this feature before making a PR. Would you send some more detail feature request doc internally and let grpc-java-team@ review?
I've re-read some internal conversation and looked at the open PR and I think I now understand what an exemplar is and now the details make more sense.
Since the exemplars are only attached to the measure map for sampled traces, that means the CPU and memory costs will be very low. And it looks like a very useful feature. I think that means this is safe to enable for everyone without requiring an option. That makes this a lot easier to add. I think this stalled earlier because there was talk of it needing to be configurable and "configurable OpenCensus" is still an open issue.
There are still some implementation difficulties to consider, since this is the first time stats and tracing interact with each other.
Is your feature request related to a problem?
The OpenCensus GRPC instrumentation does not attach exemplar spans along with the distribution stats it collects.
Google cloud monitoring can interpret and display these exemplars to link metrics to example spans.
Describe the solution you'd like
Add exemplars to grcp-census instrumentation directly near this code: https://github.com/grpc/grpc-java/blob/020325617129b6ae9b25da1126ba0f98b044bb1b/census/src/main/java/io/grpc/census/CensusStatsModule.java#L647
Adding something like this should work:
Describe alternatives you've considered
I don't think there is a way to do this without updating the grpc-census instrumentation code. AFAIK adding the exemplar to the
MeasureMap
directly is the only way to do it.Additional context
I'm not sure how to get the OpenCensus Tracer for
tracer.getCurrentSpan()
inside ofCensusStatsModule
. It may also be beneficial to check if the current span is being sampled before attaching it to theMeasureMap
.cc @nilebox