Use url path instead of view name?

[ChillarAnand]

@catcombo I have been testing out speedinfo with several projects and it is working well. Thank you for your efforts.

Any reasons to use view_name instead of URL paths to collect statistics?

One issue with view names is, resolve can't give full function name.
For example if there is a function at core.viewsets.BookViewSet.dashboard, view name will be just core.viewsets.BookViewSet. All the methods in the view set will have same view name. This results in combining stats of all views inside a model view set.

Another issue with view names is, when profiling views in admin, it will always show same view names like django.contrib.admin.options, django.contrib.admin.options.changelist_view
irrespective of the model a user is visiting.

I have made a temporary fix to test it out 7bfc2c3

[catcombo]

One issue with view names is, resolve can't give full function name.
For example if there is a function at core.viewsets.BookViewSet.dashboard, view name will be just core.viewsets.BookViewSet. All the methods in the view set will have same view name. This results in combining stats of all views inside a model view set.

Statistics in speedinfo are grouped by unique pair of view_name and HTTP method name. Thus, you get multiple rows in ViewProfiler admin for the same view name, but with different HTTP method names. This is enough in most cases to indicate what viewset method was called (get method for GET requests, post method for POST and so on). You can see the example on screenshot for accounts.views.AccountRegistrationView.

But in case If you add extra methods to handle requests with an additional URL suffix this may cause the problem you described above (example of what I'm talking about speaking of DRF). In this case I would recommend you to split methods into separate viewsets to isolate their functionality. Thus you will be able to get desired statistics for each view separately.

Another issue with view names is, when profiling views in admin, it will always show same view names like django.contrib.admin.options, django.contrib.admin.options.changelist_view
irrespective of the model a user is visiting.

I have made a temporary fix to test it out 7bfc2c3

There may be situations when the same view handles requests from the different URLs. That's why speedinfo uses view_name instead of URL. And that's why you get the same view name when profiling Django admin. For me it's more important to find most high loaded view rather than URL. URL doesn't tell me enough. Also I never profile the admin, it doesn't matter for most cases - you can't influence it performance in contrast to your code. I recommend to add admin URL to SPEEDINFO_EXCLUDE_URLS only if you don't have a specific reason not to do so.

Regarding to your fix. Keep in mind that you should also cut off query parameters for URL. Otherwise every page with a query parameter will profiling separately (e.g. /accounts/, /accounts/?page=2, /accounts/?q=peter&page=3 and so on).

[ChillarAnand]

But in case If you add extra methods to handle requests with an additional URL suffix this may cause the problem you described above (example of what I'm talking about speaking of DRF). In this case I would recommend you to split methods into separate viewsets to isolate their functionality. Thus you will be able to get desired statistics for each view separately.

In one of the project, there are several model viewsets, each with 20+ methods. This is causing lot of problems with speedinfo stats.

There may be situations when the same view handles requests from the different URLs. That's why speedinfo uses view_name instead of URL.

I see.

Regarding to your fix. Keep in mind that you should also cut off query parameters for URL. Otherwise every page with a query parameter will profiling separately (e.g. /accounts/, /accounts/?page=2, /accounts/?q=peter&page=3 and so on

Good catch.

In that case, speedinfo can show full function name path instead of just class name?

[catcombo]

Good job! Unfortunately it will not work well.

class PostViewSet(ModelViewSet):
    queryset = Post.objects.all()

    @action(detail=True, methods=["post"])
    def images(self, request, *args, **kwargs):
    	...
        return Response(serializer.data, status=status.HTTP_201_CREATED)

    @images.mapping.delete
    def delete_image(self, request, *args, **kwargs):
    	...
        return Response(status=status.HTTP_204_NO_CONTENT)

resolver.func.actions will return {'post': 'images', 'delete': 'delete_image'} for this view.

Your name resolver will always return accounts.views.PostViewSet.images for this example even if requesting posts list GET /posts/ or image delete DELETE /posts/123/images/ pages. This could be fixed as follows:

http_method_name = request.method.lower()
actions_map = resolver.func.actions.values()
view_method_name = actions_map.get(http_method_name, "")

But it will not work too. Because if we requesting POST /posts/ or DELETE /posts/123/ method_name will be assigned to images and delete_image accordingly which is not correct. Need to write more complex DRF-specific name resolver.

In that case, speedinfo can show full function name path instead of just class name?

I prefer stick to view name, anyway.

[ChillarAnand]

Because if we requesting POST /posts/ or DELETE /posts/123/ method_name will be assigned to images and delete_image accordingly which is not correct.

Can you explain why this is not correct?
I thought it is the correct behavior.
POST /posts/ -> foo.PostViewSet.images
DELETE /posts/123/ -> foo.PostViewset.delete_image

[catcombo]

Can you explain why this is not correct?
I thought it is the correct behavior.
POST /posts/ -> foo.PostViewSet.images

POST /posts/ adds a new Post -> view_method_name should be foo.PostViewSet
POST /posts/123/images/ adds an image to the specified Post -> foo.PostViewSet.images

DELETE /posts/123/ -> foo.PostViewset.delete_image

Same here:

DELETE /posts/123/ deletes specified Post -> foo.PostViewSet
DELETE /posts/123/images/ deletes an attached image -> foo.PostViewSet.delete_image