Skip to content

Commit

Permalink
fix ever-accumulating memory in logger
Browse files Browse the repository at this point in the history
  • Loading branch information
nojnhuh authored and k8s-infra-cherrypick-robot committed Nov 18, 2024
1 parent 449ecde commit bf6d8f2
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 5 deletions.
14 changes: 9 additions & 5 deletions util/tele/span_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,18 @@ func (s *spanLogSink) Error(err error, msg string, keysAndValues ...interface{})
)
}

func (s *spanLogSink) WithValues(keysAndValues ...interface{}) logr.LogSink {
s.vals = append(s.vals, keysAndValues...)
return s
func (s spanLogSink) WithValues(keysAndValues ...interface{}) logr.LogSink {
// always create a new slice to avoid multiple loggers writing to the same backing array
vals := make([]interface{}, len(s.vals)+len(keysAndValues))
copy(vals, s.vals)
copy(vals[len(s.vals):], keysAndValues)
s.vals = vals
return &s
}

func (s *spanLogSink) WithName(name string) logr.LogSink {
func (s spanLogSink) WithName(name string) logr.LogSink {

Check warning on line 100 in util/tele/span_logger.go

View check run for this annotation

Codecov / codecov/patch

util/tele/span_logger.go#L100

Added line #L100 was not covered by tests
s.name = name
return s
return &s

Check warning on line 102 in util/tele/span_logger.go

View check run for this annotation

Codecov / codecov/patch

util/tele/span_logger.go#L102

Added line #L102 was not covered by tests
}

// NewSpanLogSink is the main entry-point to this implementation.
Expand Down
48 changes: 48 additions & 0 deletions util/tele/span_logger_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package tele

import (
"testing"

"github.com/go-logr/logr"
. "github.com/onsi/gomega"
)

func TestSpanLogSinkWithValues(t *testing.T) {
g := NewGomegaWithT(t)

var log0 logr.LogSink = &spanLogSink{
// simulating a slice with cap() > len() where an append() will not create a new array
vals: make([]interface{}, 0, 4),
}

log0 = log0.WithValues("k0", "v0")

g.Expect(log0.(*spanLogSink).vals).To(HaveExactElements("k0", "v0"))

log1 := log0.WithValues("k1", "v1")

g.Expect(log0.(*spanLogSink).vals).To(HaveExactElements("k0", "v0"))
g.Expect(log1.(*spanLogSink).vals).To(HaveExactElements("k0", "v0", "k1", "v1"))

log2 := log0.WithValues("k2", "v2")

g.Expect(log0.(*spanLogSink).vals).To(HaveExactElements("k0", "v0"))
g.Expect(log1.(*spanLogSink).vals).To(HaveExactElements("k0", "v0", "k1", "v1"))
g.Expect(log2.(*spanLogSink).vals).To(HaveExactElements("k0", "v0", "k2", "v2"))
}

0 comments on commit bf6d8f2

Please sign in to comment.