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

[query] ParseTime function supports 95%+ of from / until time formats #2621

Merged
merged 27 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3fee32d
Added the ParseTimeReference function
teddywahle Sep 9, 2020
27f11a1
Completed the Parse ATTime functionality
teddywahle Sep 10, 2020
d98c5f1
Fixed broken regex
teddywahle Sep 10, 2020
3aaaee5
Merge branch 'master' into parse-time
teddywahle Sep 10, 2020
894d8f2
Update src/query/graphite/graphite/timespec.go
teddywahle Sep 10, 2020
097164c
Merge branch 'master' into parse-time
teddywahle Sep 10, 2020
15f5f68
Merge branch 'master' into parse-time
teddywahle Sep 21, 2020
a8795c3
Update src/query/graphite/graphite/timespec.go
teddywahle Sep 21, 2020
c405440
Apply suggestions from code review
teddywahle Sep 21, 2020
abaa0c0
Apply suggestions from code review
teddywahle Sep 21, 2020
858d337
Apply suggestions from code review
teddywahle Sep 21, 2020
55b55c1
ran go fmt
teddywahle Sep 21, 2020
6f007ed
Added bounds check
teddywahle Sep 21, 2020
b91f6cd
Merge branch 'master' into parse-time
teddywahle Sep 21, 2020
31c1948
Merge branch 'master' into parse-time
teddywahle Sep 23, 2020
a871c89
Update src/query/graphite/graphite/timespec_test.go
teddywahle Sep 23, 2020
93fdbc4
Merge branch 'master' into parse-time
teddywahle Oct 1, 2020
c520419
Apply suggestions from code review
teddywahle Oct 1, 2020
5a49e80
Apply suggestions from code review
teddywahle Oct 1, 2020
6d16b08
Added some (not all) of necessary changes to parse time
teddywahle Oct 1, 2020
eb6eaf7
Added more crons and error checks
teddywahle Oct 1, 2020
caf1795
ran go fmt
teddywahle Oct 1, 2020
c2ae930
Merge branch 'master' into parse-time
teddywahle Oct 1, 2020
800b95e
fixed parse time
teddywahle Oct 2, 2020
d2b3adf
Merge branch 'master' into parse-time
teddywahle Oct 2, 2020
062814c
Merge branch 'master' into parse-time
teddywahle Oct 5, 2020
91cbaeb
added nit changes from code review
teddywahle Oct 5, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 211 additions & 1 deletion src/query/graphite/graphite/timespec.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@ import (
"github.com/m3db/m3/src/query/graphite/errors"
)

var reRelativeTime = regexp.MustCompile(`(?i)^\-([0-9]+)(s|min|h|d|w|mon|y)$`)
var reRelativeTime = regexp.MustCompile(`(?i)^\-([0-9]+)(s|min|h|d|w|mon|y)$`) // allows -3min, -4d, etc.
var reTimeOffset = regexp.MustCompile(`(?i)^(\-|\+)([0-9]+)(s|min|h|d|w|mon|y)$`) // -3min, +3min, -4d, +4d
var reMonthAndDay = regexp.MustCompile(`(?i)^(january|february|march|april|may|june|july|august|september|october|november|december)([0-9]{1,2}?)$`)
var reDayOfWeek = regexp.MustCompile(`(?i)^(sunday|monday|tuesday|wednesday|thursday|friday|saturday)$`)
var reDayOfWeekOffset = regexp.MustCompile(`(?i)^(\-|\+)(sunday|monday|tuesday|wednesday|thursday|friday|saturday)$`) // +monday, +thursday, etc
var rePM = regexp.MustCompile(`(?i)^(([0-1]?)([0-9])pm)([[:ascii:]])*$`) // 8pm, 12pm, 8pm monday
var reAM = regexp.MustCompile(`(?i)^(([0-1]?)([0-9])am)([[:ascii:]])*$`) // 2am, 11am, 7am yesterday
var reTimeOfDayWithColon = regexp.MustCompile(`(?i)^(([0-1]?)([0-9]):([0-5])([0-9])((am|pm)?))([[:ascii:]])*$`) // 8:12pm, 11:20am, 2:00am

