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

eventpb: make the Timestamp field an int64 #58070

Merged
merged 2 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
110 changes: 55 additions & 55 deletions docs/generated/eventlog.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/server/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ func (n *Node) recordJoinEvent(ctx context.Context) {
nodeDetails = &ev.CommonNodeEventDetails
nodeDetails.LastUp = n.startedAt
}
event.CommonDetails().Timestamp = timeutil.Now()
event.CommonDetails().Timestamp = timeutil.Now().UnixNano()
nodeDetails.StartedAt = n.startedAt
nodeDetails.NodeID = int32(n.Descriptor.NodeID)

Expand Down
2 changes: 1 addition & 1 deletion pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ func (s *Server) Decommission(
} else {
panic("unexpected target membership status")
}
event.CommonDetails().Timestamp = timeutil.Now()
event.CommonDetails().Timestamp = timeutil.Now().UnixNano()
nodeDetails.RequestingNodeID = int32(s.NodeID())

for _, nodeID := range nodeIDs {
Expand Down
11 changes: 5 additions & 6 deletions pkg/sql/event_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ package sql
import (
"context"
"encoding/json"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/kv"
Expand All @@ -23,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
)

Expand All @@ -49,7 +49,7 @@ func logEventInternalForSchemaChanges(
mutationID descpb.MutationID,
event eventpb.EventPayload,
) error {
event.CommonDetails().Timestamp = txn.ReadTimestamp().GoTime()
event.CommonDetails().Timestamp = txn.ReadTimestamp().WallTime
scCommon, ok := event.(eventpb.EventWithCommonSchemaChangePayload)
if !ok {
return errors.AssertionFailedf("unknown event type: %T", event)
Expand Down Expand Up @@ -84,7 +84,7 @@ func logEventInternalForSQLStatements(
event eventpb.EventPayload,
) error {
// Inject the common fields into the payload provided by the caller.
event.CommonDetails().Timestamp = txn.ReadTimestamp().GoTime()
event.CommonDetails().Timestamp = txn.ReadTimestamp().WallTime
sqlCommon, ok := event.(eventpb.EventWithCommonSQLPayload)
if !ok {
return errors.AssertionFailedf("unknown event type: %T", event)
Expand Down Expand Up @@ -135,8 +135,7 @@ func InsertEventRecord(
info.CommonDetails().EventType = eventType

// The caller is responsible for the timestamp field.
var zeroTime time.Time
if info.CommonDetails().Timestamp == zeroTime {
if info.CommonDetails().Timestamp == 0 {
return errors.AssertionFailedf("programming error: timestamp field in event not populated: %T", info)
}

Expand All @@ -160,7 +159,7 @@ VALUES(
)
`
args := []interface{}{
info.CommonDetails().Timestamp,
timeutil.Unix(0, info.CommonDetails().Timestamp),
eventType,
targetID,
reportingID,
Expand Down
17 changes: 3 additions & 14 deletions pkg/testutils/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1931,11 +1931,9 @@ func TestLint(t *testing.T) {
filters := []stream.Filter{
// Ignore generated files.
stream.GrepNot(`pkg/.*\.pb\.go:`),
stream.GrepNot(`pkg/col/coldata/.*\.eg\.go:`),
stream.GrepNot(`pkg/col/colserde/arrowserde/.*_generated\.go:`),
stream.GrepNot(`pkg/sql/colexec/.*\.eg\.go:`),
stream.GrepNot(`pkg/sql/colexec/.*_generated\.go:`),
stream.GrepNot(`pkg/sql/pgwire/hba/conf.go:`),
stream.GrepNot(`pkg/.*\.pb\.gw\.go:`),
stream.GrepNot(`pkg/.*\.[eo]g\.go:`),
stream.GrepNot(`pkg/.*_generated\.go:`),

// Ignore types that can change by system.
stream.GrepNot(`pkg/util/sysutil/sysutil_unix.go:`),
Expand All @@ -1945,19 +1943,10 @@ func TestLint(t *testing.T) {
stream.GrepNot(`include/jemalloc/jemalloc\.h`),

stream.GrepNot(`declaration of "?(pE|e)rr"? shadows`),
stream.GrepNot(`\.pb\.gw\.go:[0-9:]+: declaration of "?ctx"? shadows`),
stream.GrepNot(`\.[eo]g\.go:[0-9:]+: declaration of ".*" shadows`),
// This exception is for hash.go, which re-implements runtime.noescape
// for efficient hashing.
stream.GrepNot(`pkg/sql/colexec/hash.go:[0-9:]+: possible misuse of unsafe.Pointer`),
stream.GrepNot(`^#`), // comment line
// This exception is for the colexec generated files.
stream.GrepNot(`pkg/sql/colexec/.*\.eg.go:[0-9:]+: self-assignment of .* to .*`),
// Roachpb generated switch on `error`. It's OK for now because
// the inner error is always unwrapped (it's a protobuf
// enum). Eventually we want to use generalized error
// encode/decode instead and drop the linter exception.
stream.GrepNot(`pkg/roachpb/batch_generated\.go:.*invalid direct cast on error object`),
// Roachpb's own error package takes ownership of error unwraps
// (by enforcing that errors can never been wrapped under a
// roachpb.Error, which is an inconvenient limitation but it is
Expand Down
6 changes: 2 additions & 4 deletions pkg/util/log/event_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ package log
import (
"context"
"encoding/json"
"time"

"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
"github.com/cockroachdb/cockroach/pkg/util/log/severity"
Expand All @@ -24,9 +23,8 @@ import (
func StructuredEvent(ctx context.Context, event eventpb.EventPayload) {
// Populate the missing common fields.
common := event.CommonDetails()
var zeroTime time.Time
if common.Timestamp == zeroTime {
common.Timestamp = timeutil.Now()
if common.Timestamp == 0 {
common.Timestamp = timeutil.Now().UnixNano()
}
if len(common.EventType) == 0 {
common.EventType = eventpb.GetEventTypeName(event)
Expand Down
1 change: 0 additions & 1 deletion pkg/util/log/eventpb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,5 @@ go_library(
deps = [
"//pkg/util/log/logpb",
"//vendor/github.com/gogo/protobuf/proto",
"//vendor/github.com/gogo/protobuf/types",
],
)
95 changes: 38 additions & 57 deletions pkg/util/log/eventpb/events.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions pkg/util/log/eventpb/events.proto
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ import "google/protobuf/timestamp.proto";

// CommonEventDetails contains the fields common to all events.
message CommonEventDetails {
// The timestamp of the event.
google.protobuf.Timestamp timestamp = 1 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true, (gogoproto.jsontag) = ",omitempty"];
// The timestamp of the event. Expressed as nanoseconds since
// the Unix epoch.
int64 timestamp = 1 [(gogoproto.jsontag) = ",omitempty"];
// The type of the event.
string event_type = 2 [(gogoproto.jsontag) = ",omitempty"];
}
Expand Down