Remove CAT by lrafeei · Pull Request #1489 · newrelic/newrelic-python-agent

lrafeei · 2025-09-12T02:16:05Z

This PR removes Cross Application Tracing (CAT) from the agent

github-actions · 2025-09-12T02:17:31Z

✅MegaLinter analysis: Success

Descriptor	Linter	Files	Fixed	Elapsed time
✅ ACTION	actionlint	7		1.11s
✅ MARKDOWN	markdownlint	7	0	1.48s
✅ PYTHON	ruff	961	0	1.15s
✅ PYTHON	ruff-format	961	0	0.34s
✅ YAML	prettier	15	0	1.66s
✅ YAML	v8r	15		5.26s
✅ YAML	yamllint	15		0.67s

See detailed reports in MegaLinter artifacts

codecov-commenter · 2025-09-12T02:26:52Z

Codecov Report

❌ Patch coverage is 92.53731% with 5 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop-v12.0.0@2c71e17). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
newrelic/hooks/framework_aiohttp.py	83.33%	3 Missing ⚠️
newrelic/hooks/framework_sanic.py	33.33%	2 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##             develop-v12.0.0    #1489   +/-   ##
==================================================
  Coverage                   ?   81.74%           
==================================================
  Files                      ?      210           
  Lines                      ?    24361           
  Branches                   ?     3824           
==================================================
  Hits                       ?    19913           
  Misses                     ?     3144           
  Partials                   ?     1304

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Good call--this is still needed for synthetics Co-authored-by: Hannah Stepanek <hstepanek@newrelic.com>

newrelic/api/web_transaction.py

tests/framework_tornado/_target_application.py

TimPansino · 2025-11-25T19:03:08Z

newrelic/hooks/framework_aiohttp.py

-def _nr_process_response(response, transaction):
-    nr_headers = transaction.process_response(response.status, response.headers)
-
-    response._headers.update(nr_headers)


A lot of instrumentation removed from this file, but we still need it. We should be calling process_response to capture the status code for web transactions and the headers as agent attributes.

TimPansino · 2025-11-25T19:34:53Z

tests/framework_aiohttp/test_client_async_await.py

 # limitations under the License.

 import asyncio



Could probably rename this file to just test_client.py now.

Also, we have a test_server_dt.py but no corresponding tests for outbound DT headers in the aiohttp client. We should add tests from this, as we've just removed the test_client_cat.py file that has the only thing remotely resembling tests for this.

TimPansino · 2025-11-25T19:36:53Z

tests/framework_aiohttp/test_client.py

@@ -52,7 +51,8 @@ def task(loop, method, exc_expected, url):
        assert isinstance(text_list[0], _expected_error_class), text_list[0].__class__
        assert isinstance(text_list[1], _expected_error_class), text_list[1].__class__
    else:
-        assert text_list[0] == text_list[1], text_list
+        assert text_list[0]


Why was this fine? Shouldn't we be keeping the behavior of these assertions? (Context: #1489 (comment))

TimPansino · 2025-11-26T17:50:10Z

newrelic/hooks/framework_aiohttp.py

@@ -381,9 +356,6 @@ async def _coro(*_args, **_kwargs):

 def instrument_aiohttp_web(module):
    global _nr_process_response


Suggested change

global _nr_process_response

TimPansino · 2025-11-26T18:04:36Z

newrelic/hooks/framework_aiohttp.py

@@ -279,16 +263,13 @@ async def _coro():
            response = await wrapped(*args, **kwargs)

            try:
-                trace.process_response_headers(response.headers.items())
+                trace.process_response(status_code=response.status)
            except:
                pass

            return response
-        except Exception as e:
-            try:
-                trace.process_response_headers(e.headers.items())
-            except:
-                pass
+        except Exception:
+            notice_error()

            raise


I don't think we want to call notice_error here necessarily, it's not equivalent to what we had before. I think this outer try/except can be deleted and we end up with just the success case.

response = await wrapped(*args, **kwargs) try: trace.process_response(status_code=response.status) except: pass return response

@lrafeei Github was fighting me trying to suggest too many things so I pushed it as a commit, see if you agree with everything here. ad3d4e6

I don't see any issues with that commit

TimPansino · 2025-11-26T18:22:04Z

tests/testing_support/validators/validate_synthetics_event.py


                flat_event = _flatten(event)

-                assert "nr.guid" in flat_event, f"name=nr.guid, event={flat_event!r}"


This shouldn't be deleted

TimPansino · 2025-11-26T18:22:45Z

newrelic/core/transaction_node.py


        apdex_perf_zone = self.apdex_perf_zone()
        _add_if_not_empty("apdexPerfZone", apdex_perf_zone)
-        _add_if_not_empty("nr.apdexPerfZone", apdex_perf_zone)


Are we confident this isn't used in the UI anywhere?

TimPansino · 2025-11-26T18:25:46Z

newrelic/config.py

-    _process_module_definition(
-        "aiohttp.web_reqrep", "newrelic.hooks.framework_aiohttp", "instrument_aiohttp_web_response"
-    )
-    _process_module_definition(
-        "aiohttp.web_response", "newrelic.hooks.framework_aiohttp", "instrument_aiohttp_web_response"
-    )


Don't we still need these?

TimPansino · 2025-11-26T18:37:02Z