var periods = map[string]time.Duration{
"s": time.Second,
Expand All @@ -42,6 +49,31 @@ var periods = map[string]time.Duration{
"y": time.Hour * 24 * 365,
}

var weekdays = map[string]int{
"sunday": 0,
"monday": 1,
"tuesday": 2,
"wednesday": 3,
"thursday": 4,
"friday": 5,
"saturday": 6,
}

var months = map[string]int{
"january": 1,
"february": 2,
"march": 3,
"april": 4,
"may": 5,
"june": 6,
"july": 7,
"august": 8,
"september": 9,
"october": 10,
"november": 11,
"december": 12,
}

// on Jan 2 15:04:05 -0700 MST 2006
var formats = []string{
"15:04_060102",
Expand All @@ -50,6 +82,7 @@ var formats = []string{
"15:04_02.01.06",
"02.01.06",
"01/02/06",
"01/02/2006",
"060102",
"20060102",
}
Expand All @@ -74,6 +107,17 @@ func FormatTime(t time.Time) string {
return t.Format(formats[0])
}

func getWeekDayOffset(weekday string, now time.Time) time.Duration {
expectedDay := weekdays[weekday]
today := int(now.Weekday())
dayOffset := today - expectedDay
if dayOffset < 0 {
dayOffset += 7
}

return time.Duration(dayOffset) * time.Hour * -24
}

// ParseTime translates a Graphite from/until string into a timestamp relative to the provide time
func ParseTime(s string, now time.Time, absoluteOffset time.Duration) (time.Time, error) {
if len(s) == 0 {
Expand Down Expand Up @@ -108,9 +152,153 @@ func ParseTime(s string, now time.Time, absoluteOffset time.Duration) (time.Time
return time.Unix(n, 0).UTC(), nil
}

s = strings.Replace(strings.Replace(strings.ToLower(s), ",", "", -1), " ", "", -1)
ref, offset := s, ""
if strings.Contains(s, "+") {
stringSlice := strings.Split(s, "+")
if len(stringSlice) == 2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this error if len != 2?

ref, offset = stringSlice[0], stringSlice[1]
offset = "+" + offset
}
} else if strings.Contains(s, "-") {
stringSlice := strings.Split(s, "-")
if len(stringSlice) == 2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

ref, offset = stringSlice[0], stringSlice[1]
offset = "-" + offset
}
}
parsedReference, err := ParseTimeReference(ref, now)
robskillington marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
parsedOffset, err := ParseOffset(offset, now)
if err == nil {
return parsedReference.Add(parsedOffset), nil
}
}

return now, err
}

