allow_all_headers config parameter #2155

fallwith · 2023-08-09T22:57:35Z

A new 'allow_all_headers' configuration parameter has been added to bring parity with the Node.js agent and others. This configuration parameter defaults to a value of false. When set to true and as long as the agent is not operating in high security mode, all HTTP headers gleaned from a request will be captured and relayed to New Relic instead of the default core set of headers. All existing behavior for .*attributes.include and .*attributes.exclude configuration parameters will be respected for any desired filtration of the headers when allow_all_headers is enabled. This work was done in response to a feature request submitted by community member
@jamesarosen. Thank you very much, @jamesarosen!

resolves #1029

@jamesarosen

A new 'allow_all_headers' configuration parameter has been added to bring parity with the Node.js agent and others. This configuration parameter defaults to a value of `false`. When set to `true` and as long as the agent is not operating in high security mode, all HTTP headers gleaned from a request will be captured and relayed to New Relic instead of the default core set of headers. All existing behavior for `.*attributes.include` and `.*attributes.exclude` configuration parameters will be respected for any desired filtration of the headers when `allow_all_headers` is enabled. This work was done in response to a feature request submitted by community member [@jamesarosen](https://github.com/jamesarosen). Thank you very much, @jamesarosen! resolves #1029

remove 'binding.irb' debug breakpoint

focus on transaction tracer for the maximum number of permitted headers

lib/new_relic/language_support.rb

newrelic_rpm.gemspec

CHANGELOG.md

lib/new_relic/agent/transaction/request_attributes.rb

test/new_relic/agent/transaction/request_attributes_test.rb

update entry for allow_all_headers Co-authored-by: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com>

require no Rails or Rails v5.2+ for certain RequestAttribute tests

test/new_relic/agent/transaction/request_attributes_test.rb

CHANGELOG.md

lib/new_relic/agent/transaction/request_attributes.rb

test/helpers/misc.rb

test/new_relic/agent/transaction/request_attributes_test.rb

newrelic_rpm.gemspec

Grammar rework for allow_all_headers entry Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

previously allow_all_headers was only focused on headers that don't already have accessors on the RequestAttributes model, but after some dev discussion we decided that the _all_ bit should indeed refer to ALL headers and that means removing existing conditional checks for the base headers when allow_all_headers is set.

label the tin better

Technically allow_all_headers doesn't require Rails v5.2+ but rather Rack v2+

- Note about Rack version requirements - Note about attribute names

Insist on `Rack.release` and non 1.x

CI allow_all_headers requires Rack v2+

CHANGELOG.md

kaylareopelle · 2023-08-11T17:37:46Z

CHANGELOG.md

+  A new `allow_all_headers` configuration parameter brings parity with the Node.js agent (see the [v2.7.0 changelog](https://docs.newrelic.com/docs/release-notes/agent-release-notes/nodejs-release-notes/node-agent-270/)). This configuration parameter defaults to a value of `false`. When set to `true`, and as long as the agent is not operating in high-security mode, all HTTP headers gleaned from a request will be captured and relayed to New Relic instead of the default core set of headers. All existing behavior for `.*attributes.include` and `.*attributes.exclude` configuration parameters will be respected for any desired filtration of the headers when `allow_all_headers` is enabled. This work was done in response to a feature request submitted by community member [@jamesarosen](https://github.com/jamesarosen). Thank you very much, @jamesarosen! [Issue#1029](https://github.com/newrelic/newrelic-ruby-agent/issues/1029)
+
+  NOTE: The extra headers collected by having `allow_all_headers` enabled requires Rack version 2 or higher.
+  NOTE: The extra headers will appear as attributes prefixed with `request.headers.`
+


The long block of text makes this changelog entry a bit difficult for me to follow. Would you be open to breaking things into bullets instead?

Suggested change

A new `allow_all_headers` configuration parameter brings parity with the Node.js agent (see the [v2.7.0 changelog](https://docs.newrelic.com/docs/release-notes/agent-release-notes/nodejs-release-notes/node-agent-270/)). This configuration parameter defaults to a value of `false`. When set to `true`, and as long as the agent is not operating in high-security mode, all HTTP headers gleaned from a request will be captured and relayed to New Relic instead of the default core set of headers. All existing behavior for `.*attributes.include` and `.*attributes.exclude` configuration parameters will be respected for any desired filtration of the headers when `allow_all_headers` is enabled. This work was done in response to a feature request submitted by community member [@jamesarosen](https://github.com/jamesarosen). Thank you very much, @jamesarosen! [Issue#1029](https://github.com/newrelic/newrelic-ruby-agent/issues/1029)

NOTE: The extra headers collected by having `allow_all_headers` enabled requires Rack version 2 or higher.

NOTE: The extra headers will appear as attributes prefixed with `request.headers.`

A new `allow_all_headers` configuration option brings parity with the Node.js agent (see the [v2.7.0 changelog](https://docs.newrelic.com/docs/release-notes/agent-release-notes/nodejs-release-notes/node-agent-270/)).

This configuration option:

* Defaults to `false`.

* Is not compatible with high-security mode

* When set to `true` all HTTP headers gleaned from a request will be captured and relayed to New Relic. By default, only a small group of HTTP headers are sent to New Relic.

* Respects all existing behavior for `attributes.include` and `attributes.exclude` configuration options for any desired filtration of the headers when `allow_all_headers` is enabled.

* Requires Rack version 2 or higher.

* Displays the additional headers as transaction attributes prefixed with `request.headers.`

This work was done in response to a feature request submitted by community member [@jamesarosen](https://github.com/jamesarosen). Thank you very much, @jamesarosen! [Issue#1029](https://github.com/newrelic/newrelic-ruby-agent/issues/1029)

CHANGELOG.md

test/new_relic/agent/transaction/request_attributes_test.rb

allow_all_headers entry updates Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

allow_all_headers entry parameter -> option Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

instane -> instance typo fix Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

@kaylareopelle

(as paired on with @kaylareopelle) update the CHANGELOG entry for `allow_all_headers`

github-actions · 2023-08-11T19:47:23Z

SimpleCov Report

	Coverage	Threshold
Line	94.29%	94%
Branch	85.75%	85%

CHANGELOG.md

Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

fallwith requested review from hannahramadan, kaylareopelle and tannalynn as code owners August 9, 2023 22:57

fallwith added 2 commits August 9, 2023 15:57

remove 'binding.irb'

54d4567

remove 'binding.irb' debug breakpoint

remove commented out line left over from debugging

24ca8a8

focus on transaction tracer for the maximum number of permitted headers

fallwith commented Aug 9, 2023

View reviewed changes

lib/new_relic/language_support.rb Show resolved Hide resolved

newrelic_rpm.gemspec Show resolved Hide resolved

hannahramadan reviewed Aug 9, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

hannahramadan reviewed Aug 9, 2023

View reviewed changes

lib/new_relic/agent/transaction/request_attributes.rb Show resolved Hide resolved

hannahramadan reviewed Aug 9, 2023

View reviewed changes

test/new_relic/agent/transaction/request_attributes_test.rb Show resolved Hide resolved

fallwith and others added 2 commits August 9, 2023 17:16

Update CHANGELOG.md

b36c51a

update entry for allow_all_headers Co-authored-by: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com>

allow_all_headers: don't test with Rails 4.2

d25f2db

require no Rails or Rails v5.2+ for certain RequestAttribute tests

fallwith commented Aug 10, 2023

View reviewed changes

test/new_relic/agent/transaction/request_attributes_test.rb Outdated Show resolved Hide resolved

kaylareopelle reviewed Aug 10, 2023

View reviewed changes

fallwith and others added 8 commits August 10, 2023 10:44

Update CHANGELOG.md

d106bf7

Grammar rework for allow_all_headers entry Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

Merge branch 'dev' into allow_all_headers

16be793

CI: better test helper name

aabe6c7

label the tin better

CI allow_all_headers requires Rack v2+

f4c8230

Technically allow_all_headers doesn't require Rails v5.2+ but rather Rack v2+

allow_all_headers CHANGELOG update

6b64dd4

- Note about Rack version requirements - Note about attribute names

CI: better Rack v2+ checking

7c0d3fe

Insist on `Rack.release` and non 1.x

Merge pull request #2159 from newrelic/rack_2plus

7ceb153

CI allow_all_headers requires Rack v2+

kaylareopelle reviewed Aug 11, 2023

View reviewed changes

fallwith and others added 4 commits August 11, 2023 10:56

Update CHANGELOG.md

5647b71

allow_all_headers entry updates Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

Update CHANGELOG.md

5ef96e5

allow_all_headers entry parameter -> option Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

Update test/new_relic/agent/transaction/request_attributes_test.rb

cec75be

instane -> instance typo fix Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

allow_all_headers: CHANGELOG rework

bebaeba

(as paired on with @kaylareopelle) update the CHANGELOG entry for `allow_all_headers`

kaylareopelle previously approved these changes Aug 14, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

fallwith dismissed kaylareopelle’s stale review via 912d052 August 14, 2023 16:42

Update CHANGELOG.md

912d052

Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>

kaylareopelle approved these changes Aug 14, 2023

View reviewed changes

fallwith merged commit 956f652 into dev Aug 14, 2023

fallwith deleted the allow_all_headers branch August 14, 2023 16:47

fallwith mentioned this pull request Sep 25, 2023

Update attributes configs default values #1091

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow_all_headers config parameter #2155

allow_all_headers config parameter #2155

fallwith commented Aug 9, 2023

kaylareopelle Aug 11, 2023

github-actions bot commented Aug 11, 2023

-  A new `allow_all_headers` configuration parameter brings parity with the Node.js agent (see the [v2.7.0 changelog](https://docs.newrelic.com/docs/release-notes/agent-release-notes/nodejs-release-notes/node-agent-270/)). This configuration parameter defaults to a value of `false`. When set to `true`, and as long as the agent is not operating in high-security mode, all HTTP headers gleaned from a request will be captured and relayed to New Relic instead of the default core set of headers. All existing behavior for `.*attributes.include` and `.*attributes.exclude` configuration parameters will be respected for any desired filtration of the headers when `allow_all_headers` is enabled. This work was done in response to a feature request submitted by community member [@jamesarosen](https://github.com/jamesarosen). Thank you very much, @jamesarosen! [Issue#1029](https://github.com/newrelic/newrelic-ruby-agent/issues/1029)
-  NOTE: The extra headers collected by having `allow_all_headers` enabled requires Rack version 2 or higher.
-  NOTE: The extra headers will appear as attributes prefixed with `request.headers.`
+  A new `allow_all_headers` configuration option brings parity with the Node.js agent (see the [v2.7.0 changelog](https://docs.newrelic.com/docs/release-notes/agent-release-notes/nodejs-release-notes/node-agent-270/)).
+  This configuration option:
+    * Defaults to `false`.
+    * Is not compatible with high-security mode
+    * When set to `true` all HTTP headers gleaned from a request will be captured and relayed to New Relic. By default, only a small group of HTTP headers are sent to New Relic.
+    * Respects all existing behavior for `attributes.include` and `attributes.exclude` configuration options for any desired filtration of the headers when `allow_all_headers` is enabled.
+    *  Requires Rack version 2 or higher.
+    * Displays the additional headers as transaction attributes prefixed with `request.headers.`
+  This work was done in response to a feature request submitted by community member [@jamesarosen](https://github.com/jamesarosen). Thank you very much, @jamesarosen! [Issue#1029](https://github.com/newrelic/newrelic-ruby-agent/issues/1029)

allow_all_headers config parameter #2155

allow_all_headers config parameter #2155

Conversation

fallwith commented Aug 9, 2023

kaylareopelle Aug 11, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 11, 2023

SimpleCov Report