exp/ingest/ledgerbackend: Stream stellar-core logs to file #2673
Conversation
Do you mean if logs should be streamed to a file? I'm not sure. We create a file in a tempdir that gets removed when ledger backend is closed so it shouldn't be an issue. |
|
| }() | ||
| return w | ||
| func (r *stellarCoreRunner) getLogLineWriter() (io.Writer, error) { | ||
| f, err := os.OpenFile(r.getLogFilePath(), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) |
There was a problem hiding this comment.
Having real time logs of core interleaved with the horizon logs can be valuable to reason about what's happening, I wouldn't totally remove the option of printing the logs instead of using a file.
There was a problem hiding this comment.
In fact, wouldn't it be simpler to simply print the Core logs (with the same format as the Horizon ones) but control their verbosity (including not printing any logs at all)?
There was a problem hiding this comment.
Using a temporary file which gets removed makes debugging cumbersome:
0. You need to make sure the file isn't removed (e.g. if horizon crashes or if the container terminates they will be gone).
- You need to fish for the file
- You need to reason about the timestamps of the file with respect to the timestamp in the horizon logs
There was a problem hiding this comment.
In fact, wouldn't it be simpler to simply print the Core logs (with the same format as the Horizon ones) but control their verbosity (including not printing any logs at all)?
I think it makes sense. My only worry is that the log format may change in the future breaking the parsing code but I think we should be able to catch it quickly. I'll update the PR.
There was a problem hiding this comment.
We could add a test for it, so that we catch it when it happens. Right?
There was a problem hiding this comment.
Yes but this requires integration tests (you actually need to run stellar-core to catch this) and we don't have them yet.
|
bartekn
force-pushed
the
captive-core-stream-logs-to-file
branch
from
644e175 to
e5e10f8
Compare
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Stream stellar-core logs to file.
Why
Currently we stream logs to stdout what can be hard to read when also Horizon logs are streamed.