-
Notifications
You must be signed in to change notification settings - Fork 0
Fix timestamp type mismatch between proto (uint64) and envelope (int64) #12
Copy link
Copy link
Open
Labels
area/envelopeJSON envelope helpers and MIME-contract behavior.JSON envelope helpers and MIME-contract behavior.area/protoProto definitions, message design, or wire compatibility work.Proto definitions, message design, or wire compatibility work.good first issueSmall, well-scoped tasks for new contributors.Small, well-scoped tasks for new contributors.help wantedLooking for community contributions.Looking for community contributions.kind/bugUnexpected behaviour or regression that needs fixing.Unexpected behaviour or regression that needs fixing.priority/mediumNormal priority item.Normal priority item.
Metadata
Metadata
Assignees
Labels
area/envelopeJSON envelope helpers and MIME-contract behavior.JSON envelope helpers and MIME-contract behavior.area/protoProto definitions, message design, or wire compatibility work.Proto definitions, message design, or wire compatibility work.good first issueSmall, well-scoped tasks for new contributors.Small, well-scoped tasks for new contributors.help wantedLooking for community contributions.Looking for community contributions.kind/bugUnexpected behaviour or regression that needs fixing.Unexpected behaviour or regression that needs fixing.priority/mediumNormal priority item.Normal priority item.
Problem statement
Proto messages use
uint64 timestamp_ms(unsigned) while the Go envelope struct usesint64 TimestampMs(signed). The conversion between them has edge cases:int64 → uint64: negativeTimestampMsvalues produce massive positive timestampsuint64 → int64: values >math.MaxInt64produce negative valuesCurrent conversion code in
envelope.go:While millisecond timestamps won't practically overflow in our lifetimes, the type mismatch signals unclear design intent and makes the code fragile.
Proposed change
Standardize on
uint64in the envelope since timestamps can never be negative:This eliminates both conversion paths and aligns with the proto definition.
Alternative: Use
int64everywhere (proto + envelope) since JSON doesn't have unsigned integers and some languages (Java, JavaScript) lack native uint64. If choosing this path, change the proto field toint64 timestamp_ms— varint encoding is compatible for positive values.Affected area
Compatibility / migration
int64toTimestampMsAdditional context
Identified during architectural review. The
> 0guard inToBinaryFramemasks the negative-value issue but doesn't solve it.