palantir / conjure-go-runtime

Go implementation of the Conjure runtime
Apache License 2.0
12 stars 22 forks source link

fix: Fix memory leak with ever increasing slice of tls tags #535

Closed splucs closed 1 year ago

splucs commented 1 year ago

Summary

There was a memory leak in the TLS metrics: A tags slice was instantiated when the connection was initialized and is re-used for the entire lifecycle of the connection. The function TLSHandshakeDone is called every time the TLS handshake finishes and appends some values to the tags. This works if there is only one handshake per connection.

If the connection is re-used, the function TLSHandshakeDone is called multiple times and, each time, the same tag values are appended over and over again. This causes new metric IDs to be created, which causes the metrics registry to create a new entry for each handshake instead of using the existing entry. When the metrics are emitted, a bunch of metrics with a single value are emitted instead of metrics with aggregated values.

This PR fixes this by deep copying the tags slice before appending new values to it.

Testing

Reproed the issue without the fix, applied the fix and then confirmed the issue was solved.

Changelog

==COMMIT_MSG== Fix memory leak with ever increasing slice of tls tags ==COMMIT_MSG==

splucs commented 1 year ago

Closing in favor of https://github.com/palantir/conjure-go-runtime/pull/470