Skip to content

Commit

Permalink
fix: use ValidatedSetSelector to reduce memory usage in k8s client (#317
Browse files Browse the repository at this point in the history
)

<!--
Thank you for contributing to the project! 💜
Please see our [OSS process
document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#)
to get an idea of how we operate.
-->

## Which problem is this PR solving?

- Closes #313

## Short description of the changes

When looking through the documentation for the selector matching
functionality, I noticed a
[comment](https://github.com/kubernetes/apimachinery/blob/fa98d6eaedb4caccd69fc07d90bbb6a1e551f00f/pkg/labels/selector.go#L941-L942)
about the underlying data being copied due to it being treated as
potentially mutable.

We call the `GetServiceForPod` method of the cached k8s client at least
twice per event when sending [Honeycomb
events](https://github.com/honeycombio/honeycomb-network-agent/blob/5cc35d2b1e1f2377199405b9d3889a34b6cafb7e/handlers/libhoney_event_handler.go#L117-L118)
and when [creating spans in the OTEL
handler](https://github.com/honeycombio/honeycomb-network-agent/blob/5cc35d2b1e1f2377199405b9d3889a34b6cafb7e/handlers/otel_handler.go#L132-L140).
This means the same label selector values for both source and
destination events are being copied a total of 4 times.

This PR updates the usage of the cache to use a
[`ValidatedSetSelector`](https://github.com/kubernetes/apimachinery/blob/fa98d6eaedb4caccd69fc07d90bbb6a1e551f00f/pkg/labels/selector.go#L975)
which implements a
[`Matches`](https://github.com/kubernetes/apimachinery/blob/fa98d6eaedb4caccd69fc07d90bbb6a1e551f00f/pkg/labels/selector.go#L977-L984)
method that does not copy.

## How to verify that this has the expected result

If possible, we should run a debug build in a testing environment and
compare the total RAM usage to the figures shown in the graph attached
to the original issue.

---------

Signed-off-by: Dan Bond <danbond@protonmail.com>
  • Loading branch information
loshz committed Nov 22, 2023
1 parent 5cc35d2 commit 529c18a
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion utils/cached_k8s_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,13 @@ func (c *CachedK8sClient) GetServiceForPod(pod *v1.Pod) *v1.Service {
if service.Spec.Selector == nil {
continue
}
serviceSelector := labels.SelectorFromSet(service.Spec.Selector)

// We use a ValidatedSetSelector here as the lables should be treated as immutable,
// due to the fact we receieve them directly from the client cache.
// When using a SelectorFromSet, corresponding methods treat the selectors as mutable
// and therefore perform a copy on the underlying data, which is not performant on
// large data sets.
serviceSelector := labels.ValidatedSetSelector(service.Spec.Selector)
if serviceSelector.Matches(podLabels) {
return service
}
Expand Down

0 comments on commit 529c18a

Please sign in to comment.