openedx / edx-django-utils

edX utilities for Django Application development.
https://edx.readthedocs.io/projects/edx-django-utils/en/latest/
Apache License 2.0
26 stars 20 forks source link

Fix function tracing for New Relic Agent 5.0.0.124 #27

Closed ormsbee closed 5 years ago

ormsbee commented 5 years ago

Version 5.0.0.124 of the New Relic Agent changed the function tracing API. Using it the old way was resulting in many thousands of new metrics being added (each process invocation of a function trace was creating a new metric name), so we ended up with a bunch of "metrics" named like:

determine_group_permissions/<newrelic.api.web_transaction.WSGIWebTransaction object at 0x7f09bf016ad0> determine_group_permissions/<newrelic.api.web_transaction.WSGIWebTransaction object at 0x7f09bf098510> determine_group_permissions/<newrelic.api.web_transaction.WSGIWebTransaction object at 0x7f09bf0a8bd0> determine_group_permissions/<newrelic.api.web_transaction.WSGIWebTransaction object at 0x7f09bf108b50> determine_group_permissions/<newrelic.api.web_transaction.WSGIWebTransaction object at 0x7f09bf112250>

See: https://docs.newrelic.com/docs/release-notes/agent-release-notes/python-release-notes/python-agent-500124

Description:

Describe in a couple of sentence what this PR adds

JIRA:

XXX-XXXX

Dependencies:

List dependencies on other outstanding PRs, issues, etc.

Merge deadline:

List merge deadline (if any)

Installation instructions:

List any non-trivial installation instructions.

Testing instructions:

  1. Open page A
  2. Do thing B
  3. Expect C to happen
  4. If D happened instead - check failed.

Reviewers:

Merge checklist:

Post merge:

Author concerns:

List any concerns about this PR - inelegant solutions, hacks, quick-and-dirty implementations, concerns about migrations, etc.

ormsbee commented 5 years ago

It's not clear to me how to write a test that is actually meaningful here. Suggestions welcome. :-/

nedbat commented 5 years ago

Wow, super-crappy API design. Change the signature, but silently accept the old arguments and use them to bork the monitoring.

If we had to have a test, it would mock out new relic, report a version, and check the mock call was correct for that version. But I don't think it's worth it.

ormsbee commented 5 years ago

@robrap: How much do we care about the code coverage loss? I could mock it with a test, but it would seriously be creating a mock exactly for this call, and wouldn't actually guard against regressions in the real underlying lib...?