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

services/horizon: Allow customization of the Captive Core log file. #3472

Merged
merged 7 commits into from
Mar 19, 2021
17 changes: 15 additions & 2 deletions ingest/ledgerbackend/captive_core_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ledgerbackend
import (
"context"
"encoding/hex"
"io/ioutil"
"os"
"sync"

Expand Down Expand Up @@ -111,6 +112,8 @@ type CaptiveCoreConfig struct {
NetworkPassphrase string
// HistoryArchiveURLs are a list of history archive urls
HistoryArchiveURLs []string
// LogPath is the path in which to store Core logs
LogPath string
Shaptic marked this conversation as resolved.
Show resolved Hide resolved

// Optional fields

Expand All @@ -134,11 +137,21 @@ type CaptiveCoreConfig struct {
func NewCaptive(config CaptiveCoreConfig) (*CaptiveStellarCore, error) {
// Here we set defaults in the config. Because config is not a pointer this code should
// not mutate the original CaptiveCoreConfig instance which was passed into NewCaptive()

// Create a default logging instance for Captive Core's output if no other
// logging configuration was specified.
if config.Log == nil {
config.Log = log.New()
config.Log.Logger.SetOutput(os.Stdout)
config.Log.SetLevel(logrus.InfoLevel)
if config.LogPath != "" {
// We still create an empty logger instance so there's no nil
// pointer floating around beyond this call.
config.Log.Logger.SetOutput(ioutil.Discard)
} else {
config.Log.Logger.SetOutput(os.Stdout)
config.Log.SetLevel(logrus.InfoLevel)
}
}

parentCtx := config.Context
if parentCtx == nil {
parentCtx = context.Background()
Expand Down
4 changes: 3 additions & 1 deletion ingest/ledgerbackend/stellar_core_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type pipe struct {
}

type stellarCoreRunner struct {
logPath string
executablePath string
configAppendPath string
networkPassphrase string
Expand Down Expand Up @@ -82,6 +83,7 @@ func newStellarCoreRunner(config CaptiveCoreConfig, mode stellarCoreRunnerMode)
ctx, cancel := context.WithCancel(config.Context)

runner := &stellarCoreRunner{
logPath: config.LogPath,
executablePath: config.BinaryPath,
configAppendPath: config.ConfigAppendPath,
networkPassphrase: config.NetworkPassphrase,
Expand Down Expand Up @@ -116,7 +118,7 @@ func (r *stellarCoreRunner) generateConfig() (string, error) {
fmt.Sprintf(`NETWORK_PASSPHRASE="%s"`, r.networkPassphrase),
fmt.Sprintf(`BUCKET_DIR_PATH="%s"`, filepath.Join(r.tempDir, "buckets")),
fmt.Sprintf(`HTTP_PORT=%d`, r.httpPort),
`LOG_FILE_PATH=""`,
fmt.Sprintf(`LOG_FILE_PATH="%s"`, r.logPath),
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
}

if r.mode == stellarCoreRunnerModeOffline {
Expand Down
1 change: 1 addition & 0 deletions services/horizon/internal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type Config struct {
CaptiveCoreConfigAppendPath string
RemoteCaptiveCoreURL string
CaptiveCoreHTTPPort uint
CaptiveCoreLogPath string

StellarCoreDatabaseURL string
StellarCoreURL string
Expand Down
6 changes: 6 additions & 0 deletions services/horizon/internal/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ func Flags() (*Config, support.ConfigOptions) {
OptType: types.String,
Usage: "name of the file where logs will be saved (leave empty to send logs to stdout)",
},
&support.ConfigOption{
Name: "captive-core-log-path",
ConfigKey: &config.CaptiveCoreLogPath,
OptType: types.String,
Usage: "name of the path for Core logs (leave empty to log w/ Horizon)",
bartekn marked this conversation as resolved.
Show resolved Hide resolved
Shaptic marked this conversation as resolved.
Show resolved Hide resolved
},
&support.ConfigOption{
Name: "max-path-length",
ConfigKey: &config.MaxPathLength,
Expand Down
14 changes: 13 additions & 1 deletion services/horizon/internal/ingest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type Config struct {
CaptiveCoreBinaryPath string
CaptiveCoreConfigAppendPath string
CaptiveCoreHTTPPort uint
CaptiveCoreLogPath string
RemoteCaptiveCoreURL string
NetworkPassphrase string

Expand Down Expand Up @@ -189,16 +190,27 @@ func NewSystem(config Config) (System, error) {
return nil, errors.Wrap(err, "error creating captive core backend")
}
} else {
// If the user wants a custom logging location for Captive Core (set
// via Core's LOG_FILE_PATH), it takes priority over Horizon's
// internal logging.
//
// https://github.com/stellar/go/issues/3438#issuecomment-797690998
var logger *logpkg.Entry = nil
if config.CaptiveCoreLogPath == "" {
logger = log.WithField("subservice", "stellar-core")
}

ledgerBackend, err = ledgerbackend.NewCaptive(
ledgerbackend.CaptiveCoreConfig{
LogPath: config.CaptiveCoreLogPath,
BinaryPath: config.CaptiveCoreBinaryPath,
ConfigAppendPath: config.CaptiveCoreConfigAppendPath,
HTTPPort: config.CaptiveCoreHTTPPort,
NetworkPassphrase: config.NetworkPassphrase,
HistoryArchiveURLs: []string{config.HistoryArchiveURL},
CheckpointFrequency: config.CheckpointFrequency,
LedgerHashStore: ledgerbackend.NewHorizonDBLedgerHashStore(config.HistorySession),
Log: log.WithField("subservice", "stellar-core"),
Log: logger,
Context: ctx,
},
)
Expand Down
1 change: 1 addition & 0 deletions services/horizon/internal/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func initIngester(app *App) {
CaptiveCoreBinaryPath: app.config.CaptiveCoreBinaryPath,
CaptiveCoreConfigAppendPath: app.config.CaptiveCoreConfigAppendPath,
CaptiveCoreHTTPPort: app.config.CaptiveCoreHTTPPort,
CaptiveCoreLogPath: app.config.CaptiveCoreLogPath,
RemoteCaptiveCoreURL: app.config.RemoteCaptiveCoreURL,
EnableCaptiveCore: app.config.EnableCaptiveCoreIngestion,
Comment on lines 72 to 77
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As an informational question, why do we do this, rather than passing along app.config directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's clear which global config variables are used by the component. I know that we sometimes pass entire config in other places in Horizon but I believe we should only pass specific values.

DisableStateVerification: app.config.IngestDisableStateVerification,
Expand Down