Consolidate helper functions for working with proto3 time messages (v…

…itessio#13905) Signed-off-by: Andrew Mason <[email protected]>
slackhq · Sep 1, 2023 · 7636754 · 7636754
1 parent 2de3ea9
commit 7636754
Show file tree

Hide file tree

Showing 32 changed files with 94 additions and 214 deletions.
diff --git a/go/cmd/vtctldclient/cli/awk.go b/go/cmd/vtctldclient/cli/awk.go
@@ -22,7 +22,7 @@ import (
 	"strings"
 	"time"
 
-	"vitess.io/vitess/go/vt/logutil"
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/topoproto"
 
@@ -66,7 +66,7 @@ func MarshalTabletAWK(t *topodatapb.Tablet) string {
 	// special case for old primary that hasn't been updated in the topo
 	// yet.
 	if t.PrimaryTermStartTime != nil && t.PrimaryTermStartTime.Seconds > 0 {
-		mtst = logutil.ProtoToTime(t.PrimaryTermStartTime).Format(time.RFC3339)
+		mtst = protoutil.TimeFromProto(t.PrimaryTermStartTime).UTC().Format(time.RFC3339)
 	}
 
 	return fmt.Sprintf("%v %v %v %v %v %v %v %v", topoproto.TabletAliasString(t.Alias), keyspace, shard, topoproto.TabletTypeLString(t.Type), ti.Addr(), ti.MysqlAddr(), MarshalMapAWK(t.Tags), mtst)

diff --git a/go/cmd/vtctldclient/command/backups.go b/go/cmd/vtctldclient/command/backups.go
@@ -26,7 +26,6 @@ import (
 
 	"vitess.io/vitess/go/cmd/vtctldclient/cli"
 	"vitess.io/vitess/go/protoutil"
-	"vitess.io/vitess/go/vt/logutil"
 	"vitess.io/vitess/go/vt/mysqlctl"
 	"vitess.io/vitess/go/vt/topo/topoproto"
 
@@ -246,7 +245,7 @@ func commandRestoreFromBackup(cmd *cobra.Command, args []string) error {
 	req := &vtctldatapb.RestoreFromBackupRequest{
 		TabletAlias:        alias,
 		RestoreToPos:       restoreFromBackupOptions.RestoreToPos,
-		RestoreToTimestamp: logutil.TimeToProto(restoreToTimestamp),
+		RestoreToTimestamp: protoutil.TimeToProto(restoreToTimestamp),
 		DryRun:             restoreFromBackupOptions.DryRun,
 	}
 

diff --git a/go/cmd/vtctldclient/command/keyspaces.go b/go/cmd/vtctldclient/command/keyspaces.go
@@ -25,11 +25,11 @@ import (
 	"github.com/spf13/cobra"
 
 	"vitess.io/vitess/go/mysql/sqlerror"
+	"vitess.io/vitess/go/protoutil"
 
 	"vitess.io/vitess/go/cmd/vtctldclient/cli"
 	"vitess.io/vitess/go/constants/sidecar"
 	"vitess.io/vitess/go/mysql"
-	"vitess.io/vitess/go/vt/logutil"
 	"vitess.io/vitess/go/vt/topo"
 
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
@@ -178,7 +178,7 @@ func commandCreateKeyspace(cmd *cobra.Command, args []string) error {
 			return fmt.Errorf("--snapshot-time cannot be in the future; snapshot = %v, now = %v", t, now)
 		}
 
-		snapshotTime = logutil.TimeToProto(t)
+		snapshotTime = protoutil.TimeToProto(t)
 	}
 
 	createKeyspaceOptions.SidecarDBName = strings.TrimSpace(createKeyspaceOptions.SidecarDBName)

diff --git a/go/cmd/vtctldclient/command/throttler.go b/go/cmd/vtctldclient/command/throttler.go
@@ -23,8 +23,8 @@ import (
 	"github.com/spf13/cobra"
 
 	"vitess.io/vitess/go/cmd/vtctldclient/cli"
+	"vitess.io/vitess/go/protoutil"
 
-	"vitess.io/vitess/go/vt/logutil"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle"
@@ -60,10 +60,10 @@ func commandUpdateThrottlerConfig(cmd *cobra.Command, args []string) error {
 	updateThrottlerConfigOptions.Keyspace = keyspace
 
 	if throttledAppRule.Name != "" {
-		throttledAppRule.ExpiresAt = logutil.TimeToProto(time.Now().Add(throttledAppDuration))
+		throttledAppRule.ExpiresAt = protoutil.TimeToProto(time.Now().Add(throttledAppDuration))
 		updateThrottlerConfigOptions.ThrottledApp = &throttledAppRule
 	} else if unthrottledAppRule.Name != "" {
-		unthrottledAppRule.ExpiresAt = logutil.TimeToProto(time.Now())
+		unthrottledAppRule.ExpiresAt = protoutil.TimeToProto(time.Now())
 		updateThrottlerConfigOptions.ThrottledApp = &unthrottledAppRule
 	}
 

diff --git a/go/test/endtoend/tabletmanager/throttler_topo/throttler_test.go b/go/test/endtoend/tabletmanager/throttler_topo/throttler_test.go
@@ -27,10 +27,10 @@ import (
 	"time"
 
 	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/test/endtoend/cluster"
 	"vitess.io/vitess/go/test/endtoend/throttler"
-	"vitess.io/vitess/go/vt/logutil"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/base"
 
@@ -397,7 +397,7 @@ func TestLag(t *testing.T) {
 	t.Run("exempting test app", func(t *testing.T) {
 		appRule := &topodatapb.ThrottledAppRule{
 			Name:      testAppName,
-			ExpiresAt: logutil.TimeToProto(time.Now().Add(time.Hour)),
+			ExpiresAt: protoutil.TimeToProto(time.Now().Add(time.Hour)),
 			Exempt:    true,
 		}
 		_, err := throttler.UpdateThrottlerTopoConfig(clusterInstance, false, false, throttler.DefaultThreshold.Seconds(), useDefaultQuery, appRule)
@@ -407,7 +407,7 @@ func TestLag(t *testing.T) {
 	t.Run("unexempting test app", func(t *testing.T) {
 		appRule := &topodatapb.ThrottledAppRule{
 			Name:      testAppName,
-			ExpiresAt: logutil.TimeToProto(time.Now()),
+			ExpiresAt: protoutil.TimeToProto(time.Now()),
 		}
 		_, err := throttler.UpdateThrottlerTopoConfig(clusterInstance, false, false, throttler.DefaultThreshold.Seconds(), useDefaultQuery, appRule)
 		assert.NoError(t, err)

diff --git a/go/test/endtoend/throttler/util.go b/go/test/endtoend/throttler/util.go
@@ -30,10 +30,10 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/tidwall/gjson"
 
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/test/endtoend/cluster"
 	"vitess.io/vitess/go/vt/concurrency"
 	"vitess.io/vitess/go/vt/log"
-	"vitess.io/vitess/go/vt/logutil"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/base"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/throttlerapp"
@@ -79,7 +79,7 @@ func UpdateThrottlerTopoConfigRaw(vtctldProcess *cluster.VtctldClientProcess, ke
 	}
 	if appRule != nil {
 		args = append(args, "--throttle-app", appRule.Name)
-		args = append(args, "--throttle-app-duration", logutil.ProtoToTime(appRule.ExpiresAt).Sub(time.Now()).String())
+		args = append(args, "--throttle-app-duration", protoutil.TimeFromProto(appRule.ExpiresAt).UTC().Sub(time.Now()).String())
 		args = append(args, "--throttle-app-ratio", fmt.Sprintf("%f", appRule.Ratio))
 		if appRule.Exempt {
 			args = append(args, "--throttle-app-exempt")

diff --git a/go/vt/logutil/logger.go b/go/vt/logutil/logger.go
@@ -25,6 +25,7 @@ import (
 	"sync"
 	"time"
 
+	"vitess.io/vitess/go/protoutil"
 	logutilpb "vitess.io/vitess/go/vt/proto/logutil"
 )
 
@@ -73,7 +74,7 @@ func EventToBuffer(event *logutilpb.Event, buf *bytes.Buffer) {
 		return
 	}
 
-	t := ProtoToTime(event.Time)
+	t := protoutil.TimeFromProto(event.Time).UTC()
 	_, month, day := t.Date()
 	hour, minute, second := t.Clock()
 	twoDigits(buf, int(month))
@@ -137,7 +138,7 @@ func NewCallbackLogger(f func(*logutilpb.Event)) *CallbackLogger {
 func (cl *CallbackLogger) InfoDepth(depth int, s string) {
 	file, line := fileAndLine(2 + depth)
 	cl.f(&logutilpb.Event{
-		Time:  TimeToProto(time.Now()),
+		Time:  protoutil.TimeToProto(time.Now()),
 		Level: logutilpb.Level_INFO,
 		File:  file,
 		Line:  line,
@@ -149,7 +150,7 @@ func (cl *CallbackLogger) InfoDepth(depth int, s string) {
 func (cl *CallbackLogger) WarningDepth(depth int, s string) {
 	file, line := fileAndLine(2 + depth)
 	cl.f(&logutilpb.Event{
-		Time:  TimeToProto(time.Now()),
+		Time:  protoutil.TimeToProto(time.Now()),
 		Level: logutilpb.Level_WARNING,
 		File:  file,
 		Line:  line,
@@ -161,7 +162,7 @@ func (cl *CallbackLogger) WarningDepth(depth int, s string) {
 func (cl *CallbackLogger) ErrorDepth(depth int, s string) {
 	file, line := fileAndLine(2 + depth)
 	cl.f(&logutilpb.Event{
-		Time:  TimeToProto(time.Now()),
+		Time:  protoutil.TimeToProto(time.Now()),
 		Level: logutilpb.Level_ERROR,
 		File:  file,
 		Line:  line,
@@ -198,7 +199,7 @@ func (cl *CallbackLogger) Error(err error) {
 func (cl *CallbackLogger) Printf(format string, v ...any) {
 	file, line := fileAndLine(2)
 	cl.f(&logutilpb.Event{
-		Time:  TimeToProto(time.Now()),
+		Time:  protoutil.TimeToProto(time.Now()),
 		Level: logutilpb.Level_CONSOLE,
 		File:  file,
 		Line:  line,

diff --git a/go/vt/logutil/logger_test.go b/go/vt/logutil/logger_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 	"time"
 
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/race"
 	logutilpb "vitess.io/vitess/go/vt/proto/logutil"
 )
@@ -31,7 +32,7 @@ func TestLogEvent(t *testing.T) {
 	}{
 		{
 			event: &logutilpb.Event{
-				Time:  TimeToProto(time.Date(2014, time.November, 10, 23, 30, 12, 123456000, time.UTC)),
+				Time:  protoutil.TimeToProto(time.Date(2014, time.November, 10, 23, 30, 12, 123456000, time.UTC)),
 				Level: logutilpb.Level_INFO,
 				File:  "file.go",
 				Line:  123,
@@ -41,7 +42,7 @@ func TestLogEvent(t *testing.T) {
 		},
 		{
 			event: &logutilpb.Event{
-				Time:  TimeToProto(time.Date(2014, time.January, 20, 23, 30, 12, 0, time.UTC)),
+				Time:  protoutil.TimeToProto(time.Date(2014, time.January, 20, 23, 30, 12, 0, time.UTC)),
 				Level: logutilpb.Level_WARNING,
 				File:  "file2.go",
 				Line:  567,
@@ -51,7 +52,7 @@ func TestLogEvent(t *testing.T) {
 		},
 		{
 			event: &logutilpb.Event{
-				Time:  TimeToProto(time.Date(2014, time.January, 20, 23, 30, 12, 0, time.UTC)),
+				Time:  protoutil.TimeToProto(time.Date(2014, time.January, 20, 23, 30, 12, 0, time.UTC)),
 				Level: logutilpb.Level_ERROR,
 				File:  "file2.go",
 				Line:  567,
@@ -61,7 +62,7 @@ func TestLogEvent(t *testing.T) {
 		},
 		{
 			event: &logutilpb.Event{
-				Time:  TimeToProto(time.Date(2014, time.January, 20, 23, 30, 12, 0, time.UTC)),
+				Time:  protoutil.TimeToProto(time.Date(2014, time.January, 20, 23, 30, 12, 0, time.UTC)),
 				Level: logutilpb.Level_CONSOLE,
 				File:  "file2.go",
 				Line:  567,

diff --git a/go/vt/logutil/proto3.go b/go/vt/logutil/proto3.go
@@ -17,37 +17,11 @@ limitations under the License.
 package logutil
 
 import (
-	"time"
-
 	logutilpb "vitess.io/vitess/go/vt/proto/logutil"
-	vttimepb "vitess.io/vitess/go/vt/proto/vttime"
 )
 
 // This file contains a few functions to help with proto3.
 
-// ProtoToTime converts a vttimepb.Time to a time.Time.
-// proto3 will eventually support timestamps, at which point we'll retire
-// this.
-//
-// A nil pointer is like the empty timestamp.
-func ProtoToTime(ts *vttimepb.Time) time.Time {
-	if ts == nil {
-		// treat nil like the empty Timestamp
-		return time.Time{}
-	}
-	return time.Unix(ts.Seconds, int64(ts.Nanoseconds)).UTC()
-}
-
-// TimeToProto converts the time.Time to a vttimepb.Time.
-func TimeToProto(t time.Time) *vttimepb.Time {
-	seconds := t.Unix()
-	nanos := int64(t.Sub(time.Unix(seconds, 0)))
-	return &vttimepb.Time{
-		Seconds:     seconds,
-		Nanoseconds: int32(nanos),
-	}
-}
-
 // EventStream is an interface used by RPC clients when the streaming
 // RPC returns a stream of log events.
 type EventStream interface {

diff --git a/go/vt/logutil/proto3_test.go b/go/vt/logutil/proto3_test.go
diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go
@@ -39,6 +39,7 @@ import (
 	"vitess.io/vitess/go/ioutil"
 	"vitess.io/vitess/go/mysql"
 	"vitess.io/vitess/go/mysql/replication"
+	"vitess.io/vitess/go/protoutil"
 	"vitess.io/vitess/go/vt/concurrency"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/logutil"
@@ -348,9 +349,9 @@ func (be *BuiltinBackupEngine) executeIncrementalBackup(ctx context.Context, par
 		return false, vterrors.Errorf(vtrpc.Code_ABORTED, "empty binlog name in response. Request=%v, Response=%v", req, resp)
 	}
 	incrDetails := &IncrementalBackupDetails{
-		FirstTimestamp:       FormatRFC3339(logutil.ProtoToTime(resp.FirstTimestamp)),
+		FirstTimestamp:       FormatRFC3339(protoutil.TimeFromProto(resp.FirstTimestamp).UTC()),
 		FirstTimestampBinlog: filepath.Base(resp.FirstTimestampBinlog),
-		LastTimestamp:        FormatRFC3339(logutil.ProtoToTime(resp.LastTimestamp)),
+		LastTimestamp:        FormatRFC3339(protoutil.TimeFromProto(resp.LastTimestamp).UTC()),
 		LastTimestampBinlog:  filepath.Base(resp.LastTimestampBinlog),
 	}
 	// It's worthwhile we explain the difference between params.IncrementalFromPos and incrementalBackupFromPosition.
@@ -917,7 +918,7 @@ func (be *BuiltinBackupEngine) executeRestoreIncrementalBackup(ctx context.Conte
 		}
 		req := &mysqlctlpb.ApplyBinlogFileRequest{
 			BinlogFileName:        binlogFile,
-			BinlogRestoreDatetime: logutil.TimeToProto(params.RestoreToTimestamp),
+			BinlogRestoreDatetime: protoutil.TimeToProto(params.RestoreToTimestamp),
 		}
 		if params.RestoreToPos.GTIDSet != nil {
 			req.BinlogRestorePosition = params.RestoreToPos.GTIDSet.String()