tests/testing_support/fixtures.py

                assert intrinsics["error.class"] == required_attrs["error.class"]
                assert intrinsics["error.message"].startswith(required_attrs["error.message"])
                assert intrinsics["error.expected"] == required_attrs["error.expected"]
-                assert intrinsics["nr.transactionGuid"] is not None


As far as I can tell this still exists and we need this line

You're right. Even though this was initially a CAT attribute, it is also used for error events.

TimPansino · 2025-11-26T19:01:32Z

tests/agent_features/test_attributes_in_action.py

 # Test default settings.

-_expected_attributes = {"agent": TRACE_ERROR_AGENT_KEYS, "user": ERROR_USER_ATTRS, "intrinsic": ["trip_id"]}
+_expected_attributes = {"agent": TRACE_ERROR_AGENT_KEYS, "user": ERROR_USER_ATTRS, "intrinsic": ["guid"]}


All of these changes where we deleted trip_id are breaking this file, it's supposed to test for the behavior of intrinsics. I replaced them all with guid and that should solve all those issues.

lrafeei changed the base branch from main to develop-11.0.0 September 12, 2025 02:16

mergify bot added the merge-conflicts Merge conflicts detected. label Sep 12, 2025

mergify bot added the tests-failing Tests failing in CI. label Sep 12, 2025

mergify bot removed the merge-conflicts Merge conflicts detected. label Sep 12, 2025

lrafeei added 24 commits September 12, 2025 15:59

Remove cross_application_tracer.enabled setting

61e2b9a

Remove cat_enabled fixture

e9bab52

Remove validate_outbound_headers, make_cross_agent_headers

fb1b85e

Remove path_hash

4df07f3

Fix tornado tests

e5a0d4e

Fix aiohttp tests

d145940

Fix httpx tests

01da681

Fix pika tests

e3389f3

Fix sanic tests

a32ca51

Remove more CAT tests

5b52a78

Remove nr.guid

931b1c6

Rename synthetics in CatHeaderMixin

c9e7278

Remove process_response_headers

3df9e14

Remove validate_external_node_params

89c9fb5

Rename CatHeaderMixin to HeaderMixin

23ff187

Remove _generate_response_headers

566d250

Remove insert_incoming_headers

521496c

Remove create_incoming_headers

f7bd2ee

Remove X-NewRelic-App-Data

5ed7055

Remove cross_process_id

af451d6

Remove external_txn_name

1adb307

Remove referring_transaction_guid

46436eb

Remove trip_id

855d65e

Remove convert_to_cat_metadata_value

954177d

Fixup: missing import

d5a2458

hmstepanek force-pushed the remove_CAT branch from b718186 to d5a2458 Compare October 24, 2025 01:30

mergify bot removed the tests-failing Tests failing in CI. label Oct 24, 2025

mergify bot and others added 3 commits October 24, 2025 05:05

Merge branch 'develop-v12.0.0' into remove_CAT

f245ebf

Merge branch 'develop-v12.0.0' into remove_CAT

140450f

Apply suggestions from code review

7ba05ce

Good call--this is still needed for synthetics Co-authored-by: Hannah Stepanek <hstepanek@newrelic.com>

mergify bot added the tests-failing Tests failing in CI. label Nov 18, 2025

TimPansino self-assigned this Nov 24, 2025

TimPansino requested changes Nov 25, 2025

View reviewed changes

newrelic/api/web_transaction.py Show resolved Hide resolved

tests/framework_tornado/_target_application.py Show resolved Hide resolved

TimPansino reviewed Nov 25, 2025

View reviewed changes

lrafeei added 2 commits November 25, 2025 17:08

Reviewer requested changes

34fe961

Reviewer requested changes

bda0c5a

TimPansino reviewed Nov 26, 2025

View reviewed changes

TimPansino and others added 2 commits November 26, 2025 10:12

Clean up aiohttp instrumentation

ad3d4e6

[MegaLinter] Apply linters fixes

b4984c8

TimPansino reviewed Nov 26, 2025

View reviewed changes

TimPansino added 9 commits November 26, 2025 10:42

Restore missing assertion

d699965

Restore aiohttp hooks

fc27b94

Add nr.guid back to synthetics validator

94458a1

Simplify test logic

5d4a43f

Clarify process_response return value

177bc65

Remove unused code path

d76b06f

Reorder decorators in tests

c504346

Remove unused imports

4d31395

Fix attributes_in_action tests

e98ce5e

TimPansino reviewed Nov 26, 2025

View reviewed changes

TimPansino and others added 2 commits February 13, 2026 15:43

Merge remote-tracking branch 'origin/develop-v12.0.0' into remove_CAT

428de39

Merge branch 'develop-v12.0.0' into remove_CAT

960e3ed

		@@ -381,9 +356,6 @@ async def _coro(_args, *_kwargs):

		def instrument_aiohttp_web(module):
		global _nr_process_response


		flat_event = _flatten(event)

		assert "nr.guid" in flat_event, f"name=nr.guid, event={flat_event!r}"

Conversation

lrafeei commented Sep 12, 2025

Uh oh!

github-actions bot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅MegaLinter analysis: Success

Uh oh!

codecov-commenter commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimPansino Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimPansino Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Sep 12, 2025 •

edited

Loading

codecov-commenter commented Sep 12, 2025 •

edited

Loading

TimPansino Nov 25, 2025 •

edited

Loading

TimPansino Nov 26, 2025 •

edited

Loading