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.

Sign up for GitHub

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

Reduce memory leakage in API client caused by otelgrpc interceptors #30767

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Reduce memory leakage in API client caused by otelgrpc interceptors #30767

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
333bb8b
Avoid memory leaking in `otelgrpc` interceptors
strideynet Aug 21, 2023
7a7b62b
Fix for non go 1.21 usage
strideynet Aug 21, 2023
3296695
Add link to issue reported in otel-go-contrib
strideynet Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions api/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,12 @@ func (c *Client) dialGRPC(ctx context.Context, addr string) error {
dialOpts = append(dialOpts, grpc.WithContextDialer(c.grpcDialer()))
dialOpts = append(dialOpts,
grpc.WithChainUnaryInterceptor(
otelgrpc.UnaryClientInterceptor(),
otelUnaryClientInterceptor(),
metadata.UnaryClientInterceptor,
breaker.UnaryClientInterceptor(cb),
),
grpc.WithChainStreamInterceptor(
otelgrpc.StreamClientInterceptor(),
otelStreamClientInterceptor(),
metadata.StreamClientInterceptor,
breaker.StreamClientInterceptor(cb),
),
Expand Down Expand Up @@ -487,6 +487,36 @@ func (c *Client) dialGRPC(ctx context.Context, addr string) error {
return nil
}

// TODO(noah): Once we upgrade to go 1.21, change invocations of this to
// sync.OnceValue
func onceValue[T any](f func() T) func() T {
var (
value T
once sync.Once
)
return func() T {
once.Do(func() {
value = f()
})
return value
}
}

// We wrap the creation of the otelgrpc interceptors in a sync.Once - this is
// because each time this is called, they create a new underlying metric. If
// something (e.g tbot) is repeatedly creating new clients and closing them,
// then this leads to a memory leak since the underlying metric is not cleaned
// up.
// See https://github.com/gravitational/teleport/issues/30759
// See https://github.com/open-telemetry/opentelemetry-go-contrib/issues/4226
var otelStreamClientInterceptor = onceValue(func() grpc.StreamClientInterceptor {
return otelgrpc.StreamClientInterceptor()
})

var otelUnaryClientInterceptor = onceValue(func() grpc.UnaryClientInterceptor {
return otelgrpc.UnaryClientInterceptor()
})

// ConfigureALPN configures ALPN SNI cluster routing information in TLS settings allowing for
// allowing to dial auth service through Teleport Proxy directly without using SSH Tunnels.
func ConfigureALPN(tlsConfig *tls.Config, clusterName string) *tls.Config {
Expand Down