[Performance] gRPC instrumentation: interceptor causes full payload to marshal into memory

[a-feld]

This call causes the proto to get marshalled into memory

Stack Trace:

go.opentelemetry.io/collector/model/internal/data/protogen/collector/metrics/v1.(*ExportMetricsServiceRequest).Marshal (opentelemetry-collector/model/internal/data/protogen/collector/metrics/v1/metrics_service.pb.go:238)
google.golang.org/protobuf/internal/impl.legacyMarshal (google.golang.org/[email protected]/internal/impl/legacy_message.go:404)
google.golang.org/protobuf/proto.MarshalOptions.size (google.golang.org/[email protected]/proto/size.go:43)
google.golang.org/protobuf/proto.MarshalOptions.Size (google.golang.org/[email protected]/proto/size.go:26)
google.golang.org/protobuf/proto.Size (google.golang.org/[email protected]/proto/size.go:16)
github.com/golang/protobuf/proto.Size (github.com/golang/[email protected]/proto/wire.go:18)
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.messageType.Event (go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/[email protected]/interceptor.go:52)
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.UnaryServerInterceptor.func1 (go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/[email protected]/interceptor.go:336)
go.opentelemetry.io/collector/model/internal/data/protogen/collector/metrics/v1._MetricsService_Export_Handler (opentelemetry-collector/model/internal/data/protogen/collector/metrics/v1/metrics_service.pb.go:218)
google.golang.org/grpc.(*Server).processUnaryRPC (google.golang.org/[email protected]/server.go:1292)
google.golang.org/grpc.(*Server).handleStream (google.golang.org/[email protected]/server.go:1617)
google.golang.org/grpc.(*Server).serveStreams.func1.2 (google.golang.org/[email protected]/server.go:940)
runtime.goexit (/usr/local/Cellar/go/1.17/libexec/src/runtime/asm_amd64.s:1581)

Calling proto.size() from go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.messageType.Event leads to the full payload being marshalled in memory. This ultimately leads to garbage getting created on each request.

[MrAlias]

Do you have a proposed solution?

Based on the specification this event should be annotated the way it currently is. I do not know of another way to get the uncompressed request payload size without calling the function we are calling. I'm interested to know know what you had in mind here.

[a-feld]

I'm looking into it right now 😄 I think the solution here may have to be in the collector protos. It looks like there's a fast path if there's a size method available

[a-feld]

I'm also wondering if there's a way to omit this parameter if we can detect if we have to go on the slow path?

[jack-berg]

I ran this in debug mode. The fast path @a-feld mentioned doesn't called because there's logic which normalizes proto v1 and v2 messages as v2 messages. The the otel protos (like ExportTraceServiceRequest) get wrapped in a legacy message because they qualify as v1 messages here.

This causes the reflection to fail to resolve that a Size method exists, and the slow path to be taken rather than the fast path.