-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CBL-6155: Improve LiteCore Log #2164
Conversation
velicuvlad
commented
Oct 15, 2024
- repl config is now logged only once, at the time replicator is started
- error response from blob update specified is a blob req error
- certificates errors are all on the same line now - see CBL-6155: Replace extra line with space in between couchbasedeps/mbedtls#3 (once is it merged, I will update this PR)
- add URL endpoint to Repl log
Code Coverage Results:
|
Ignoring Blackduck failure. It complains because we have changed mbedtls without bumping the version |
@@ -104,7 +104,6 @@ namespace litecore::repl { | |||
DatabaseImpl* impl = asInternal(db); | |||
return impl->dataFile()->loggingName(); | |||
}); | |||
logInfo("DB=%s Instantiated %s", dbLogName.c_str(), string(*options).c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this info not important or is it logged the same info somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same exact info is logged on Starting Replicator ...
, which is the next logged line after is Instantiated in every log I've seen pretty much
logInfo("Starting Replicator %s with config: {%s}\n", _replicator->loggingName().c_str(), | ||
std::string(*_options).c_str()); | ||
logInfo("Starting Replicator %s with config: {%s} with endpoing: %.*s", _replicator->loggingName().c_str(), | ||
std::string(*_options).c_str(), SPLAT(_replicator->remoteURL())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if the replicator is for db-to-db or p2p sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whenever Replicator is instantiated it sets _remoteURL = webSocket->url()
. So I believe this would be something like ws://loopback
. I will run test and verify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DB to DB endpoint looks like file:////var/folders/dq/4702h1qn7wdclczftpkmv1dr0000gp/T/LiteCore_Tests_1729177323.cblite2/cbl_core_test2.cblite2.cblite2/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah this is ReplicatorImpl so for P2P the implementation is by platform - so this exact log isn't in the P2P replicators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what we should do about this - @pasin any ideas? Should LiteCore be logging the Replicator options from P2P replicator like it does for remote replicator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand this. The c4ReplicatorImpl's start() should be used by any types of the replicator. I think we can focus on what is being logged here with the remoteURL() with the replicator with message endpoint is used. Maybe the easiest way to find out is to look at the log when running the message endpoint replicator tests from the platform (e.g. iOS) that uses LiteCore in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
endpoint: x-msg-endpt:////_blipsync
endpoint: wss://localhost:55369/otherdb/_blipsync
endpoint: file:////var/folders/q7/sjmz4q8n073f1lynlw5fw70w0000gp/T/CouchbaseLite_EE/otherdb.cblite2/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've also seen this for the passive replicator
endpoint: x-msg-conn:///0x104397e20
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good enough.
logInfo("Starting Replicator %s with config: {%s}\n", _replicator->loggingName().c_str(), | ||
std::string(*_options).c_str()); | ||
logInfo("Starting Replicator %s with config: {%s} with endpoing: %.*s", _replicator->loggingName().c_str(), | ||
std::string(*_options).c_str(), SPLAT(_replicator->remoteURL())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what we should do about this - @pasin any ideas? Should LiteCore be logging the Replicator options from P2P replicator like it does for remote replicator?