rocicorp / replicache

Realtime Sync for Any Backend Stack
https://doc.replicache.dev
1.04k stars 37 forks source link

Change LogContext to attach an object #991

Open arv opened 2 years ago

arv commented 2 years ago

We have an internal class called LogContext. It's implementation is at https://github.com/rocicorp/logger/blob/main/src/logger.ts#L145

Basic usage is:

const lc = new LogContext().addContext('a', 'b');
lc?.info('c', 'd');
// roughly same as 
console.info('a=b', 'c', 'd');

This might not be too bad if it wasn't that our context always starts with the clientID which is ephemeral. For tools like Sentry that means that it is impossible to group the log lines.

An alternative approach would be to attach the context as a "JSON object". Then we would get something like:

const lc = new LogContext().addContext('a', 'b');
lc?.info('c', 'd');
// roughly same as 
console.info('c', 'd', {a: 'b'});

Then sentry would display this something along the lines of:

image

phritz commented 2 years ago

Yes this seems sensible, when it comes time to implement I'd like to talk about what goes into the message vs the attached extra context. There are a couple of ways we could go and it has bearing on how we log server-side, because we use LogContext ubiquitously and have have a few requirements on the server that are less relevant on the client.

aboodman commented 2 years ago

I was looking at this and a few questions (trying to setup Sentry myself to experiment):

  1. Can it really be true that it's impossible in Sentry to group by something other than the leading parameter?? It's quite pleasant/easy to read for me with clientID at the head of the line when displayed textually.
  2. Is it just the order of the log line that matters? If the context was at the end, textually, would that help?
KeKs0r commented 2 years ago

@aboodman Sentry can be configured how to group issues. But it is a more sophisticated effort and I think having sane grouping by default would be beneficial.

  1. I think just the log message itself is a very strong parameter on whether something is the same error or not.

The way I solved this now, is by wrapping error level logs into errors and parsing the log info into an object in order to attach it to the error: https://gist.github.com/KeKs0r/ece9a454561bb864f806549d3b8e715c