Add viewfunc custom span tag on Django requests

[timmc-edx]

This ticket needs to be updated to reflect the following from a comment below:

Agreed that we don't want to monkey-patch. I think opening an issue with DRF would be a good idea. So maybe this ticket actually becomes: 1) See if DRF can expose the info; 2) Add a middleware to use the new info and update resource name (behind a flag); 3) Make a feature request for Datadog to use the new info (so we can drop our middleware).

A/C:

IMPORTANT: These AC need to be updated based on above thoughts.

• [ ] edx_django_utils supplies a middleware that adds a span tag with the full viewfunc
• [ ] Teams advised to upgrade and given instructions on how

Datadog's resource name format gives us insufficient information. The class EnterpriseCustomerViewSet contains view methods like enroll_learners_in_courses, toggle_universal_link, and partial_update but Datadog just names the resource enterprise.api.v1.views.enterprise_customer.EnterpriseCustomerViewSet. (New Relic gives the full name, e.g. WebTransaction/Function/enterprise.api.v1.views.enterprise_customer:EnterpriseCustomerViewSet.enroll_learners_in_courses.)

Private ticket with Datadog requesting feature parity: https://help.datadoghq.com/hc/en-us/requests/1694229

Implementation notes:

• Probably want to enhance CachedCustomMonitoringMiddleware since so many IDAs already use it
• We can use request.resolver_match to get the result of Django's own resolve call, and if it's not None, we can get the view_func. This is a function and we can get the __module__ and __name__ from it.
  • DRF ViewSets pose a special problem. The view_func is a function with the same name as the ViewSet class, and doesn't point to a specific view on that class. DRF does a lot of "magic" stuff and the view_func is actually a closure with modified properties. We might be able to get something interesting off of the actions property, or maybe we can contribute something to DRF that will make this easier.
• Tag name TBD
• Tag value should be module.function (for regular views) or module.Class.method (for DRF ViewSet views)

[robrap]

According the DD support ticket, we also have the option of updating the resource name. See details below:

Currently, we only display the class name but without the method for resource names in our Django integration. As a workaround, you can use custom instrumentation to override this behavior. For example, you can include this snippet in you endpoints to override the resource name set by the Django integration:

from ddtrace import tracer
span = tracer.current_span()
if span:
span.resource = ……

Another option is to configure a TraceFilter to override resource names:

from ddtrace import tracer
from ddtrace.filters import TraceFilter

This filter updates the resource on all django.request spans

class RenameDjangoRequestSpan(TraceFilter): def process_trace(self, trace): for span in trace: if span.name == "django.request": url = span.get_tag("http.url") span.resource = # use url to set the resource return trace

Apply the filter to the tracer

tracer.configure(settings={"FILTERS": [RenameDjangoRequestSpan()]})

**UPDATE:** In theory, the benefit of updating the resource_name (in addition to or instead of adding a separate tag) is that this would affect all the resource-based metrics as well. Also, if we put this behind a flag, each service owner could decide if and when to update.

[timmc-edx]

We'd still have to figure out how to extract the DRF view name, but more immediately this would allow us to implement set_monitoring_transaction_name in edx-django-utils, helping solve the XBlock custom resource name issue.

[robrap]

@timmc-edx: Would this be reasonable next steps:

1. Spin off a ticket to implement set_monitoring_transaction_name in edx-django-utils, and communicate with TNL about this affecting the XBlock custom resource name. Note: maybe this would even be a TNL ticket? We'd need to discuss with @jristau1984.
2. Regarding this original ticket, do we agree that we would not want to maintain the monkey patching code required for this? So, we should either close this ticket as won't-do, and/or open an issue with DRF to expose more in their API to make this possible.

Thoughts?

[jristau1984]

I'm not sure what work would need to be done by xblock owning teams, but I believe it would include T&L (HTML, Video, Problem, CAPA), Cosmonauts (LTI), and Aurora (ORA).

[timmc-edx]

@robrap There's already https://github.com/edx/edx-arch-experiments/issues/621 that covers the broader issue of removing direct NR references, but to complete it we'll likely need a separate ticket for enhancing our DD support (for a handful of span-related functions). I've created that ticket now: https://github.com/openedx/edx-django-utils/issues/419 -- we can break out specific parts of it if needed.

Agreed that we don't want to monkey-patch. I think opening an issue with DRF would be a good idea. So maybe this ticket actually becomes: 1) See if DRF can expose the info; 2) Add a middleware to use the new info and update resource name (behind a flag); 3) Make a feature request for Datadog to use the new info (so we can drop our middleware).

[robrap]

Thanks @timmc-edx. I noted in the description the updates that need to happen to this ticket, and we'll groom it sometime in the future.

[robrap]

@jristau1984 @timmc-edx:

• I think this ticket is a P5=>Close as unfinished. So, I'm going to close this as unfinished.
  • I don't think it is worth our time dealing with DRF.
  • If you all disagree, maybe we'll reopen.
• I think https://github.com/openedx/edx-django-utils/issues/419 addresses the work that still needs to happen.