Reduce logs #2695
Reduce logs #2695
Conversation
pkg/pillar/base/logobjecttypes.go
Outdated
| return fmt.Sprintf("%s:%p", key, object) | ||
| return fmt.Sprintf("%s:%p", key, object.logger) |
There was a problem hiding this comment.
The key question here is whether we always create unique loggers for each microservice running in the same process.
@gkodali-zededa do we also need to keep them apart when we have a microservice subscribe to something it publishes? In that case using the logger wouldn't be sufficient.
There was a problem hiding this comment.
As I can see we create separate logger for each service
eve/pkg/pillar/zedbox/zedbox.go
Line 221 in b2471c5
There was a problem hiding this comment.
In the case we have a publisher and subscriber in the same service (which we have at least in nim and probably elsewhere) they will not be treated as separate in this log map.
Also, you don't even consider the type of the object. We have the same UUID (or UUID+version, or sha) as key for objects of completely different types.
There was a problem hiding this comment.
Oops this is using the LogKey and not the Key and the LogKey() functions all use obe of the define LogKey strings which capture the type of the device.
But we still need to distinguish between a publish and subscribe of the identical thing. One way to do that would be to pass another arg from pubsub to LogCreate(), LogModify() and LogDelete() to indicate "publish" vs, "subscribe" and include that in they key used for this map.
There was a problem hiding this comment.
Do you mean that we have publisher and subscriber of the same type in the same agent? Let me check.
There was a problem hiding this comment.
Yes, we have that in a few places.
There was a problem hiding this comment.
I think care is required because in the LogModify we typically fatal if the map entry is not found. So we can't accidentally delete from the map.
Maybe it makes sense to split out that commit so we can merge the other commits?
There was a problem hiding this comment.
Yes, sounds reasonable.
There was a problem hiding this comment.
Seems I found root cause: #2714
I can see lots of "touchSavedConfig failed: chtimes /persist/checkpoint/lastlocalprofile: no such file or directory" in log. Seems we should ensure that we have checkpoint file to decide should we touch or create it. Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
I can see that we show array of bytes in logs which looks suboptimal Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
I can see a lot of "readConfigResponseProtoMessage\", \"level\":\"error\",\"msg\":\"Unmarshalling failed: proto: cannot parse invalid wire-format data". Seems, we should send uuidRequest and expect uuidResponse according to API docs. Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
giggsoff
force-pushed
the
reduce-logs
branch
2 times, most recently
from
c38eed3 to
ce9feb9
Compare
Changes aimed to reduce odd logs:
touchSavedConfig failed: chtimes /persist/checkpoint/lastlocalprofile: no such file or directory)Unmarshalling failed: proto: cannot parse invalid wire-format data)