Skip to content

feat: Remove unused data from session trace requests #570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

patrickhousley
Copy link
Contributor

@patrickhousley patrickhousley commented Jun 6, 2023

Data defined by APM agents and the customer will no longer be included in session trace requests since the data is not consumed downstream and can result in 414 errors in cases where the custom data would cause the URL to exceed the maximum length of 8,192 bytes.

Overview

Removed the ja, ua, and at query parameters from the session trace network requests. These parameters, especially ja, could grow so large that it would cause the resulting URL to exceed the 8192 bytes limit. This data was never actually consumed downstream and doesn't need to be sent on any session trace request.

Related Issue(s)

https://issues.newrelic.com/browse/NEWRELIC-7997

Testing

The integration tests have been updated to remove reliance on these properties. To test locally, build the agent and inject it into a test project. Verify in NR that session traces are still getting created.

BEGIN_COMMIT_OVERRIDE
feat: Remove unused data from session trace requests
END_COMMIT_OVERRIDE

@patrickhousley patrickhousley added small Small Engineering Effort MMF Work directly planned for in an MMF safe to test labels Jun 6, 2023
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #570 (fa212e3) into main (937812a) will increase coverage by 0.04%.
The diff coverage is 0.00%.

❗ Current head fa212e3 differs from pull request most recent head 87022fa. Consider uploading reports for the commit 87022fa to get more accurate results

@@            Coverage Diff             @@
##             main     #570      +/-   ##
==========================================
+ Coverage   26.22%   26.26%   +0.04%     
==========================================
  Files         126      126              
  Lines        4241     4234       -7     
  Branches     1085     1083       -2     
==========================================
  Hits         1112     1112              
+ Misses       2491     2486       -5     
+ Partials      638      636       -2     
Impacted Files Coverage Δ
src/features/session_trace/aggregate/index.js 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@cwli24 cwli24 left a comment

Choose a reason for hiding this comment

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

Verified that atts, userAttributes, and jsAttributes added to the info block still gets sent in RUM request but not on initial Trace request.

@patrickhousley patrickhousley merged commit 276c4f6 into main Jun 7, 2023
@patrickhousley patrickhousley deleted the stn-payload-update branch June 7, 2023 21:15
@bjfield bjfield changed the title feat: remove unused data from stn requests feat: Remove unused data from session trace requests Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MMF Work directly planned for in an MMF small Small Engineering Effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants