Calculate time series duration starting from start of period unit

[thanosa75]

Not an issue more of a question really: there are some time ranges that really make sense only from the beginning of the unit, e.g. "month" range should calculate from 1st of month, "week" from Sunday, etc. How can this module be modified or what would be the approach to handle such cases?

Cheers,

Thanos

[risseraka]

If you're looking for a way to modify the module, you might want to check out this line in particular:

getRoundedTime(properties.duration, currentTime - count*properties.duration)

Ref: https://github.com/tonyskn/node-redis-timeseries/blob/master/src/timeseries.js#L100

Right now, the module does a simple "how many Xs" * "X's duration" where X is the granularity. To get timeseries from "1st of month", you'd have to calculate how many days/hours/minutes/seconds since "1st of month" there are and set that as "count".

[thanosa75]

Still buffed by the way the datetime is handled. Example:

granularities: {
'h'    : { ttl: ts.hours(2) , duration: ts.minutes(60) },
'd'    : { ttl: ts.days(2) , duration: ts.hours(24) },
'w'    : { ttl: ts.days(14) , duration: ts.days(7) },
'm'    : { ttl: ts.months(2) , duration: ts.months(1) } }

Although "h" (hour) and "d" (day) work on the exact hour, e.g. 13:00:00Z and 00:00:00Z, the week "w" goes back X days and hits a Thursday. Same for the month.

What is wrong?

[risseraka]

Jan 1st of the year 1970, (UNIX epoch), was a thursday, throwing getRoundedTime computation "off".

new Date(0).toUTCString()
// "Thu, 01 Jan 1970 00:00:00 GMT"
new Date(1000 * 60 * 60 * 24 * 7).toUTCString() // one week later
// "Thu, 08 Jan 1970 00:00:00 GMT"
new Date(Math.round(Date.now() / WEEK) * WEEK).toUTCString()
// "Thu, 19 Feb 2015 00:00:00 GMT"

To "fix" that, you would probably have to do something special for the week, like:

new Date(Math.round(Date.now() / WEEK) * WEEK - 3 * DAY).toUTCString()
// "Mon, 16 Feb 2015 00:00:00 GMT"

[thanosa75]

This was really killing me, thanks for the answer. Shouldn't this however be a bug really for the node-redis-timeseries module?

I would appreciate a solution for this module - and my javascript is not as advanced...

[risseraka]

This issue could be changed as bug. ping @tonyskn & @alph486.

[alph486]

Reading through it looks like we're saying the Month and Week granularities are computing incorrect dates, is that correct and what's being reported?

[risseraka]

I don't know for Month granularity but Week granularity appears to give records from Thursday instead of Monday due to Math.round calculus.

[alph486]

Marked to look into. Thank you for the discussion here.

If either of you are already in that code already you're welcome to make a pull request as well. I'm not sure @tonyskn 's bandwidth but I may not be able to get to it right away...

[thanosa75]

Hi @alph486, @tonyskn

I can confirm that it is the case for both week and month intervals. Weeks tend to start on a Thursday, and Months tend to start on anything between 27-th and 30-th but not 1st. As mentioned in other comments, my JS skills are nothing to be proud of and I am using this on a production system so I would really appreciate if someone could provide a solution to this and help me!

[thanosa75]

Hi guys, any update re/time line? Everyone too busy :(

[risseraka]

Thing aren't that trivial.

This library provides sliding windows timeseries where the last week corresponds to the "last 7 days", not "from beginning of the week to its end". What you need is calendar-based timeseries where when you can query for "this week" or "this month".

The last option is something totally different and a bit more complicated to implement. Maybe this could be done with date libraries like moment.js.

Busy, indeed.

[thanosa75]

Hi Adrien,

Understood but why give the false impression for minutes, hours and days? Στις 9 Μαρ 2015 5:55 μ.μ., ο χρήστης "Adrien Risser" < notifications@github.com> έγραψε:

Thing aren't that trivial.

This library provides sliding windows timeseries where the last week corresponds to the "last 7 days", not "from beginning of the week to its end". What you need is calendar-based timeseries where when you can query for "this week" or "this month".

The last option is something totally different and a bit more complicated to implement. Maybe this could be done with date libraries like moment.js.

Busy, indeed.

— Reply to this email directly or view it on GitHub https://github.com/tonyskn/node-redis-timeseries/issues/11#issuecomment-77881269 .

[risseraka]

Good point, I guess @tonyskn only knows. : )

[alph486]

Hi @thanosa75 - yes if there is a misbehavior here we will have to get to it and will treat this as a bug to investigate. However, if you have already pinpointed it and have good insight into the problem, you're welcome to take a stab at providing a pull request which we can review. You may be the best person to do it if you know the problem :-). It'll be faster for us to review code at this point in time than to develop the solution based on this thread, unfortunately.

Thanks!

[risseraka]

@alph486

I don't think this is necessarily a bug per se.

From the doc:

// Give me the number of hits per day for the last 2 weeks
ts.getHits('your_stats_key', '1day', 14, function(err, data){
    //process the data
});

It really boils down to what "the last X weeks" means. It could be from beginning of the week (is it Sunday or Monday BTW?) or counting 7 days from Date.now().

My two cents.

[alph486]

I see your point. Yes, it is my understanding that this library was originally designed with sort of a "sliding window" as the goal, not necessarily a "predefined start point" as you describe here. So in the example, and in my opinion, its designed to do "past 14 days from right now". If you did "2 weeks" and not "14 days" I would say the answers are to be the same. It's just different ways to express time periods so that you can get the granularity you need for your use case.

I notice before you were mentioning inconsistencies though. That's different. If the above assertion I make about 14 days and 2 weeks being the same did not hold true in your case, there may be something to fix there. Otherwise I think we could hold onto this conversation and consider it an enhancement. If we get some more attention on this topic or realize something's not being communicated properly in the docs, we can implement the changes.

Am I understanding correctly? Does that sound reasonable?

[risseraka]

@alph486 IMHO what's best is indeed clarifying the ambiguity in the docs, first and foremost.

[thanosa75]

As discussed in previous posts, the problem is that the small granularities (hour, day, minute) work from the start of each. Only the larger ones exhibit the issue starting from the week and going up. From a user perspective this is very confusing: I have implemented contact rules on top of this library and the week/month rules are unusable.... Users expect weeks and months to start... as expected :( Στις 13 Μαρ 2015 1:07 π.μ., ο χρήστης "Adrien Risser" < notifications@github.com> έγραψε:

@alph486 https://github.com/alph486 IMHO what's best is indeed clarifying the ambiguity in the docs, first and foremost.

— Reply to this email directly or view it on GitHub https://github.com/tonyskn/node-redis-timeseries/issues/11#issuecomment-78678642 .

[dangerdespain]

If anyone is still interested in this challenge being tackled, I propose that the "count" param in the get function(s) be replaced by an "options" object, containing an optional count & other params.

A couple opts that would be super handy here might be opts.startTime and opts.endTime, so that the queries can be wrangled with as much flexibility as possible. To solve the above problem, moment.js would pair nicely with the user's app to generate the start/endTime options.

The options object is implemented in a different feature build that I have on deck - https://github.com/tonyskn/node-redis-timeseries/pull/13