opentelemetry-python-contrib icon indicating copy to clipboard operation
opentelemetry-python-contrib copied to clipboard

Route used in Django span name is resolved incorrectly and inefficiently

Open alexmojaki opened this issue 1 year ago • 3 comments
trafficstars

See the conversation in https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2624/files#r1646685931 corresponding to changes in https://github.com/open-telemetry/opentelemetry-python-contrib/commit/dc42bdd986cd136559fda6a3032061d4eed7e517#r1646685931

In summary, only using the route obtained in process_view is more reliably correct and more efficient than calling resolve(request.path) when the span is started. The only reason to do the latter is so that the span name contains the route when it's started so samplers can take this into account.

In my opinion, a better behaviour would be:

  1. Add a new attribute (e.g. django.request.path_info) with the value of request.path_info. This is expected to equal http.target (which BTW isn't set and should be) or maybe a suffix of it. Samplers can use this to resolve the route if they want.
  2. Don't resolve the route when the span is started. Only use http.method as the initial span name.
  3. After getting the route in process_view, use Span.update_name to add the route to the span name.

If this change is undesirable, then at least resolve(request.path) should be changed to resolve(request.path_info).

alexmojaki avatar Jul 16 '24 11:07 alexmojaki