// ParseTimeReference takes a Graphite time reference ("8am", "today", "monday") and returns a time.Time
func ParseTimeReference(ref string, now time.Time) (time.Time, error) {
Copy link
Collaborator

@robskillington robskillington Sep 25, 2020

Choose a reason for hiding this comment

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

Need to add comment for exported symbols. i.e. // ParseTimeReference ...

var (
hour = now.Hour()
minute = now.Minute()
refDate time.Time
)

if ref == "" || ref == "now" {
return now, nil
}

// check if the time ref fits an absolute time format
for _, format := range formats {
t, err := time.Parse(format, ref)
if err == nil {
return t, nil
}
}

rawRef := ref

// Time of Day Reference (8:12pm, 11:20am, 2:00am, etc.)
if reTimeOfDayWithColon.MatchString(rawRef) {
i := strings.Index(ref, ":")
newHour, err := strconv.ParseInt(ref[:i], 10, 0)
if err != nil {
return time.Time{}, err
}
hour = int(newHour)
if len(ref) >= i+3 {
newMinute, err := strconv.ParseInt(ref[i+1:i+3], 10, 32)
if err != nil {
return time.Time{}, err
}
minute = int(newMinute)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this make sure it's not over 60 or is that allowed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I will throw an error and add a test case for this.

if minute > 59 {
return time.Time{}, errors.NewInvalidParamsError(fmt.Errorf("unknown time reference %s", rawRef))
}
ref = ref[i+3:]
}

if len(ref) >= 2 {
if ref[:2] == "am" {
ref = ref[2:]
} else if ref[:2] == "pm" {
hour = (hour + 12) % 24
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this valid for e.g. 23:00PM or should that be a parse error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I will throw an error and add a test case for this.

ref = ref[2:]
}
}
}

// Xam or XXam
if reAM.MatchString(rawRef) {
i := strings.Index(ref, "am")
newHour, err := strconv.ParseInt(ref[:i], 10, 32)
if err != nil {
return time.Time{}, err
}
hour = int(newHour)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this parse something like 10:00pm6am?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Added a check and an error test case for this.

minute = 0
ref = ref[i+2:]
}

// Xpm or XXpm
if rePM.MatchString(rawRef) {
i := strings.Index(ref, "pm")
newHour, err := strconv.ParseInt(ref[:i], 10, 32)
if err != nil {
return time.Time{}, err
}
if newHour > 24 {
return time.Time{}, errors.NewInvalidParamsError(fmt.Errorf("unknown time reference %s", rawRef))
}
hour = int((newHour + 12) % 24)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it correct to parse 99pm or better to fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Added a check and an error test case for this.

minute = 0
ref = ref[i+2:]
}

if strings.HasPrefix(ref, "noon") {
hour, minute = 12, 0
ref = ref[4:]
} else if strings.HasPrefix(ref, "midnight") {
hour, minute = 0, 0
ref = ref[8:]
} else if strings.HasPrefix(ref, "teatime") {
hour, minute = 16, 0
ref = ref[7:]
}

refDate = time.Date(now.Year(), now.Month(), now.Day(), hour, minute, 0, 0, now.Location())

// Day reference
if ref == "yesterday" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these match against the initial ref before it's been shortened?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because this function needs to be able to parse "noon yesterday" which will actually be "noonyesterday" when passed in because whitespace gets stripped. Parsing the unshortened ref could break this.

refDate = refDate.Add(time.Hour * -24)
} else if ref == "tomorrow" {
refDate = refDate.Add(time.Hour * 24)
} else if ref == "today" {
return refDate, nil
} else if reMonthAndDay.MatchString(ref) { // monthDay (january10, may6, may06 etc.)
day := 0
monthString := ""
if val, err := strconv.ParseInt(ref[len(ref)-2:], 10, 64); err == nil {
day = int(val)
monthString = ref[:len(ref)-2]
} else if val, err := strconv.ParseInt(ref[len(ref)-1:], 10, 64); err == nil {
day = int(val)
monthString = ref[:len(ref)-1]
} else {
return refDate, errors.NewInvalidParamsError(fmt.Errorf("day of month required after month name"))
}
refDate = time.Date(refDate.Year(), time.Month(months[monthString]), day, hour, minute, 0, 0, refDate.Location())
} else if reDayOfWeek.MatchString(ref) { // DayOfWeek (Monday, etc)
refDate = refDate.Add(getWeekDayOffset(ref, refDate))
} else if ref != "" {
return time.Time{}, errors.NewInvalidParamsError(fmt.Errorf("unknown time reference %s", rawRef))
}

return refDate, nil
}

