[config/confighttp] add memorylimiterextension to confighttp #9397

timannguyen · 2024-01-25T15:50:17Z

Description:

integrate MemoryLimiterExtension with confighttp. The line of thinking is if there is a limiter extension all http servers would be restricted based on that memory requirement in order to not crash the collector from OOM.

Link to tracking Issue: 8632

Testing:

unit test

Documentation:

confighttp README linking to memorylimiterextension.

extension/memorylimiterextension/memorylimiter.go

codecov · 2024-01-25T19:25:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.91%. Comparing base (420772e) to head (960c5a2).
Report is 191 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9397      +/-   ##
==========================================
- Coverage   90.93%   90.91%   -0.02%     
==========================================
  Files         348      348              
  Lines       18379    18399      +20     
==========================================
+ Hits        16713    16728      +15     
- Misses       1344     1348       +4     
- Partials      322      323       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

config/confighttp/confighttp.go

dmitryax · 2024-01-25T22:47:35Z

I'm not sure that silently applying any available memory_limiter extension is the right approach to start. What if I want some receivers to be more aggressive in rejecting based on memory utilization than others? There must be a way to configure that even if we allow the behavior defined in this PR. To avoid introducing breaking changes going forward, we need to design the user interface before applying this change.

splunkericl · 2024-01-26T16:44:47Z

I'm not sure that silently applying any available memory_limiter extension is the right approach to start. What if I want some receivers to be more aggressive in rejecting based on memory utilization than others? There must be a way to configure that even if we allow the behavior defined in this PR. To avoid introducing breaking changes going forward, we need to design the user interface before applying this change.

Yeah I think what we can do is simulate how Auth is handled in confighttp. We can add a component ID in HTTPServerSettings for rate(memory) limiting components and only load it if users specify it.

timannguyen · 2024-01-29T20:02:30Z

@dmitryax updated to only add the extension to the struct. nonfunctional.

config/confighttp/confighttp.go

extension/memorylimiterextension/memorylimiter.go

config/confighttp/confighttp.go

extension/memorylimiterextension/memorylimiter.go

config/confighttp/confighttp_test.go

.chloggen/timn_memorylimiterextension-HTTPServerConfig.yaml

config/confighttp/README.md

dmitryax

Couple nits. Otherwise LGTM

dmitryax · 2024-02-09T04:16:54Z

config/confighttp/README.md


+`memory_limiter`: [Memory limiter extension](../../extension/memorylimiterextension/README.md) that will reject incoming requests once the memory utilization grows above configured limits.


Please keep it as another option in the list

Suggested change

`memory_limiter`: [Memory limiter extension](../../extension/memorylimiterextension/README.md) that will reject incoming requests once the memory utilization grows above configured limits.

- `memory_limiter`: [Memory limiter extension](../../extension/memorylimiterextension/README.md) that will reject incoming requests once the memory utilization grows above configured limits.

dmitryax · 2024-02-09T04:19:28Z

config/confighttp/confighttp.go

