"Advanced" Date Time in Time Range Filter is Incorrect

[sam-hieken]

Bug description

Specifying "Last minutes" in the "Advanced" section of time range filters on dashboards will sometimes think it's a time from the day before.

Screenshots/recordings

For some clarity, the following test was performed on 2024-10-13T19:38:00-0400, give or take a few seconds. As should be obvious by the -0400, I'm in EDT/EST zone.

Last Hour:

Last 2 Minutes:

Obviously "Last 2 minutes" is way off - the timestamp is nowhere close, and the date shown is yesterday. Every dashboard with a time range filter I've tried this on has the same issue, and it happens in both the Start and End inputs.

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Firefox

Additional context

No response

Checklist

• [X] I have searched Superset docs and Slack and didn't find a solution to my problem.
• [X] I have searched the GitHub issue tracker and didn't find a similar bug report.
• [X] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

[dosubot[bot]]

The issue you're experiencing with the "Last minutes" time range filter might be related to timezone handling. Superset uses the timezone-mock library to simulate different timezones and calculate time offsets. This can sometimes lead to discrepancies if the timezone isn't correctly accounted for in the calculations ^[1].

To address this, you might want to check if the timezone settings in your Superset configuration are correctly set to match your local timezone (EDT/EST). Additionally, ensure that both your database and Superset backend are configured to use UTC, as this can help avoid issues with temporal fields lacking explicit timezones ^[2].

If the problem persists, consider reviewing the logic in the get_since_until function, which calculates start and end times for relative time ranges. This function uses parameters like relative_start, relative_end, and time_shift to determine the correct time range ^[3].

To continue talking to Dosu, mention @dosu.

[sam-hieken]

@dosu Nope, changed both my database timezone and superset timezone (DEFAULT_TIMEZONE in superset_config.py) to use EST/EDT: same issue. Did the exact same thing but UTC for both Superset and my database: still the same issue.

[dosubot[bot]]

Superset assumes timestamps are in UTC, and configuring the backend to a non-UTC timezone like EST/EDT can lead to issues. It's recommended to keep both the database and Superset backend in UTC to ensure consistency ^[1].

If you've already set both to UTC and the issue persists, it might be worth checking if there are any specific configurations or overrides in your setup that could affect timezone handling. Additionally, reviewing the logic in the get_since_until function might help identify any discrepancies in how relative time ranges are calculated.

To continue talking to Dosu, mention @dosu.

[rusackas]

I wonder if the underlying library (which I can't recall the name of) is interpreting "last two minutes" as the "last two minutes of the day" rather than the "last two minutes that occurred." In that case, it's right... and it's kind of a language/clarity failure. What happens if you say "prior two minutes" or "two minutes ago"?

[yousoph]

Hi @sam-hieken ! Thanks for filing this issue! I was about to report something very similar, hope you don't mind me making some edits/additions to your original issue to include some more details and other cases where the advanced filters aren't working.

[yousoph]

I wonder if the underlying library (which I can't recall the name of) is interpreting "last two minutes" as the "last two minutes of the day" rather than the "last two minutes that occurred." In that case, it's right... and it's kind of a language/clarity failure. What happens if you say "prior two minutes" or "two minutes ago"?

It seems to be using the last 2 minutes of yesterday though, which seems like a strange interpretation if that's what's happening. Plus the "last hour" uses the start of the hour before the current hour so that one isn't using yesterday.

You're right that 2 minutes ago does work as expected though so that could be a workaround for the interim

[sam-hieken]

Hi @yousoph, no worries, please feel free!

[sam-hieken]

@rusackas So I've found that "2 minutes ago" works as desired like @yousoph said, but "Prior 2 minutes" will actually go 2 minutes into the future:

I'm not familiar with Superset's codebase, but I've narrowed down the problem to the /api/v1/time_range endpoint in the backend. For example, in the above image, the following request was produced to parse the plain language into a date:

Endpoint: /api/v1/time_range/?q='now : Prior 2 minutes' Response:

{
    "result": {
        "since": "2024-11-22T15:16:19",
        "until": "2024-11-22T15:18:19",
        "timeRange": "now : Prior 2 minutes"
    }
}

[aybanda]

Hey @geido can you assign this issue to me ?

[geido]

Hi @aybanda how is this going?

[aybanda]

Hey @geido I will be generating a PR very shortly.

[Niharika0104]

@geido is this still open I see a pr but it's closed can I work on this?

[fzh075]

I'm in line behind @Niharika0104.

[alexandrusoare]

Hey @geido can I have a look at this issue?

[Antonio-RiveroMartnez]

Hey all! My two cents on this:

1. All Human dates should be handled either by the current library or some extra addition of better ones, but we should not be handling specific strings like "Prior" etc, otherwise maintaining this API can quickly become cumbersome, even more, when we have already things like "ago" in our code base to address "past" dates
2. This API is extensively used in Superset and thus, changes on it would be considered breaking changes, for example, right now Last 2 Hours is expected to take the relative start of today, changing this could break existing charts/dashboards. Not saying that it's semantically correct to take today, just that it's a breaking change.

[geido]

@Antonio-RiveroMartnez

Not saying that it's semantically correct to take today, just that it's a breaking change

If we all agree this is not the correct behaviour then this is likely a bug, thus a bug fix that does not quality as a breaking change.

[geido]

@sam-hieken curious if you had a chance to have a look at this PR https://github.com/apache/superset/pull/31867 and if you believe this fixes the problems that you are mentioning.

[sam-hieken]

Hey there @geido, just took a look at the before and after changes in the PR and it looks like that solved the issue, though unfortunately I don't have enough resources to test the changes myself. Thanks for taking a look at this!