From d36f18973c571dbf4141a0f3bf52312a8348d10a Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Fri, 10 Nov 2023 11:53:14 -0800 Subject: [PATCH 1/5] grpc.StatsHandler should record RPC durations in ms instead o fns --- CHANGELOG.md | 4 ++++ .../google.golang.org/grpc/otelgrpc/stats_handler.go | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4839547bea2..4bfd8510b2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108) +### Fixed + +- The `grpc.StatsHandler` now records RPC durations in ms instead of ns. (#4547) + ## [1.21.0/0.46.0/0.15.0/0.1.0] - 2023-11-10 ### Added diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go b/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go index 0211e55e003..f4ef62399d5 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go @@ -194,7 +194,11 @@ func (c *config) handleRPC(ctx context.Context, rs stats.RPCStats) { span.End() metricAttrs = append(metricAttrs, rpcStatusAttr) - c.rpcDuration.Record(wctx, float64(rs.EndTime.Sub(rs.BeginTime)), metric.WithAttributes(metricAttrs...)) + + // Use floating point division here for higher precision (instead of Millisecond method). + elapsedTime := float64(rs.EndTime.Sub(rs.BeginTime)) / float64(time.Millisecond) + + c.rpcDuration.Record(wctx, elapsedTime, metric.WithAttributes(metricAttrs...)) c.rpcRequestsPerRPC.Record(wctx, gctx.messagesReceived, metric.WithAttributes(metricAttrs...)) c.rpcResponsesPerRPC.Record(wctx, gctx.messagesSent, metric.WithAttributes(metricAttrs...)) From a454728ea21d0a9b2ebccf0bac2e108b3bb86148 Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Fri, 10 Nov 2023 12:57:55 -0800 Subject: [PATCH 2/5] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bfd8510b2f..fa55ccacaf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- The `grpc.StatsHandler` now records RPC durations in ms instead of ns. (#4547) +- The `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` instrumentation now records RPC durations in `ms` instead of `ns`. (#4547, #4548) ## [1.21.0/0.46.0/0.15.0/0.1.0] - 2023-11-10 From 007d2520b7509db47c052eb8590fd6fe3bca454c Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Fri, 10 Nov 2023 14:08:23 -0800 Subject: [PATCH 3/5] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa55ccacaf3..bdd6719385d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- The `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` instrumentation now records RPC durations in `ms` instead of `ns`. (#4547, #4548) +- The `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` instrumentation now records RPC durations in `ms` instead of `ns`. (#4548) ## [1.21.0/0.46.0/0.15.0/0.1.0] - 2023-11-10 From 4b3e369d676a7bcd113b393df36d62a9215fe177 Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Fri, 10 Nov 2023 14:10:23 -0800 Subject: [PATCH 4/5] Update gRPC interceptor to perform foating point division when calculating elapsed time --- .../google.golang.org/grpc/otelgrpc/interceptor.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go index fa015e9ac88..42b65688bd6 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go @@ -391,9 +391,11 @@ func UnaryServerInterceptor(opts ...Option) grpc.UnaryServerInterceptor { grpcStatusCodeAttr := statusCodeAttr(s.Code()) span.SetAttributes(grpcStatusCodeAttr) - elapsedTime := time.Since(before).Milliseconds() + // Use floating point division here for higher precision (instead of Millisecond method). + elapsedTime := float64(time.Since(before)) / float64(time.Millisecond) + metricAttrs = append(metricAttrs, grpcStatusCodeAttr) - cfg.rpcDuration.Record(ctx, float64(elapsedTime), metric.WithAttributes(metricAttrs...)) + cfg.rpcDuration.Record(ctx, elapsedTime, metric.WithAttributes(metricAttrs...)) return resp, err } From 445c84dc80f861c8071a515838dae039a675f840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 14 Nov 2023 10:00:35 +0100 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdd6719385d..e2c5440518e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- The `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` instrumentation now records RPC durations in `ms` instead of `ns`. (#4548) +- The stats handlers `NewClientHandler`, `NewServerHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` now records RPC durations in `ms` instead of `ns`. (#4548) ## [1.21.0/0.46.0/0.15.0/0.1.0] - 2023-11-10