Merge pull request #113 from quickwit-oss/ddelemeny/revamp-parse-time

Revamp time parser function

Author: ddelemeny

Diff for: go.mod

@@ -38,6 +38,7 @@ require (
 	github.com/hashicorp/go-plugin v1.4.9 // indirect
 	github.com/hashicorp/yamux v0.1.1 // indirect
 	github.com/invopop/yaml v0.1.0 // indirect
+	github.com/itchyny/timefmt-go v0.1.5
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/klauspost/compress v1.13.1 // indirect

Diff for: go.sum

@@ -211,6 +211,8 @@ github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbg
 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
 github.com/invopop/yaml v0.1.0 h1:YW3WGUoJEXYfzWBjn00zIlrw7brGVD0fUKRYDPAPhrc=
 github.com/invopop/yaml v0.1.0/go.mod h1:2XuRLgs/ouIrW3XNzuNj7J3Nvu/Dig5MXvbCEdiBN3Q=
+github.com/itchyny/timefmt-go v0.1.5 h1:G0INE2la8S6ru/ZI5JecgyzbbJNs5lG1RcBqa7Jm6GE=
+github.com/itchyny/timefmt-go v0.1.5/go.mod h1:nEP7L+2YmAbT2kZ2HfSs1d8Xtw9LY8D2stDBckWakZ8=
 github.com/jhump/protoreflect v1.6.0 h1:h5jfMVslIg6l29nsMs0D8Wj17RDVdNYti0vDN/PZZoE=
 github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=

Diff for: pkg/quickwit/response_parser.go

@@ -12,10 +12,10 @@ import (
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
-	"golang.org/x/exp/slices"
 
 	es "github.com/quickwit-oss/quickwit-datasource/pkg/quickwit/client"
 	"github.com/quickwit-oss/quickwit-datasource/pkg/quickwit/simplejson"
+	utils "github.com/quickwit-oss/quickwit-datasource/pkg/utils"
 )
 
 const (
@@ -247,7 +247,7 @@ func processDocsToDataFrameFields(docs []map[string]interface{}, propNames []str
 		if propName == configuredFields.TimeField {
 			timeVector := make([]*time.Time, size)
 			for i, doc := range docs {
-				timeValue, err := ParseToTime(doc[configuredFields.TimeField], configuredFields.TimeOutputFormat)
+				timeValue, err := utils.ParseTime(doc[configuredFields.TimeField], configuredFields.TimeOutputFormat)
 				if err != nil {
 					continue
 				}
@@ -291,44 +291,6 @@ func processDocsToDataFrameFields(docs []map[string]interface{}, propNames []str
 	return allFields
 }
 
-// Parses a value into Time given a timeOutputFormat. The conversion
-// only works with float64 as this is what we get when parsing a response.
-// TODO: understand why we get a float64?
-func ParseToTime(value interface{}, timeOutputFormat string) (time.Time, error) {
-
-	if timeOutputFormat == Iso8601 || timeOutputFormat == Rfc3339 {
-		value_string := value.(string)
-		timeValue, err := time.Parse(time.RFC3339, value_string)
-		if err != nil {
-			return time.Time{}, err
-		}
-		return timeValue, nil
-	} else if timeOutputFormat == Rfc2822 {
-		value_string := value.(string)
-		timeValue, err := time.Parse(time.RFC822Z, value_string)
-		if err != nil {
-			return time.Time{}, err
-		}
-		return timeValue, nil
-	} else if slices.Contains([]string{TimestampSecs, TimestampMillis, TimestampMicros, TimestampNanos}, timeOutputFormat) {
-		typed_value, ok := value.(float64)
-		if !ok {
-			return time.Time{}, errors.New("parse time only accepts float64 with timestamp based format")
-		}
-		int64_value := int64(typed_value)
-		if timeOutputFormat == TimestampSecs {
-			return time.Unix(int64_value, 0), nil
-		} else if timeOutputFormat == TimestampMillis {
-			return time.Unix(0, int64_value*1_000_000), nil
-		} else if timeOutputFormat == TimestampMicros {
-			return time.Unix(0, int64_value*1_000), nil
-		} else if timeOutputFormat == TimestampNanos {
-			return time.Unix(0, int64_value), nil
-		}
-	}
-	return time.Time{}, fmt.Errorf("timeOutputFormat not supported yet %s", timeOutputFormat)
-}
-
 func processBuckets(aggs map[string]interface{}, target *Query,
 	queryResult *backend.DataResponse, props map[string]string, depth int) error {
 	var err error

Diff for: pkg/quickwit/response_parser_qw_test.go

@@ -278,11 +278,3 @@ func TestProcessLogsResponseWithDifferentTimeOutputFormat(t *testing.T) {
 		require.Equal(t, &expectedTimeValue, logsFieldMap["testtime"].At(0))
 	})
 }
-
-func TestConvertToTime(t *testing.T) {
-	t.Run("Test parse unix timestamps nanosecs of float type", func(t *testing.T) {
-		inputValue := interface{}(1234567890000000000.0)
-		value, _ := ParseToTime(inputValue, "unix_timestamp_nanos")
-		require.Equal(t, time.Unix(1234567890, 0), value)
-	})
-}

Diff for: pkg/utils/parse_time.go

@@ -0,0 +1,85 @@
+package utils
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"time"
+
+	timefmt "github.com/itchyny/timefmt-go"
+)
+
+const (
+	Iso8601         string = "iso8601"
+	Rfc2822         string = "rfc2822"  // timezone name
+	Rfc2822z        string = "rfc2822z" // explicit timezone
+	Rfc3339         string = "rfc3339"
+	TimestampSecs   string = "unix_timestamp_secs"
+	TimestampMillis string = "unix_timestamp_millis"
+	TimestampMicros string = "unix_timestamp_micros"
+	TimestampNanos  string = "unix_timestamp_nanos"
+)
+
+const Rfc2822Layout string = "%a, %d %b %Y %T %Z"
+const Rfc2822zLayout string = "%a, %d %b %Y %T %z"
+
+// Parses a value into Time given a timeOutputFormat. The conversion
+// only works with float64 as this is what we get when parsing a response.
+func ParseTime(value any, timeOutputFormat string) (time.Time, error) {
+	switch timeOutputFormat {
+	case Iso8601, Rfc3339:
+		value_string := value.(string)
+		timeValue, err := time.Parse(time.RFC3339, value_string)
+		if err != nil {
+			return time.Time{}, err
+		}
+		return timeValue, nil
+
+	case Rfc2822:
+		// XXX: the time package's layout for RFC2822 is bogus, don't use that.
+		value_string := value.(string)
+		timeValue, err := timefmt.Parse(value_string, Rfc2822Layout)
+		if err != nil {
+			return time.Time{}, err
+		}
+		return timeValue, nil
+	case Rfc2822z:
+		// XXX: the time package's layout for RFC2822 is bogus, don't use that.
+		value_string := value.(string)
+		timeValue, err := timefmt.Parse(value_string, Rfc2822zLayout)
+		if err != nil {
+			return time.Time{}, err
+		}
+		return timeValue, nil
+
+	case TimestampSecs, TimestampMillis, TimestampMicros, TimestampNanos:
+		var value_i64 int64
+		switch value.(type) {
+		case int, int8, int16, int32, int64:
+			value_i64 = reflect.ValueOf(value).Int()
+		case float32, float64:
+			value_f64 := reflect.ValueOf(value).Float()
+			value_i64 = int64(value_f64)
+		default:
+			return time.Time{}, errors.New("parseTime only accepts float64 or int64 values with timestamp based formats")
+		}
+
+		if timeOutputFormat == TimestampSecs {
+			return time.Unix(value_i64, 0), nil
+		} else if timeOutputFormat == TimestampMillis {
+			return time.Unix(0, value_i64*1_000_000), nil
+		} else if timeOutputFormat == TimestampMicros {
+			return time.Unix(0, value_i64*1_000), nil
+		} else if timeOutputFormat == TimestampNanos {
+			return time.Unix(0, value_i64), nil
+		}
+	default:
+		value_string := value.(string)
+		timeValue, err := timefmt.Parse(value_string, timeOutputFormat)
+		if err != nil {
+			return time.Time{}, err
+		}
+		return timeValue, nil
+	}
+	return time.Time{}, fmt.Errorf("timeOutputFormat not supported yet %s", timeOutputFormat)
+}

Diff for: pkg/utils/parse_time_test.go

@@ -0,0 +1,74 @@
+package utils
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	testYear        int = 2024
+	testMonth       int = 3
+	testDay         int = 28
+	testHour        int = 12
+	testMinute      int = 34
+	testSecond      int = 56
+	testUnixSeconds int = 1711629296
+	testMilli       int = testUnixSeconds*1000 + 987
+	testMicro       int = testMilli*1000 + 654
+	testNano        int = testMicro*1000 + 321
+)
+
+var successTests = []struct {
+	value            any
+	timeOutputFormat string
+}{
+	// RFC3339
+	{"2024-03-28T12:34:56.987Z", Rfc3339},
+	// RFC2822
+	{"Thu, 28 Mar 2024 12:34:56 GMT", Rfc2822},
+	{"Thu, 28 Mar 2024 12:34:56 +0000", Rfc2822z},
+	// Custom layout
+	{"2024-03-28 12:34:56", "%Y-%m-%d %H:%M:%S"},
+	{"2024-03-28 12:34:56.987", "%Y-%m-%d %H:%M:%S.%f"},
+	// Int timestamps
+	{1711629296, TimestampSecs},
+	{1711629296987, TimestampMillis},
+	{1711629296987654, TimestampMicros},
+	{1711629296987654321, TimestampNanos},
+	// Float timestamps
+	{1711629296., TimestampSecs},
+	{1711629296987., TimestampMillis},
+	{1711629296987654., TimestampMicros},
+	// {1711629296987654321., TimestampNanos}, // Float precision fail
+}
+
+func TestParseTime(t *testing.T) {
+	assert := assert.New(t)
+	for _, tt := range successTests {
+		t.Run(fmt.Sprintf("Parse %s", tt.timeOutputFormat), func(t *testing.T) {
+			time, err := ParseTime(tt.value, tt.timeOutputFormat)
+			assert.Nil(err)
+			assert.NotNil(time)
+			// Check day
+			assert.Equal(testYear, int(time.UTC().Year()), "Year mismatch")
+			assert.Equal(testMonth, int(time.UTC().Month()), "Month mismatch")
+			assert.Equal(testDay, int(time.UTC().Day()), "Day mismatch")
+			assert.Equal(testHour, int(time.UTC().Hour()), "Hour mismatch")
+			assert.Equal(testMinute, int(time.UTC().Minute()), "Minute mismatch")
+			assert.Equal(testSecond, int(time.UTC().Second()), "Second mismatch")
+
+			switch tt.timeOutputFormat {
+			case TimestampNanos:
+				assert.Equal(testNano, int(time.UTC().UnixNano()), "Nanosecond mismatch")
+				fallthrough
+			case TimestampMicros:
+				assert.Equal(testMicro, int(time.UTC().UnixMicro()), "Microsecond mismatch")
+				fallthrough
+			case Rfc3339, TimestampMillis:
+				assert.Equal(testMilli, int(time.UTC().UnixMilli()), "Millisecond mismatch")
+			}
+		})
+	}
+}