Skip to content
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

Error trace linking #1019

Merged
merged 36 commits into from
Feb 20, 2024
Merged

Error trace linking #1019

merged 36 commits into from
Feb 20, 2024

Conversation

lrafeei
Copy link
Contributor

@lrafeei lrafeei commented Jan 2, 2024

Overview

This PR adds guid as an intrinsic field instead of just having a guid in the error trace when Distributed Tracing is enabled. This links the Transaction Error and the Error Trace even if distributed tracing is off, allowing for the user to see more attributes that are attached to the Error Trace in the Errors Inbox UI.

Copy link

github-actions bot commented Jan 2, 2024

🦙 MegaLinter status: ❌ ERROR

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON bandit 5 0 5.63s
✅ PYTHON black 9 4 0 2.65s
✅ PYTHON flake8 9 0 1.18s
✅ PYTHON isort 9 4 0 0.32s
❌ PYTHON pylint 9 35 9.56s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@mergify mergify bot added the tests-failing label Jan 2, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2024

Codecov Report

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

Comparison is base (57d2236) 81.19% compared to head (738df82) 81.07%.

Files Patch % Lines
newrelic/api/transaction.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1019      +/-   ##
==========================================
- Coverage   81.19%   81.07%   -0.12%     
==========================================
  Files         190      190              
  Lines       19745    19747       +2     
  Branches     3468     3468              
==========================================
- Hits        16031    16009      -22     
- Misses       2724     2743      +19     
- Partials      990      995       +5     

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

@mergify mergify bot removed the tests-failing label Jan 3, 2024
@mergify mergify bot added the tests-failing label Jan 4, 2024
@lrafeei lrafeei changed the base branch from develop-error-trace-UI-linking to main January 4, 2024 19:52
@lrafeei lrafeei marked this pull request as ready for review January 5, 2024 19:21
@lrafeei lrafeei requested a review from a team as a code owner January 5, 2024 19:21
Copy link
Contributor

@TimPansino TimPansino left a comment

Choose a reason for hiding this comment

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

Lots of things to move around, let me know when this is separated and in the meantime I'll have to bring this up with the spec reviewers.

tests/datastore_psycopg2/test_slow_sql.py Show resolved Hide resolved
newrelic/core/stats_engine.py Outdated Show resolved Hide resolved
newrelic/core/stats_engine.py Outdated Show resolved Hide resolved
tests/agent_features/test_distributed_tracing.py Outdated Show resolved Hide resolved
tests/agent_features/test_asgi_distributed_tracing.py Outdated Show resolved Hide resolved
@@ -83,7 +83,6 @@
"parent.transportType",
"parent.transportDuration",
"parentId",
"guid",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's put this line back as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, the distributed tracing intrinsics list goes into an expected absenses attributes list. Since guid is now DT agnostic, it is expected that this make it to the intrinsics list.

Copy link
Contributor

@TimPansino TimPansino left a comment

Choose a reason for hiding this comment

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

Waiting for tests on this one to pass but otherwise it looks good to go

@lrafeei lrafeei merged commit d53f278 into main Feb 20, 2024
47 of 49 checks passed
@lrafeei lrafeei deleted the error_trace_linking branch February 20, 2024 22:03
@mergify mergify bot removed the tests-failing label Feb 20, 2024
TimPansino added a commit that referenced this pull request Feb 21, 2024
* Error trace linking (#1019)

* Add guid to error event & trace intrinsics w/o DT

* Update tests to show guid in intrinsics sans DT

* Fix cross agent DT tests and some pylint errors

* Remove memcached testing for Python 2.7

* Temporarily pin hypercorn to <0.16

* Remove guid from TracedError & add to intrinsics

* Rearrange intrinsics to remove redundancy

* Move guid to make it agnostic to DT

* Change param initialization type

* Use getattr instead of calling root.guid directly

* Revert cross agent tests and add guid to DT as well

* Separate outside transaction error trace linking

* Remove guid from DT specific attribute checks

* Fix slow SQL test

* Remove guid from JSON

* Revert error_trace_collector validator

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Update PyPy in CI to 3.10 (#1065)

* Update PyPy3.8 to PyPy3.10 in CI

* Fix broken utilization test

* Pin CI sha to dev image

* Fix failing sklearn test

* Unroll sklearn changes

* Update falcon tests for 2024 SLAs

* Remove pypy38 from image

* Unpin dev CI image sha

* Change all pypy38 to pypy310

---------

Co-authored-by: Lalleh Rafeei <84813886+lrafeei@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@umaannamalai umaannamalai added this to the v9.7.0 milestone Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants