Skip to content

Conversation

olivielpeau
Copy link
Member

What does this PR do?

Fix small mem leak in set_external_tags. Only happens when an empty dict is passed for a given hostname.

Added test that demonstrates the leak.

Motivation

💅

Additional Notes

Was easy to find and fix thanks to the very useful mem tracking facilities that were added in #4134!

Only happens when an empty dict is passed for a given hostname.

Added test that demonstrates the leak.
@olivielpeau olivielpeau added [deprecated] team/agent-core Deprecated. Use metrics-logs / shared-components labels instead.. component/rtloader labels Oct 22, 2019
@olivielpeau olivielpeau added this to the 6.16.0 milestone Oct 22, 2019
@olivielpeau olivielpeau requested review from a team as code owners October 22, 2019 14:12
Copy link
Contributor

@ogaca-dd ogaca-dd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! We either had an almost identical bug somewhere else, or something weird happened because I would've sworn we had fixed this.

👌

@@ -384,6 +384,29 @@ func TestSetExternalTagInvalidTagsList(t *testing.T) {
helpers.AssertMemoryUsage(t)
}

func TestSetExternalTagEmptyDict(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍰

@olivielpeau olivielpeau merged commit 54b235e into master Nov 5, 2019
@olivielpeau olivielpeau deleted the olivielpeau/tiny-memleak-set-external-tags branch November 5, 2019 19:23
xlucas pushed a commit that referenced this pull request Nov 18, 2019
Only happens when an empty dict is passed for a given hostname.

Added test that demonstrates the leak.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/rtloader [deprecated] team/agent-core Deprecated. Use metrics-logs / shared-components labels instead..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants