Skip to content

Add memory leak scenario to email service#2481

Merged
julianocosta89 merged 5 commits intoopen-telemetry:mainfrom
svrnm:add-memory-leak-scenario
Sep 5, 2025
Merged

Add memory leak scenario to email service#2481
julianocosta89 merged 5 commits intoopen-telemetry:mainfrom
svrnm:add-memory-leak-scenario

Conversation

@svrnm
Copy link
Copy Markdown
Member

@svrnm svrnm commented Aug 26, 2025

Changes

There is an existing memory leak in the email service, this PR turns this existing issue into a scenario that can be toggled by a feature flag. Since the leak is growing slowly by default (~1.4KB/email) I also added some way to speed things up by adding lots of whitespaces to the end of an email, up to 10000x, which leads to ~14MB/email, filling the memory rather quickly, especially in k8s where the default limit is ~100MB.

Note, that the default behavior is that the memory leak no longer shows up, by running Mail::TestMailer.deliveries.clear after each test email being put into memory with the "test" transport. I ran it that way for a while and the memory consumption stayed stable around ~50MB.

Edited for more details

Merge Requirements

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
  • Appropriate Helm chart updates in the helm-charts

Signed-off-by: svrnm <sneumann@causely.ai>
@svrnm svrnm requested a review from a team as a code owner August 26, 2025 16:22
@github-actions github-actions bot added docs-update-required Requires documentation update helm-update-required Requires an update to the Helm chart when released labels Aug 26, 2025
svrnm and others added 2 commits August 26, 2025 19:27
@julianocosta89
Copy link
Copy Markdown
Member

@svrnm I'm having a hard time validating this.
I've started the demo and let it run for some time, then I've enabled the max value of the new feature flag.
I was expecting a huge spike on the memory consumption, but it didn't change anything.

I've enabled the feature flag at 14:10.

This is what I have in Grafana:
Screenshot 2025-08-28 at 14 16 43

During the spike from 13:45 to 13:50 the feature flag was disabled.

@svrnm
Copy link
Copy Markdown
Member Author

svrnm commented Aug 28, 2025

@julianocosta89 thanks for checking, odd, that you can not see the spike. Here's how I tested it:

I ran the flagd and email service in a docker container each
I ran a curl script that send ~1 request per second and with that one email was dispatched every time

I have not checked in those scripts, but I can make them available as needed.

Let me go back myself and run it again.

…ker compose

Signed-off-by: svrnm <severin.neumann@altmuehlnet.de>
@svrnm
Copy link
Copy Markdown
Member Author

svrnm commented Aug 28, 2025

I forgot to add the FLAGD_ environment variables to docker-compose.yml, fixed that with the last commit.

For my own testing I created the following 2 files:

When I use the "1000x" feature I can grow the memory quickly

Interestingly I observed that the memory fills up until the memory limit (~100MB) and then garbage collection kicks in and cleans out some of the emails as well, so it would never crash, only fill memory up to max.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 5, 2025

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 5, 2025
Copy link
Copy Markdown
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

awesome!
Thank you @svrnm 🤩

Sorry for taking some time to come back to you.

I've tested it again today and it worked like a charm:

Image

@julianocosta89 julianocosta89 merged commit 7830d56 into open-telemetry:main Sep 5, 2025
31 checks passed
@julianocosta89
Copy link
Copy Markdown
Member

@svrnm would you be able to take care of adding this scenario here: https://opentelemetry.io/docs/demo/feature-flags/ ?

@svrnm
Copy link
Copy Markdown
Member Author

svrnm commented Sep 5, 2025

@svrnm would you be able to take care of adding this scenario here: opentelemetry.io/docs/demo/feature-flags ?

of course :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-required Requires documentation update helm-update-required Requires an update to the Helm chart when released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants