chore: improve logging #130
Conversation
internal/nostr/nostr.go
Outdated
| svc.Logger.WithFields(logrus.Fields{ | ||
| "subscription_id": subscription.ID, | ||
| svc.Logger.WithError(ctx.Err()).WithFields(logrus.Fields{ | ||
| "subscription_id": subscription.Uuid, | ||
| "relay_url": subscription.RelayUrl, | ||
| }).Debug("Context canceled, stopping subscription") |
There was a problem hiding this comment.
Do we really want to change this to error level?
There was a problem hiding this comment.
I think so - I don't think we want to receive context cancelled errors, do we? I guess this means a hub is offline and we cannot do anything, but it's something that is good to know. Maybe getalby.com eventually should stop trying to request to offline hubs CC @bumi ?
There was a problem hiding this comment.
And before we did not know for certain it's a context cancelled error. Before we just assume it is. I think therefore the message should also be changed. It should not say Context canceled,
There was a problem hiding this comment.
True, I was thinking from one-off susbcriptions pov where this might be hit because of timeout, thereby causing bloat, will address this too in the one-off separation PR 👍
There was a problem hiding this comment.
Leaving this as Error level for now (the separated one-off subscription function will have debug log)
There was a problem hiding this comment.
where this might be hit because of timeout
I still think that we should know this is happening and not just hide it? I still think we need to solve the root problem.
chore: log relay errors only once per app
No description provided.