properly name functions

[hiaselhans]

Is your feature request related to a problem? Please describe.

I am using django-prometheus for statistics.
The view calls counters are bundled for function names and thus, all my api calls are in the name of GET /api-1.0.0:ninja.operation._sync_view
I would prefer to have the original function's name or the api-name

Describe the solution you'd like

django-ninja/ninja/operation.py

Lines 314 to 322 in 3d745e7

	def get_view(self) -> Callable:
	view: Callable
	if self.is_async:
	view = self._async_view
	else:
	view = self._sync_view
	view.__func__.csrf_exempt = True # type: ignore
	return view

we could set func.__module__ and func.__name__ here

[elnelsonperez]

@hiaselhans I'm running into a similar problem when trying to get the original function name in a django middleware. Could you please tell me how you solved this issue?

[pseidemann]

how did you fix this? I have the same problem with new relic.

[bqumsiyeh]

We had the same problem with New Relic, and I was able to work around it by implementing some middleware like this:

class NewRelicMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response
        self._set_ninja_new_relic_transaction_names()

    def __call__(self, request):
        response = self.get_response(request)
        return response

    def _set_ninja_new_relic_transaction_names(self):
        """
        Without this, all django-ninja requests/errors show up in NewRelic with a
        transaction name like `ninja.operation:PathView._sync_view`, which sucks for debugging.
        This is a gross hack to fix that, so they are actually grouped in NR correctly,
        by the actual api endpoint function name
        """

        def nr_wrapper(api_fn, operation_run_fn):
            def inner(*args, **kwargs):
                # Properly set the NR transaction name based on the api function
                # and then call the original operation.run()
                transaction_name = f"{api_fn.__module__}.{api_fn.__name__}"
                newrelic.agent.set_transaction_name(transaction_name)
                return operation_run_fn(*args, **kwargs)

            inner._ttam_nr_wrapped = True
            return inner

        all_urls = NewRelicMiddleware._get_all_resolved_urls()
        for url_pattern in all_urls:
            view_func = url_pattern.callback
            is_ninja_request = isinstance(getattr(view_func, "__self__", None), PathView)
            if not is_ninja_request:
                continue

            path_view: PathView = view_func.__self__
            ninja_operation: Operation
            for ninja_operation in path_view.operations:
                if getattr(ninja_operation.run, "_ttam_nr_wrapped", False):  # dont double-wrap the run function
                    continue

                # This is the actual endpoint fn that we care about
                api_function = ninja_operation.view_func

                # Wrap ninja's Operation.run() function so that set the NR transaction name before it runs
                ninja_operation.run = nr_wrapper(api_function, ninja_operation.run)

    @classmethod
    def _get_all_resolved_urls(cls, url_patterns=None):
        """Recursive function that gets all URLs registered with Django"""
        if url_patterns is None:
            # Start with the top level url patterns
            url_patterns = get_resolver().url_patterns

        url_patterns_resolved = []
        for entry in url_patterns:
            if hasattr(entry, "url_patterns"):
                url_patterns_resolved += cls._get_all_resolved_urls(entry.url_patterns)
            else:
                url_patterns_resolved.append(entry)
        return url_patterns_resolved

This is kind of a gross hack though :) @vitalik do you have an ideas on how we could fix this in the library?

Edit: updated the example, because there was a bug in the original implementation

[brianglass]

My first attempt was to try using Newrelic's newrelic.agent.web_transaction decorator, but that didn't seem to work consistently. My workaround to this problem was to write my own decorator. You have to decorate each function, but I only have 4, so this is the simplest approach for me.

...

import functools
import newrelic.agent

...

def instrument_endpoint(view):
    @functools.wraps(view)
    async def wrapped_view(*args, **kwargs):
        transaction_name = f"{view.__module__}.{view.__name__}"
        newrelic.agent.set_transaction_name(transaction_name)
        return await view(*args, **kwargs)

    return wrapped_view

...

@api.get('/{cal:cal}/{year:year}/{month:month}/{day:day}/', response=DaySchema)
@instrument_endpoint
async def get_calendar_day(request, cal: Calendar, year: year, month: month, day: day):
    ...

[vkmasters]

any update or ETA for this ?

[c4ffein]

Tbh, I tried fixing it without reproducing with django-prometheus first, by just writing a UT to check the method name.
I realized only after that, that it seems that django-prometheus is actually working. I tried adding it to my RealWorld Demo App, and I got:
django_http_requests_latency_seconds_by_view_method_count{method="GET",view="api-1.0.0:list_articles"} 15.0
I guess they started using the url_name instead, and so it just works (maybe I'm incorrect, I didn't verify that supposition).

I'd rather not check New Relic honestly, but maybe it should be possible to do something similar and just use the url_name in the logs instead.

Also, we could still provide named functions.
The problem though is we still have to use a common function for all HTTP verbs since that's how the Django resolver works.
IMO the best trade-off would be to generate a function with a name based on the url_name, as it is common to all the verbs (I think simply setting __name__ could cause problems in the case of asynchronous execution).
To ensure this is a correct function name, a first version could:

• replace any non-alphanumeric char by _
• start with _ if there are only numbers
  The linked PR is just a PoC, but it shows how it could be done, and we may iterate on this.

Still, I'm really not sure this is the way to go, maybe we should just expect all logging libraries to be able to use url_name by themselves instead.
The PoC is at #1270

Also,

• Should we set __name__ depending on the name of the first function that was added to this url if url_name was not defined?
• Should we set __module__ depending on the file name that contains the first function that was added to this url?