711233 - Use full days in date range in search

Status: VERIFIED WONTFIX

(Socorro :: General, task)

Description

[[DEACTIVATED] Adrian Gaudebert]

When search was moved to the middleware, I changed the way date ranges where created. It used to take full days (meaning that a date would either be 2011-11-11 00:00:00 or 2011-11-11 23:59:59), and it now uses precise dates. This led to inconsistency across different pages, query showing different numbers of crashes than report/list or tcbs. 

We need to roll back to the old way. For postgresql it means using the SQL functions utc_day_begins_pacific and utc_day_ends_pacific, for ElasticSearch it means simply removing the date range as we have one index per day and we use only the concerned indexes.

Comment 1

[[DEACTIVATED] Adrian Gaudebert]

The reason why I removed that behavior is that we can search for only a few hours of crash reports. Say that a user wants to see the crashes that happened in the last 3 hours (because we shipped a new Firefox 3 hours ago), changing the date range to be an entire day will make the feature useless. 

I think I'm a bit lost, so here is my understanding: the date_processed field of a crash report, which we rely on a lot, is a PST date. TCBS uses UTC dates through a date conversion. Search and report/list used to change each date range to UTC start of day and UTC end of day, meaning that a date like 2011-11-11 12:23:34 would become 2011-11-10 16:00:00 if it's the start date. 

When I moved search to the middleware, I removed that limitation and made ranges what they are from the user point of view. But report/list remained the same, and we had inconsistency between search, report/list and tcbs. 

Now I'm in the process of moving report/list to the middleware too, and my first move was to remove that UTC conversion. If I do so, search and report/list will be consistent, but TCBS will have different numbers. I do think this is the best solution, because it allows for very precise search options. I don't think consistency between tcbs and report/list matters more than consistency between search and report/list, but I may be wrong. I so please correct me. 

From what Josh told me, there is no influence on performances here, so we can do what we want. We have the choice to value (1) consistency everywhere or (2) precise options. 

(1) implies technical difficulties and eventual performance degradation with ElasticSearch. 
(2) implies TCBS' numbers won't match those from report/list and search, but is simpler to implement with ES (it already works this way). 

I personally recommend going for number (2). KaiRo, Laura, what do you think?

Comment 2

[Robert Helmer [:rhelmer]]

(In reply to Adrian Gaudebert [:adrian] from comment #1)

We just discussed in IRC, I think the clearest thing to do would be to expose timezone in the UI, and allow it to be specified in the UI also (whether the default should be UTC or Pacific is up for debate :) could be hardcoded to one of these, or try to detect the browser's TZ).

Comment 3

[Robert Kaiser]

(In reply to Adrian Gaudebert [:adrian] from comment #1)
> Now I'm in the process of moving report/list to the middleware too, and my
> first move was to remove that UTC conversion. If I do so, search and
> report/list will be consistent, but TCBS will have different numbers. 

This sounds good as long as the links to reports/list from TCBS will reflect the time range of that particular TCBS report.

Comment 4

[[:jberkus] Josh Berkus]

TCBS will be in UTC, no matter what the user's time zone is.  Doing otherwise would require major refactoring at this point.

Comment 5

[Robert Helmer [:rhelmer]]

(In reply to [:jberkus] Josh Berkus from comment #4)
> TCBS will be in UTC, no matter what the user's time zone is.  Doing
> otherwise would require major refactoring at this point.

Didn't mean to suggest that, just suggesting that we can convert it to the user's preferred timezone in the UI, and continue to use UTC internally (or anything we want, as long as we know the timezone the data is stored in - UTC would be ideal so we avoid DST bugs and the like.)

Comment 6

[[DEACTIVATED] Adrian Gaudebert]

Opened bug 712586 for the way we decided to go. We won't do what was initially said here.

Comment 7

[Matt Brandt [:mbrandt]]

moving to verified per comment 6 and bug 712586.