Fix for out of memory error with request body #806

aayush-ap · 2023-10-23T13:16:54Z

Fix for out-of-memory error with request body

Consumes a lot of memory during large body size requests #795
Added new implementation to add a limit on request body size.
To configure body_limit on read request body use nrsecurityagent.ConfigSecurityRequestBodyLimit method or NEW_RELIC_SECURITY_REQUEST_BODY_LIMIT env. By default, this is "300kb"
Example :
export NEW_RELIC_SECURITY_REQUEST_BODY_LIMIT=100
When the security agent is not active, the APM agent does not access the request body.
This change will required min v0.5.0 version of csec-go-agent

* minor fix for complete security disable flag * Create FastHTTP Client Functions * FastHTTP Request Integration * FastHTTP example file * FastHTTP Request Integration * FastHTTP Response file * mod file * update security agent version * supportability metric * Created unit tests and removed extraneous file * Moved FastHTTP to internal instrumentation * Added testing for errors * chore: add logs-in-context example with logrus * chore: move example to specific folder * FastHTTP external segments/Client example * License for Server Example * Added test for external segment/minor fixes * FastHTTP Integration (newrelic#774) Added Support For FastHTTP * V3.25.0 Changelog (newrelic#781) * V3.25.0 * update version * corrected changelog for 3.25 release * Fixed test not passing * Update segments.go Removed extra function --------- Co-authored-by: aayush-ap <agarg@newrelic.com> Co-authored-by: Steve Willoughby <76975199+nr-swilloughby@users.noreply.github.com> Co-authored-by: Julien Erard <jerard@newrelic.com> Co-authored-by: Emilio Garcia <iamemilio@users.noreply.github.com> Co-authored-by: Steve Willoughby <swilloughby@newrelic.com>

Release 3.25.1

Release 3.26.0

Release 3.27.0

codecov-commenter · 2023-10-23T13:18:44Z

Codecov Report

Attention: 86 lines in your changes are missing coverage. Please review.

Comparison is base (a1142ca) 81.10% compared to head (9cfa052) 80.27%.
Report is 5 commits behind head on develop.

Files	Patch %	Lines
v3/integrations/nrmicro/nrmicro.go	7.31%	36 Missing and 2 partials ⚠️
v3/newrelic/secure_agent.go	0.00%	34 Missing ⚠️
v3/newrelic/transaction.go	12.50%	13 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #806      +/-   ##
===========================================
- Coverage    81.10%   80.27%   -0.83%     
===========================================
  Files          136      134       -2     
  Lines        12393    12328      -65     
===========================================
- Hits         10051     9896     -155     
- Misses        2062     2150      +88     
- Partials       280      282       +2

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

v3/newrelic/transaction.go

mirackara · 2023-10-26T19:10:14Z

@aayush-ap It looks like this change also breaks the nrmicro and nrgrpc integrations (You can see more details in the GHA test suite for those two tests respectively. Once these are fixed, this looks good to merge :)

aayush-ap · 2023-10-30T04:10:39Z

@mirackara
Please verify with latest changes

mirackara

LGTM! Thank you

mirackara · 2023-11-13T17:17:13Z

v3/newrelic/transaction.go

@@ -633,7 +626,7 @@ func (webrequest WebRequest) GetHost() string {
 	return webrequest.Host
 }

-func (webrequest WebRequest) GetBody() []byte {
+func (webrequest WebRequest) GetBody() any {


Hello @aayush-ap, is there a specific reason why this is an any type? Is this function to be used only for the security agent? If there is a specific type we want to make sure any external usage has an expected return value.

yes,getBody is only used by the cses-go-agent(security agent).
https://github.com/newrelic/csec-go-agent/blob/main/security_intercept/intercept.go#L748

If thats the case it would be best practices to have GetBody return a type and have the cses-go-agent convert it as it needs be. Once this is fixed, we can merge this :)

For now, this is not possible because csec-go-agent has already been released we can make this change in the next release of both agents.
According to me, this will not make any issue because GetBody is only used by the csec agent.

Alright. I think we're going to put this on ice for now until the changes to the csec-go-agent are in place. We just want to make sure that any public facing API's remain the same so customer who may be using the function in their code bases aren't affected by this change

* Error Expected Bug The attribute error.expected should be a boolean, not a string. It is also good practice to use a constant value for the key. * Bump google.golang.org/grpc from 1.54.0 to 1.56.3 in /v3/integrations/nrgraphqlgo/example (#811) --------- * Bump google.golang.org/grpc in /v3/integrations/nrgraphqlgo/example Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.54.0 to 1.56.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.54.0...v1.56.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> --------- * Bump google.golang.org/grpc from 1.54.0 to 1.56.3 in /v3/integrations/nrgrpc (#810) --------- * Bump google.golang.org/grpc in /v3/integrations/nrgrpc Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.54.0 to 1.56.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.54.0...v1.56.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> --------- * Bump google.golang.org/grpc from 1.54.0 to 1.56.3 in /v3 (#809) --------- * Bump google.golang.org/grpc from 1.54.0 to 1.56.3 in /v3 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.54.0 to 1.56.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.54.0...v1.56.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> --------- * Bump golang.org/x/net from 0.8.0 to 0.17.0 in /v3/integrations/nrgraphqlgo/example (#804) --------- * Bump golang.org/x/net in /v3/integrations/nrgraphqlgo/example Bumps [golang.org/x/net](https://github.com/golang/net) from 0.8.0 to 0.17.0. - [Commits](golang/net@v0.8.0...v0.17.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> --------- * Fix for out of memory error with request body (#806) * Release 3.25.0 (#782) * minor fix for complete security disable flag * Create FastHTTP Client Functions * FastHTTP Request Integration * FastHTTP example file * FastHTTP Request Integration * FastHTTP Response file * mod file * update security agent version * supportability metric * Created unit tests and removed extraneous file * Moved FastHTTP to internal instrumentation * Added testing for errors * chore: add logs-in-context example with logrus * chore: move example to specific folder * FastHTTP external segments/Client example * License for Server Example * Added test for external segment/minor fixes * FastHTTP Integration (#774) Added Support For FastHTTP * V3.25.0 Changelog (#781) * V3.25.0 * update version * corrected changelog for 3.25 release * Fixed test not passing * Update segments.go Removed extra function --------- Co-authored-by: aayush-ap <agarg@newrelic.com> Co-authored-by: Steve Willoughby <76975199+nr-swilloughby@users.noreply.github.com> Co-authored-by: Julien Erard <jerard@newrelic.com> Co-authored-by: Emilio Garcia <iamemilio@users.noreply.github.com> Co-authored-by: Steve Willoughby <swilloughby@newrelic.com> * fix out of memory issue for req body * Added new config parameter for read request body * update request body buffer * minor fix for dataTruncated * Update readme file * Update csec-go-agent version * Added new wrapper for go-micro stream server * minor fix for GHA * Fix for cpu overhead * backward compatibility * update agent version * minor fix --------- Co-authored-by: Mirac Kara <55501260+mirackara@users.noreply.github.com> Co-authored-by: Steve Willoughby <76975199+nr-swilloughby@users.noreply.github.com> Co-authored-by: Julien Erard <jerard@newrelic.com> Co-authored-by: Emilio Garcia <iamemilio@users.noreply.github.com> Co-authored-by: Steve Willoughby <swilloughby@newrelic.com> * move fasthttp out of core library, and into integration package (#808) * move fasthttp out of core library, and into integration package * move examples over * add security agent headers to fasthttp object * fix examples and external segment * add fasthttp tests * cleanup of go mods * fix segment collection * add security agent inbound write capture to wrapped handle func * Update go.mod * Update Changelog * update version.go --------- Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: aayush-ap <59004877+aayush-ap@users.noreply.github.com> Co-authored-by: Mirac Kara <55501260+mirackara@users.noreply.github.com> Co-authored-by: Steve Willoughby <76975199+nr-swilloughby@users.noreply.github.com> Co-authored-by: Julien Erard <jerard@newrelic.com> Co-authored-by: Steve Willoughby <swilloughby@newrelic.com> Co-authored-by: mirackara <mirackara2000@outlook.com>

mirackara and others added 12 commits September 18, 2023 11:35

Merge branch 'develop'

56cfa1c

Merge pull request newrelic#790 from nr-swilloughby/master

e9f0cf7

Release 3.25.1

Merge pull request newrelic#794 from newrelic/develop

d0a46d0

Release 3.26.0

fix out of memory issue for req body

24c4648

Merge pull request newrelic#801 from newrelic/develop

79d3bfd

Release 3.27.0

Merge branch 'newrelic:master' into requestbody-fix

ee1899a

Added new config parameter for read request body

93283f4

update request body buffer

476fefd

minor fix for dataTruncated

32826a6

Update readme file

3e087ce

Update csec-go-agent version

31ac930

mirackara self-requested a review October 26, 2023 16:30

mirackara reviewed Oct 26, 2023

View reviewed changes

v3/newrelic/transaction.go Outdated Show resolved Hide resolved

aayush-ap added 2 commits October 27, 2023 12:55

Added new wrapper for go-micro stream server

5eea3c6

minor fix for GHA

e5a77be

mirackara previously approved these changes Oct 30, 2023

View reviewed changes

Fix for cpu overhead

536d99d

aayush-ap dismissed mirackara’s stale review via 536d99d October 31, 2023 17:18

aayush-ap requested a review from mirackara October 31, 2023 17:19

mirackara reviewed Nov 13, 2023

View reviewed changes

aayush-ap added 3 commits November 16, 2023 06:43

backward compatibility

e5f4211

update agent version

1a30a28

minor fix

9cfa052

aayush-ap requested a review from mirackara November 16, 2023 18:13

Merge branch 'develop' into master

8fb0ecf

mirackara approved these changes Nov 16, 2023

View reviewed changes

mirackara merged commit b3ce5df into newrelic:develop Nov 16, 2023
43 of 47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for out of memory error with request body #806

Fix for out of memory error with request body #806

aayush-ap commented Oct 23, 2023

codecov-commenter commented Oct 23, 2023 •

edited

Loading

mirackara commented Oct 26, 2023

aayush-ap commented Oct 30, 2023

mirackara left a comment

mirackara Nov 13, 2023

aayush-ap Nov 14, 2023

mirackara Nov 14, 2023

aayush-ap Nov 15, 2023 •

edited

Loading

mirackara Nov 15, 2023

Fix for out of memory error with request body #806

Fix for out of memory error with request body #806

Conversation

aayush-ap commented Oct 23, 2023