+	if hss.MemoryLimiter != nil {
+		ml, err := getMemoryLimiterExtension(hss.MemoryLimiter, host.GetExtensions())
+		if err != nil {
+			return nil, err


Please cover this path to make codecov happy. Should be easy I guess

github-actions · 2024-03-20T03:15:38Z

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

github-actions · 2024-04-04T03:16:10Z

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

github-actions · 2024-04-20T03:15:47Z

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

github-actions · 2024-05-08T03:16:06Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

timannguyen requested review from a team and Aneurysm9 January 25, 2024 15:50

timannguyen changed the title ~~add memorylimiterextension to confighttp~~ [config/confighttp] add memorylimiterextension to confighttp Jan 25, 2024

TylerHelmuth reviewed Jan 25, 2024

View reviewed changes

extension/memorylimiterextension/memorylimiter.go Outdated Show resolved Hide resolved

timannguyen force-pushed the http_memlimit branch from c21489e to 2a902ce Compare January 25, 2024 19:39

splunkericl reviewed Jan 25, 2024

View reviewed changes

config/confighttp/confighttp.go Outdated Show resolved Hide resolved

config/confighttp/confighttp.go Outdated Show resolved Hide resolved

timannguyen force-pushed the http_memlimit branch 2 times, most recently from c3d1ec7 to 7691c98 Compare January 26, 2024 16:06

timannguyen force-pushed the http_memlimit branch 2 times, most recently from 97e6f04 to a87cc5f Compare January 29, 2024 19:54

dmitryax reviewed Feb 1, 2024

View reviewed changes

config/confighttp/confighttp.go Outdated Show resolved Hide resolved

extension/memorylimiterextension/memorylimiter.go Outdated Show resolved Hide resolved

timannguyen force-pushed the http_memlimit branch 2 times, most recently from 021512f to 579c4a0 Compare February 1, 2024 20:56

dmitryax reviewed Feb 5, 2024

View reviewed changes

config/confighttp/confighttp.go Outdated Show resolved Hide resolved

dmitryax reviewed Feb 5, 2024

View reviewed changes

config/confighttp/confighttp.go Outdated Show resolved Hide resolved

timannguyen force-pushed the http_memlimit branch from 579c4a0 to 664ddc2 Compare February 5, 2024 21:57

dmitryax reviewed Feb 7, 2024

View reviewed changes

timannguyen force-pushed the http_memlimit branch from 9cade05 to 4f52623 Compare February 8, 2024 16:38

dmitryax reviewed Feb 9, 2024

View reviewed changes

timannguyen force-pushed the http_memlimit branch 2 times, most recently from 8027d83 to 9b4a301 Compare February 13, 2024 17:21

dmitryax mentioned this pull request Feb 15, 2024

Applying memory_limiter extension #9591

Open

timannguyen force-pushed the http_memlimit branch from ee588e9 to 7e876fa Compare February 22, 2024 17:28

[config/confighttp] add memorylimiterextension to HTTPServerConfig

4fbc7a5

timannguyen force-pushed the http_memlimit branch from 7e876fa to 5902134 Compare March 5, 2024 15:43

improve coverage

960c5a2

timannguyen force-pushed the http_memlimit branch from 5902134 to 960c5a2 Compare March 5, 2024 16:25

github-actions bot added Stale and removed Stale labels Mar 20, 2024

github-actions bot added Stale and removed Stale labels Apr 4, 2024

github-actions bot added the Stale label Apr 20, 2024

github-actions bot closed this May 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[config/confighttp] add memorylimiterextension to confighttp #9397

[config/confighttp] add memorylimiterextension to confighttp #9397

timannguyen commented Jan 25, 2024 •

edited

Loading

codecov bot commented Jan 25, 2024 •

edited

Loading

dmitryax commented Jan 25, 2024

splunkericl commented Jan 26, 2024 •

edited

Loading

timannguyen commented Jan 29, 2024

dmitryax left a comment

dmitryax Feb 9, 2024

timannguyen Feb 21, 2024

dmitryax Feb 9, 2024

timannguyen Feb 21, 2024

github-actions bot commented Mar 20, 2024

github-actions bot commented Apr 4, 2024

github-actions bot commented Apr 20, 2024

github-actions bot commented May 8, 2024


		`memory_limiter`: [Memory limiter extension](../../extension/memorylimiterextension/README.md) that will reject incoming requests once the memory utilization grows above configured limits.


	`memory_limiter`: [Memory limiter extension](../../extension/memorylimiterextension/README.md) that will reject incoming requests once the memory utilization grows above configured limits.
	- `memory_limiter`: [Memory limiter extension](../../extension/memorylimiterextension/README.md) that will reject incoming requests once the memory utilization grows above configured limits.

[config/confighttp] add memorylimiterextension to confighttp #9397

[config/confighttp] add memorylimiterextension to confighttp #9397

Conversation

timannguyen commented Jan 25, 2024 • edited Loading

codecov bot commented Jan 25, 2024 • edited Loading

Codecov Report

dmitryax commented Jan 25, 2024

splunkericl commented Jan 26, 2024 • edited Loading

timannguyen commented Jan 29, 2024

dmitryax left a comment

Choose a reason for hiding this comment

dmitryax Feb 9, 2024

Choose a reason for hiding this comment

timannguyen Feb 21, 2024

Choose a reason for hiding this comment

dmitryax Feb 9, 2024

Choose a reason for hiding this comment

timannguyen Feb 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Mar 20, 2024

github-actions bot commented Apr 4, 2024

github-actions bot commented Apr 20, 2024

github-actions bot commented May 8, 2024

timannguyen commented Jan 25, 2024 •

edited

Loading

codecov bot commented Jan 25, 2024 •

edited

Loading

splunkericl commented Jan 26, 2024 •

edited

Loading