Remove conversion to UTC in Chargeback

[lpichler]

pulled out from #13398
description is in ^

currently it is tricky to cover it by tests because of zone setting in CI.

cc @jrafanie @gtanzillo

Links

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1587857

[jrafanie]

@lpichler Can you write a test for this? Also, could you summarize the problem and why we're removing the UTC conversion in the PR description and commit message? It's unclear why this was done by just looking at the commit/PR.

From what I'm understanding, it's something like this: "Because we expect resources to be charged for their usage in their local time, we must remove UTC conversion for the report data range to ensure the range starts and ends in local time to avoid incorrect results such as double costs for one month and no charge for the next month"

[jrafanie]

We might want to put a comment above ts = Time.now.in_time_zone(tz) saying something like # Use local time for determining the start and end of the date range since we use and are charged for consumption in local time. Or something that clearly states why we're using local time here.

[lpichler]

@jrafanie it is described here #13398 because we caught it in past time and there is just different time zone used. It is pretty edge case. Look on it and let me if it brings light to you. I did not finish it because it was never ending story to write test for in CI environment for me but there is test and it us briefly describing it in comment in code.

Briefly.
We have month(for example) report and we are splitting time interval for reporting. One interval is month. But in this case the start of month is not in the same month as it suppose to be because of time zone shift and superabundant converting to UTC.

Let me know if it is more understandable.

thanks

[gtanzillo]

The effect of this is that the report is picking up the last day of April in addition to the entire month of May when the report should just be for May. This is what's happening in the code -

2.3.1 :043 > ts = Time.now.in_time_zone("Seoul")
 => Tue, 19 Jun 2018 21:49:31 KST +09:00 
2.3.1 :044 > start_time = (ts - 1.months).beginning_of_month.utc
 => 2018-04-30 15:00:00 UTC 
2.3.1 :045 > end_time   = (ts - 1.months).end_of_month.utc
 => 2018-05-31 14:59:59 UTC 

[JPrause]

@miq-bot add_label blocker

[JPrause]

@lpichler if this can be backported, can you add the gaprindashvili/yes label.

[lpichler]

@miq-bot add_label gaprindashvili/yes

[jrafanie]

@lpichler Document it once in the commit message for future everyone. Looking at this commit/change 6 months from now, I have no idea why we're using local time here. Make your commits clear as to why you're making a change. It's automatically used in the description of the PR (if single commit). The commit message(s) get auto-copied to the BZ on merge so there's no need for you to describe it any further.

[lpichler]

@jrafanie @gtanzillo I described issue in commit message 👍

[miq-bot]

Checked commit lpichler@9dc1e1c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

[lpichler]

I will add test in follow up PR.

[jrafanie]

@lpichler next time, please put the bz link in the commit message so the commit message will be automatically copied to the BZ (and you don't have to explain what you did again in the bz)

[simaishi]

Fine backport details:

$ git log -1
commit 553db93f1aa65a2504c93cfdf23966ddf935da5f
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Jun 20 10:54:38 2018 +0200

    Merge pull request #17606 from lpichler/remove_utc_conversion_from_report_interval_in_chargeback
    
    Remove conversion to UTC in Chargeback
    (cherry picked from commit 9fe07d02c8ee1367b84a0c80a42e25386662349a)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1593678

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit e40b6fe1a8bed02504f463799b9263c50a094984
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Jun 20 10:54:38 2018 +0200

    Merge pull request #17606 from lpichler/remove_utc_conversion_from_report_interval_in_chargeback
    
    Remove conversion to UTC in Chargeback
    (cherry picked from commit 9fe07d02c8ee1367b84a0c80a42e25386662349a)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1593677