Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profile some of the CLE report's methods #35830

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
16 changes: 13 additions & 3 deletions corehq/apps/reports/standard/cases/basic.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import contextlib

from django.utils.translation import gettext as _
from django.utils.translation import gettext_lazy

Expand All @@ -13,7 +15,7 @@
from corehq.apps.reports.generic import ElasticProjectInspectionReport
from corehq.apps.reports.standard import (
ProjectReport,
ProjectReportParametersMixin,
ProjectReportParametersMixin, ESQueryProfilerMixin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: isort.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

)
from corehq.apps.reports.standard.cases.filters import CaseSearchFilter
from corehq.apps.reports.standard.cases.utils import (
Expand All @@ -28,7 +30,7 @@
from .data_sources import CaseDisplayES


class CaseListMixin(ElasticProjectInspectionReport, ProjectReportParametersMixin):
class CaseListMixin(ESQueryProfilerMixin, ElasticProjectInspectionReport, ProjectReportParametersMixin):
fields = [
'corehq.apps.reports.filters.case_list.CaseListFilter',
'corehq.apps.reports.filters.select.CaseTypeFilter',
Expand Down Expand Up @@ -107,7 +109,8 @@ def _build_query(self):
@memoized
def es_results(self):
try:
return self._build_query().run().raw
with self.profiler.timing_context("ES query") if self.profiler_enabled else contextlib.nullcontext():
return self._build_query().run().raw
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: for sake of code clarity can this be extracted in a method and decorated with profile as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except ESError as e:
original_exception = e.args[0]
if isinstance(original_exception, TransportError):
Expand Down Expand Up @@ -234,3 +237,10 @@ def rows(self):
display.modified_on,
display.closed_display
]

@property
def json_response(self):
with self.profiler.timing_context if self.profiler_enabled else contextlib.nullcontext():
response = super().json_response

return response
58 changes: 35 additions & 23 deletions corehq/apps/reports/standard/cases/case_list_explorer.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class CaseListExplorer(CaseListReport, XpathCaseSearchFilterMixin):
documentation_link = DOCS_LINK_CASE_LIST_EXPLORER
use_bootstrap5 = False

profiler_enabled = True
profiler_name = "Case List Explorer"

exportable = True
exportable_all = True
emailable = True
Expand Down Expand Up @@ -93,30 +96,36 @@ def es_results(self):
def _build_query(self, sort=True):
query = super(CaseListExplorer, self)._build_query()
query = self._populate_sort(query, sort)
query = self.apply_xpath_case_search_filter(query)
query = self._apply_xpath_case_search_filter(query)
return query

def _populate_sort(self, query, sort):
if not sort:
# Don't sort on export
query = query.set_sorting_block(['_doc'])
return query

num_sort_columns = int(self.request.GET.get('iSortingCols', 0))
for col_num in range(num_sort_columns):
descending = self.request.GET['sSortDir_{}'.format(col_num)] == 'desc'
column_id = int(self.request.GET["iSortCol_{}".format(col_num)])
column = self.headers.header[column_id]
try:
meta_property = INDEXED_METADATA_BY_KEY[column.prop_name]
if meta_property.key == '@case_id':
# This condition is added because ES 5 does not allow sorting on _id.
# When we will have case_id in root of the document, this should be removed.
query.es_query['sort'] = get_case_id_sort_block(descending)
return query
query = query.sort(meta_property.es_field_name, desc=descending)
except KeyError:
query = query.sort_by_case_property(column.prop_name, desc=descending)
with self.profiler.timing_context("Populate sort"):
if not sort:
# Don't sort on export
query = query.set_sorting_block(['_doc'])
return query

num_sort_columns = int(self.request.GET.get('iSortingCols', 0))
for col_num in range(num_sort_columns):
descending = self.request.GET['sSortDir_{}'.format(col_num)] == 'desc'
column_id = int(self.request.GET["iSortCol_{}".format(col_num)])
column = self.headers.header[column_id]
try:
meta_property = INDEXED_METADATA_BY_KEY[column.prop_name]
if meta_property.key == '@case_id':
# This condition is added because ES 5 does not allow sorting on _id.
# When we will have case_id in root of the document, this should be removed.
query.es_query['sort'] = get_case_id_sort_block(descending)
return query
query = query.sort(meta_property.es_field_name, desc=descending)
except KeyError:
query = query.sort_by_case_property(column.prop_name, desc=descending)
return query

def _apply_xpath_case_search_filter(self, query):
with self.profiler.timing_context("Apply Xpath case filter"):
query = self.apply_xpath_case_search_filter(query)
return query

@property
Expand Down Expand Up @@ -173,8 +182,11 @@ def headers(self):
@property
def rows(self):
self.track_search()
data = (wrap_case_search_hit(row) for row in self.es_results['hits'].get('hits', []))
return self._get_rows(data)

with self.profiler.timing_context("Retrieving rows"):
data = (wrap_case_search_hit(row) for row in self.es_results['hits'].get('hits', []))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could consider tracking "Fetching results from ES" and "Wrapping result" as two different metrics, just to be very specific about how long ES took.

with self.profiler.timing_context("Parsing rows"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: preparing data for display?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that. I'll update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return self._get_rows(data)

def track_search(self):
track_workflow(
Expand Down