Skip to content
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

ingest: Render captive core logs as Horizon logs #3189

Merged
merged 18 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions ingest/ledgerbackend/captive_core_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,20 @@ func NewCaptive(executablePath, configPath, networkPassphrase string, historyURL
}, nil
}

func (c *CaptiveStellarCore) SetLogger(logger *log.Entry) {
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
// If the caller decides to set a custom logger, we recreate the factory to
// pass this along to all subsequently-created instances.
previousFactory := c.stellarCoreRunnerFactory
c.stellarCoreRunnerFactory = func(configPath string) (stellarCoreRunnerInterface, error) {
core, err := previousFactory(configPath)
if err != nil {
return core, err
}
core.setLogger(logger)
return core, nil
}
}

func (c *CaptiveStellarCore) getLatestCheckpointSequence() (uint32, error) {
has, err := c.archive.GetRootHAS()
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions ingest/ledgerbackend/captive_core_backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stellar/go/historyarchive"
"github.com/stellar/go/network"
"github.com/stellar/go/support/errors"
"github.com/stellar/go/support/log"
"github.com/stellar/go/xdr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -47,6 +48,8 @@ func (m *stellarCoreRunnerMock) close() error {
return a.Error(0)
}

func (m *stellarCoreRunnerMock) setLogger(*log.Entry) {}

func buildLedgerCloseMeta(sequence uint32) xdr.LedgerCloseMeta {
opResults := []xdr.OperationResult{}
opMeta := []xdr.OperationMeta{}
Expand Down
59 changes: 54 additions & 5 deletions ingest/ledgerbackend/stellar_core_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ import (
"time"

"github.com/pkg/errors"
"github.com/stellar/go/support/log"
)

type stellarCoreRunnerInterface interface {
catchup(from, to uint32) error
runFrom(from uint32, hash string) error
getMetaPipe() io.Reader
getProcessExitChan() <-chan error
setLogger(*log.Entry)
close() error
}

Expand All @@ -42,6 +44,9 @@ type stellarCoreRunner struct {
metaPipe io.Reader
tempDir string
nonce string

// Optionally, logging can be done to something other than stdout.
Log *log.Entry
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
}

func newStellarCoreRunner(executablePath, configPath, networkPassphrase string, historyURLs []string) (*stellarCoreRunner, error) {
Expand Down Expand Up @@ -106,9 +111,10 @@ func (r *stellarCoreRunner) getConfFileName() string {
return filepath.Join(r.tempDir, "stellar-core.conf")
}

func (*stellarCoreRunner) getLogLineWriter() io.Writer {
r, w := io.Pipe()
br := bufio.NewReader(r)
func (r *stellarCoreRunner) getLogLineWriter() io.Writer {
rd, wr := io.Pipe()
br := bufio.NewReader(rd)

// Strip timestamps from log lines from captive stellar-core. We emit our own.
dateRx := regexp.MustCompile(`^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3} `)
go func() {
Expand All @@ -118,10 +124,45 @@ func (*stellarCoreRunner) getLogLineWriter() io.Writer {
break
}
line = dateRx.ReplaceAllString(line, "")
fmt.Print(line)

// If there's a logger, we attempt to extract metadata about the log
// entry, then redirect it to the logger. Otherwise, we just use stdout.
if r.Log == nil {
fmt.Print(line)
continue
}

levelRx := regexp.MustCompile(`\[(\w+) ([A-Z]+)\]`)
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
indices := levelRx.FindStringSubmatchIndex(line)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too complicated. If you use FindSubmatch and rewrite the regexp like in https://play.golang.com/p/mYf-9iMkOCg you won't have to deal with cutting the string yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially did this, but the problem is that I still needed the index to trim the line at the end. To do the above, I'd unfortunately have to match twice; do we want to do this? The code is definitely cleaner but slightly less efficient; it's done in 7187374, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the problem is that I still needed the index to trim the line at the end

You don't have to if you change the regexp to G[A-Z]{4} \[(\w+) ([A-Z]+)\] (.*) (as in play.golang example above). The line remaining is also a group match so it's in [3].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 of course; I didn't notice the last match.

if len(indices) >= 6 {
// Identify the indices that match our regex subexpressions
categoryIdx := indices[2:4]
levelIdx := indices[4:6]

// Extract the substrings from the log entry
category := line[categoryIdx[0]:categoryIdx[1]]
level := line[levelIdx[0]:levelIdx[1]]

line = line[indices[1]+1:] // dump the matched part of the line

levelMapping := map[string]func(string, ...interface{}){
"FATAL": r.Log.Errorf,
"ERROR": r.Log.Errorf,
"WARNING": r.Log.Warnf,
"INFO": r.Log.Infof,
}

if writer, ok := levelMapping[strings.ToUpper(level)]; ok {
writer("%s: %s", category, line)
} else {
r.Log.Infof(line)
}
} else {
r.Log.Infof(line)
}
}
}()
return w
return wr
}

// Makes the temp directory and writes the config file to it; called by the
Expand Down Expand Up @@ -221,6 +262,14 @@ func (r *stellarCoreRunner) getProcessExitChan() <-chan error {
return r.processExit
}

func (r *stellarCoreRunner) setLogger(logger *log.Entry) {
if logger != nil {
r.Log = logger.WithField("subservice", "stellar-core")
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
} else {
r.Log = nil
}
}

func (r *stellarCoreRunner) close() error {
var err1, err2 error

Expand Down
6 changes: 4 additions & 2 deletions services/horizon/internal/ingest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ func NewSystem(config Config) (System, error) {
return nil, errors.Wrap(err, "error creating captive core backend")
}
} else {
//
ledgerBackend, err = ledgerbackend.NewCaptive(
var captiveCoreBackend *ledgerbackend.CaptiveStellarCore
captiveCoreBackend, err = ledgerbackend.NewCaptive(
config.StellarCoreBinaryPath,
config.StellarCoreConfigPath,
config.NetworkPassphrase,
Expand All @@ -182,6 +182,8 @@ func NewSystem(config Config) (System, error) {
cancel()
return nil, errors.Wrap(err, "error creating captive core backend")
}
captiveCoreBackend.SetLogger(log)
ledgerBackend = captiveCoreBackend
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you can leave the old code and write:

ledgerBackend.(*ledgerbackend.CaptiveStellarCore).SetLogger(log)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though less lines, is it cleaner? As someone newish to Go, it's quite a bit harder to read, especially with the new change about name + subservice, it becomes:

ledgerBackend.(*ledgerbackend.CaptiveStellarCore).SetStellarCoreLogger(log.WithField("subservice", "stellar-core"))

😮

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right!

}
} else {
coreSession := config.CoreSession.Clone()
Expand Down