LOGC-42: Replace timestamp by startTime in log ordering and offset tracking#80
LOGC-42: Replace timestamp by startTime in log ordering and offset tracking#80
Conversation
StartTime is going to be used in the place of timestamp for ordering logs and for offsets so it cannot be NULL.
Log objects were sorted by the internal Timestamp field which doesn't appear in the logs. The visible StartTime field (AWS "Time" field) didn't match the sort order. The fix is to sort by StartTime which appears in the logs.
Change the offset storage layer to use StartTime instead of Timestamp.
Update BatchFinder to use StartTime in the composite offset comparison when discovering unprocessed logs.
The timestamp field is no longer used by log-courier. Remove it from the LogRecord struct and ClickHouse schema used in tests.
Update all test files to remove references to deprecated timestamp field.
pkg/logcourier/logobject_test.go
Outdated
| AclRequired: testutil.StrPtr("Yes"), | ||
| Timestamp: now, | ||
| InsertedAt: time.Now(), | ||
| InsertedAt: time.Now(), |
There was a problem hiding this comment.
Indentation looks off, maybe a round of go fmt is advisable
|
@fredmnl Thanks for reminding me of this! I don't think we actually need I'd prefer to do this work in a separate PR (and merge this one). |
Use MergeTree was incorrect.
Codecov Report❌ Patch coverage is
Additional details and impacted files
@@ Coverage Diff @@
## main #80 +/- ##
==========================================
- Coverage 73.35% 73.20% -0.15%
==========================================
Files 21 21
Lines 1625 1620 -5
==========================================
- Hits 1192 1186 -6
- Misses 347 348 +1
Partials 86 86
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
fredmnl
left a comment
There was a problem hiding this comment.
I"ll check out the Confluence doc. I'm ok with merging this as is

Problem
Log objects were being sorted by the internal
Timestampfield (end time), but this field isn't written to the log output. The visible field in the logs isStartTime(AWS "Time" field). This made it impossible for readers to verify chronological ordering by examining the logs - the visible timestamps may not always match the actual sort order.FIx
StartTimeinstead ofTimestamp.StartTimeinstead ofTimestamp.Timestampis no longer used by log-courier and can be removed from theaccess_logstable.