// ParseDuration parses a duration
func ParseDuration(s string) (time.Duration, error) {
if m := reRelativeTime.FindStringSubmatch(s); len(m) != 0 {
Expand All @@ -125,3 +313,25 @@ func ParseDuration(s string) (time.Duration, error) {

return 0, errors.NewInvalidParamsError(fmt.Errorf("invalid relative time %s", s))
}

// ParseOffset parses a time offset (like a duration, but can be 0 or positive)
func ParseOffset(s string, now time.Time) (time.Duration, error) {
if s == "" {
return time.Duration(0), nil
}

if m := reTimeOffset.FindStringSubmatch(s); len(m) != 0 {
parity := 1
if m[1] == "-" {
parity = -1
}
timeInteger, err := strconv.ParseInt(m[2], 10, 32)
if err != nil {
return 0, errors.NewInvalidParamsError(fmt.Errorf("invalid time offset %v", err))
}
period := periods[strings.ToLower(m[3])]
return period * time.Duration(timeInteger) * time.Duration(parity), nil
}

return 0, errors.NewInvalidParamsError(fmt.Errorf("invalid time offset %s", s))
}
103 changes: 103 additions & 0 deletions src/query/graphite/graphite/timespec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ func TestParseTime(t *testing.T) {
{"20140307", time.Date(2014, time.March, 7, 0, 0, 0, 0, time.UTC)},
{"140307", time.Date(2014, time.March, 7, 0, 0, 0, 0, time.UTC)},
{"1432581620", time.Date(2015, time.May, 25, 19, 20, 20, 0, time.UTC)},
{"now", time.Date(2013, time.April, 3, 4, 5, 0, 0, time.UTC)},
{"midnight", time.Date(2013, time.April, 3, 0, 0, 0, 0, time.UTC)},
{"midnight+1h", time.Date(2013, time.April, 3, 1, 0, 0, 0, time.UTC)},
{"april08+1d", time.Date(2013, time.April, 9, 4, 5, 0, 0, time.UTC)},
{"monday", time.Date(2013, time.April, 1, 4, 5, 0, 0, time.UTC)},
{"9am monday", time.Date(2013, time.April, 1, 9, 0, 0, 0, time.UTC)},
{"9am monday +5min", time.Date(2013, time.April, 1, 9, 5, 0, 0, time.UTC)},
{"9:00am monday +5min", time.Date(2013, time.April, 1, 9, 5, 0, 0, time.UTC)},
}

for _, test := range tests {
Expand Down Expand Up @@ -74,6 +82,27 @@ func TestParseDuration(t *testing.T) {
}
}

func TestParseOffset(t *testing.T) {
tests := []struct {
timespec string
expectedDuration time.Duration
}{
{"-4h", -4 * time.Hour},
{"-35MIN", -35 * time.Minute},
{"-10s", -10 * time.Second},
{"+4h", 4 * time.Hour},
{"+35MIN", 35 * time.Minute},
{"+10s", 10 * time.Second},
}

for _, test := range tests {
s := test.timespec
parsed, err := ParseOffset(s, relativeTo)
assert.Nil(t, err, "error parsing %s", s)
assert.Equal(t, test.expectedDuration, parsed, "incorrect parsed value for %s", s)
}
}

func TestParseDurationErrors(t *testing.T) {
tests := []string{
"10s",
Expand Down Expand Up @@ -104,3 +133,77 @@ func TestAbsoluteOffset(t *testing.T) {
assert.Equal(t, test.expectedTime, parsed, "incorrect parsed value for %s", s)
}
}

// April 3 2013, 4:05
func TestParseTimeReference(t *testing.T) {
tests := []struct {
ref string
expectedTime time.Time
}{
{"", relativeTo},
{"now", relativeTo},
{"8:50", relativeTo.Add((time.Hour * 4) + (time.Minute * 45))},
{"8:50am", relativeTo.Add((time.Hour * 4) + (time.Minute * 45))},
{"8:50pm", relativeTo.Add((time.Hour * 16) + (time.Minute * 45))},
{"8am", relativeTo.Add((time.Hour * 3) + (time.Minute * 55))},
{"10pm", relativeTo.Add((time.Hour * 17) + (time.Minute * 55))},
{"noon", relativeTo.Add((time.Hour * 7) + (time.Minute * 55))},
{"midnight", relativeTo.Add((time.Hour * -4) + (time.Minute * -5))},
{"teatime", relativeTo.Add((time.Hour * 12) + (time.Minute * -5))},
{"yesterday", relativeTo.Add(time.Hour * 24 * -1)},
{"today", relativeTo},
{"tomorrow", relativeTo.Add(time.Hour * 24)},
{"04/24/13", relativeTo.Add((time.Hour * 24 * 21) + (time.Hour * -4) + (time.Minute * -5))},
{"04/24/2013", relativeTo.Add((time.Hour * 24 * 21) + (time.Hour * -4) + (time.Minute * -5))},
{"20130424", relativeTo.Add((time.Hour * 24 * 21) + (time.Hour * -4) + (time.Minute * -5))},
{"may6", relativeTo.Add(time.Hour * 24 * 33)},
{"may06", relativeTo.Add(time.Hour * 24 * 33)},
{"december17", relativeTo.Add(time.Hour * 24 * 258)},
{"monday", relativeTo.Add(time.Hour * 24 * -2)},
// strings have whitespace removed before being passed into ParseTimeReference
{"8ammonday", relativeTo.Add((time.Hour * 24 * -2) + (time.Hour * 3) + (time.Minute * 55))},
{"10pmyesterday", relativeTo.Add((time.Hour * 17) + (time.Minute * 55) + (time.Hour * 24 * -1))},
}

for _, test := range tests {
ref := test.ref
parsed, err := ParseTimeReference(ref, relativeTo)
assert.Nil(t, err, "error parsing %s", ref)
assert.Equal(t, test.expectedTime, parsed, "incorrect parsed value for %s", ref)
}
}

func TestParseTimeReferenceErrors(t *testing.T) {
tests := []string{
"january800",
"january",
"random",
":",
"8:5",
"99pm",
"12:77pm",
"23:00pm",
"10:00pm6am",
}

for _, test := range tests {
parsed, err := ParseTimeReference(test, relativeTo)
assert.Error(t, err)
assert.Equal(t, time.Time{}, parsed)
}
}

func TestParseOffsetErrors(t *testing.T) {
tests := []string{
"something",
"1m",
"10",
"month",
}

for _, test := range tests {
parsed, err := ParseOffset(test, relativeTo)
assert.Error(t, err)
assert.Equal(t, time.Duration(0), parsed)
}
}