Logs: switch to an agnostic primary/secondary backend paradigm #1908
Conversation
Page-
force-pushed
the
logs-agnostic-secondary-primary
branch
from
449ccce to
c96184c
Compare
Page-
force-pushed
the
logs-agnostic-secondary-primary
branch
2 times, most recently
from
dca59d9 to
2cb3db3
Compare
| const timestamp = new loki.Timestamp(); | ||
| timestamp.setSeconds(Math.floor(Number(log.nanoTimestamp / 1000000000n))); | ||
| timestamp.setNanos(Number(log.nanoTimestamp % 1000000000n)); | ||
| // store log line as JSON | ||
| const logJson = JSON.stringify(log, omitNanoTimestamp); | ||
| const logJson = JSON.stringify( | ||
| { ...log, version: VERSION }, |
There was a problem hiding this comment.
This change is to not mutate the log, it isn't a problem when loki is the secondary but when it's the primary it can cause problems for the secondary as they're getting a loki versioned log which they don't expect
Page-
force-pushed
the
logs-agnostic-secondary-primary
branch
from
2cb3db3 to
5a985c6
Compare
Change-type: minor
Page-
force-pushed
the
logs-agnostic-secondary-primary
branch
from
5a985c6 to
3f04276
Compare
| ? secondaryBackend?.publish(ctx, buffer).catch((err) => { | ||
| captureException( | ||
| err, | ||
| `Failed to publish logs to ${LOGS_PRIMARY_BACKEND === 'loki' ? 'redis' : 'loki'}`, |
There was a problem hiding this comment.
NIT
| `Failed to publish logs to ${LOGS_PRIMARY_BACKEND === 'loki' ? 'redis' : 'loki'}`, | |
| `Failed to publish logs to ${LOGS_SECONDARY_BACKEND}`, |
| export const getSecondaryBackend = _.once( | ||
| async (): Promise<DeviceLogsBackend> => { | ||
| if (LOGS_SECONDARY_BACKEND_ENABLED === false) { | ||
| throw new Error('Secondary backend is not enabled'); |
There was a problem hiding this comment.
NIT How about just returning undefined here when LOGS_SECONDARY_BACKEND_ENABLED is false, so that we don't need to do LOGS_SECONDARY_BACKEND_ENABLED ? getSecondaryBackend() : undefined? This way we encapsulate everything in here and we don't have to check LOGS_SECONDARY_BACKEND_ENABLED both right before calling this fn & inside it.
There was a problem hiding this comment.
Doing the throw helps avoid accidental mistakes and also makes it simpler in the case of reading, in the case of writing we need both backends permanently available though (and cannot fetch them on demand because we can't have async boundaries) hence that one needing to be special cased and that's why I went with a throw rather than an implicit == null -> disabled
Change-type: minor