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

refactor: Replace logging package with corelog #2406

Merged
merged 9 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 2 additions & 2 deletions cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ package cli
import (
"github.com/spf13/cobra"

"github.com/sourcenetwork/defradb/logging"
"github.com/sourcenetwork/corelog"
)

var log = logging.MustNewLogger("cli")
var log = corelog.NewLogger("cli")

// NewDefraCommand returns the root command instanciated with its tree of subcommands.
func NewDefraCommand() *cobra.Command {
Expand Down
67 changes: 17 additions & 50 deletions cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ import (
"path/filepath"
"strings"

"github.com/sourcenetwork/corelog"
"github.com/spf13/pflag"
"github.com/spf13/viper"

"github.com/sourcenetwork/defradb/logging"
)

const (
Expand All @@ -41,11 +40,12 @@ var configPaths = []string{

// configFlags is a mapping of config keys to cli flags to bind to.
var configFlags = map[string]string{
"log.level": "loglevel",
"log.output": "logoutput",
"log.format": "logformat",
"log.stacktrace": "logtrace",
"log.nocolor": "lognocolor",
"log.level": "log-level",
"log.output": "log-output",
"log.format": "log-format",
"log.stacktrace": "log-stacktrace",
"log.source": "log-source",
"log.overrides": "log-overrides",
"api.address": "url",
"datastore.maxtxnretries": "max-txn-retries",
"datastore.store": "store",
Expand Down Expand Up @@ -125,14 +125,17 @@ func loadConfig(rootdir string, flags *pflag.FlagSet) (*viper.Viper, error) {
}
}

logCfg := loggingConfig(cfg.Sub("log"))
logCfg.OverridesByLoggerName = make(map[string]logging.Config)
// set default logging config
corelog.SetConfig(corelog.Config{
Level: cfg.GetString("log.level"),
Format: cfg.GetString("log.format"),
Output: cfg.GetString("log.output"),
EnableStackTrace: cfg.GetBool("log.stacktrace"),
EnableSource: cfg.GetBool("log.source"),
})

// apply named logging overrides
for key := range cfg.GetStringMap("log.overrides") {
logCfg.OverridesByLoggerName[key] = loggingConfig(cfg.Sub("log.overrides." + key))
}
logging.SetConfig(logCfg)
// set logging config overrides
corelog.SetConfigOverrides(cfg.GetString("log.overrides"))

return cfg, nil
}
Expand All @@ -147,39 +150,3 @@ func bindConfigFlags(cfg *viper.Viper, flags *pflag.FlagSet) error {
}
return nil
}

// loggingConfig returns a new logging config from the given config.
func loggingConfig(cfg *viper.Viper) logging.Config {
var level int8
switch value := cfg.GetString("level"); value {
case configLogLevelDebug:
level = logging.Debug
case configLogLevelInfo:
level = logging.Info
case configLogLevelError:
level = logging.Error
case configLogLevelFatal:
level = logging.Fatal
default:
level = logging.Info
}

var format logging.EncoderFormat
switch value := cfg.GetString("format"); value {
case configLogFormatJSON:
format = logging.JSON
case configLogFormatCSV:
format = logging.CSV
default:
format = logging.CSV
}

return logging.Config{
Level: logging.NewLogLevelOption(level),
EnableStackTrace: logging.NewEnableStackTraceOption(cfg.GetBool("stacktrace")),
DisableColor: logging.NewDisableColorOption(cfg.GetBool("nocolor")),
EncoderFormat: logging.NewEncoderFormatOption(format),
OutputPaths: []string{cfg.GetString("output")},
EnableCaller: logging.NewEnableCallerOption(cfg.GetBool("caller")),
}
}
7 changes: 0 additions & 7 deletions cli/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,4 @@ func TestLoadConfigNotExist(t *testing.T) {
assert.Equal(t, true, cfg.GetBool("net.pubsubenabled"))
assert.Equal(t, false, cfg.GetBool("net.relay"))
assert.Equal(t, []string{}, cfg.GetStringSlice("net.peers"))

assert.Equal(t, "info", cfg.GetString("log.level"))
assert.Equal(t, false, cfg.GetBool("log.stacktrace"))
assert.Equal(t, "csv", cfg.GetString("log.format"))
assert.Equal(t, "stderr", cfg.GetString("log.output"))
assert.Equal(t, false, cfg.GetBool("log.nocolor"))
assert.Equal(t, false, cfg.GetBool("log.caller"))
}
24 changes: 15 additions & 9 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,33 +38,39 @@ Start a DefraDB node, interact with a local or remote node, and much more.
)

cmd.PersistentFlags().String(
"loglevel",
"log-level",
"info",
"Log level to use. Options are debug, info, error, fatal",
)

cmd.PersistentFlags().String(
"logoutput",
"log-output",
"stderr",
"Log output path",
"Log output path. Options are stderr or stdout.",
)

cmd.PersistentFlags().String(
"logformat",
"csv",
"Log format to use. Options are csv, json",
"log-format",
"text",
"Log format to use. Options are text or json",
)

cmd.PersistentFlags().Bool(
"logtrace",
"log-stacktrace",
false,
"Include stacktrace in error and fatal logs",
)

cmd.PersistentFlags().Bool(
"lognocolor",
"log-source",
false,
"Disable colored log output",
"Include source location in logs",
)

cmd.PersistentFlags().String(
"log-overrides",
"",
"Logger config overrides. Format <name>,<key>=<val>,...;<name>,...",
)

cmd.PersistentFlags().String(
Expand Down
2 changes: 1 addition & 1 deletion cli/server_dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
Short: "Dumps the state of the entire database",
RunE: func(cmd *cobra.Command, _ []string) error {
cfg := mustGetContextConfig(cmd)
log.FeedbackInfo(cmd.Context(), "Dumping DB state...")
log.InfoContext(cmd.Context(), "Dumping DB state...")

Check warning on line 27 in cli/server_dump.go

View check run for this annotation

Codecov / codecov/patch

cli/server_dump.go#L27

Added line #L27 was not covered by tests

if cfg.GetString("datastore.store") != configStoreBadger {
return errors.New("server-side dump is only supported for the Badger datastore")
Expand Down
8 changes: 4 additions & 4 deletions cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@

defer func() {
if err := n.Close(cmd.Context()); err != nil {
log.FeedbackErrorE(cmd.Context(), "Stopping DefraDB", err)
log.ErrorContextE(cmd.Context(), "Stopping DefraDB", err)

Check warning on line 111 in cli/start.go

View check run for this annotation

Codecov / codecov/patch

cli/start.go#L111

Added line #L111 was not covered by tests
}
}()

log.FeedbackInfo(cmd.Context(), "Starting DefraDB")
log.InfoContext(cmd.Context(), "Starting DefraDB")

Check warning on line 115 in cli/start.go

View check run for this annotation

Codecov / codecov/patch

cli/start.go#L115

Added line #L115 was not covered by tests
if err := n.Start(cmd.Context()); err != nil {
return err
}
Expand All @@ -122,9 +122,9 @@

select {
case <-cmd.Context().Done():
log.FeedbackInfo(cmd.Context(), "Received context cancellation; shutting down...")
log.InfoContext(cmd.Context(), "Received context cancellation; shutting down...")

Check warning on line 125 in cli/start.go

View check run for this annotation

Codecov / codecov/patch

cli/start.go#L125

Added line #L125 was not covered by tests
case <-signalCh:
log.FeedbackInfo(cmd.Context(), "Received interrupt; shutting down...")
log.InfoContext(cmd.Context(), "Received interrupt; shutting down...")

Check warning on line 127 in cli/start.go

View check run for this annotation

Codecov / codecov/patch

cli/start.go#L127

Added line #L127 was not covered by tests
}

return nil
Expand Down
6 changes: 3 additions & 3 deletions datastore/blockstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
// Get returns a block from the blockstore.
func (bs *bstore) Get(ctx context.Context, k cid.Cid) (blocks.Block, error) {
if !k.Defined() {
log.Error(ctx, "Undefined CID in blockstore")
log.ErrorContext(ctx, "Undefined CID in blockstore")
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: We should remove the error logging here since an error is returned to the caller.

return nil, ipld.ErrNotFound{Cid: k}
}
bdata, err := bs.store.Get(ctx, dshelp.MultihashToDsKey(k.Hash()))
Expand Down Expand Up @@ -164,13 +164,13 @@
return
}
if e.Error != nil {
log.ErrorE(ctx, "Blockstore.AllKeysChan errored", e.Error)
log.ErrorContextE(ctx, "Blockstore.AllKeysChan errored", e.Error)

Check warning on line 167 in datastore/blockstore.go

View check run for this annotation

Codecov / codecov/patch

datastore/blockstore.go#L167

Added line #L167 was not covered by tests
return
}

hash, err := dshelp.DsKeyToMultihash(ds.RawKey(e.Key))
if err != nil {
log.ErrorE(ctx, "Error parsing key from binary", err)
log.ErrorContextE(ctx, "Error parsing key from binary", err)

Check warning on line 173 in datastore/blockstore.go

View check run for this annotation

Codecov / codecov/patch

datastore/blockstore.go#L173

Added line #L173 was not covered by tests
continue
}
k := cid.NewCidV1(cid.Raw, hash)
Expand Down
5 changes: 3 additions & 2 deletions datastore/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import (
blockstore "github.com/ipfs/boxo/blockstore"
ds "github.com/ipfs/go-datastore"

"github.com/sourcenetwork/corelog"

"github.com/sourcenetwork/defradb/datastore/iterable"
"github.com/sourcenetwork/defradb/logging"
)

var (
log = logging.MustNewLogger("store")
log = corelog.NewLogger("store")
)

// RootStore wraps Batching and TxnDatastore requiring datastore to support both batching and transactions.
Expand Down
2 changes: 1 addition & 1 deletion db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@
go func() {
defer func() {
if err := q.Close(); err != nil {
log.ErrorE(ctx, errFailedtoCloseQueryReqAllIDs, err)
log.ErrorContextE(ctx, errFailedtoCloseQueryReqAllIDs, err)

Check warning on line 874 in db/collection.go

View check run for this annotation

Codecov / codecov/patch

db/collection.go#L874

Added line #L874 was not covered by tests
}
close(resCh)
c.discardImplicitTxn(ctx, txn)
Expand Down
2 changes: 1 addition & 1 deletion db/collection_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@
// If the plan isn't properly closed at any exit point log the error.
defer func() {
if err := selectionPlan.Close(); err != nil {
log.ErrorE(ctx, "Failed to close the request plan, after filter delete", err)
log.ErrorContextE(ctx, "Failed to close the request plan, after filter delete", err)

Check warning on line 182 in db/collection_delete.go

View check run for this annotation

Codecov / codecov/patch

db/collection_delete.go#L182

Added line #L182 was not covered by tests
}
}()

Expand Down
2 changes: 1 addition & 1 deletion db/collection_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@
// If the plan isn't properly closed at any exit point log the error.
defer func() {
if err := selectionPlan.Close(); err != nil {
log.ErrorE(ctx, "Failed to close the selection plan, after filter update", err)
log.ErrorContextE(ctx, "Failed to close the selection plan, after filter update", err)

Check warning on line 243 in db/collection_update.go

View check run for this annotation

Codecov / codecov/patch

db/collection_update.go#L243

Added line #L243 was not covered by tests
}
}()

Expand Down
20 changes: 10 additions & 10 deletions db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
blockstore "github.com/ipfs/boxo/blockstore"
ds "github.com/ipfs/go-datastore"
dsq "github.com/ipfs/go-datastore/query"
"github.com/sourcenetwork/corelog"
"github.com/sourcenetwork/immutable"

"github.com/sourcenetwork/defradb/client"
Expand All @@ -30,12 +31,11 @@
"github.com/sourcenetwork/defradb/errors"
"github.com/sourcenetwork/defradb/events"
"github.com/sourcenetwork/defradb/lens"
"github.com/sourcenetwork/defradb/logging"
"github.com/sourcenetwork/defradb/request/graphql"
)

var (
log = logging.MustNewLogger("db")
log = corelog.NewLogger("db")
)

// make sure we match our client interface
Expand Down Expand Up @@ -109,7 +109,7 @@
}

func newDB(ctx context.Context, rootstore datastore.RootStore, options ...Option) (*implicitTxnDB, error) {
log.Debug(ctx, "Loading: internal datastores")
log.DebugContext(ctx, "Loading: internal datastores")
multistore := datastore.MultiStoreFrom(rootstore)

parser, err := graphql.NewParser()
Expand Down Expand Up @@ -197,15 +197,15 @@
}
defer txn.Discard(ctx)

log.Debug(ctx, "Checking if DB has already been initialized...")
log.DebugContext(ctx, "Checking if DB has already been initialized...")
exists, err := txn.Systemstore().Has(ctx, ds.NewKey("init"))
if err != nil && !errors.Is(err, ds.ErrNotFound) {
return err
}
// if we're loading an existing database, just load the schema
// and migrations and finish initialization
if exists {
log.Debug(ctx, "DB has already been initialized, continuing")
log.DebugContext(ctx, "DB has already been initialized, continuing")
err = db.loadSchema(ctx, txn)
if err != nil {
return err
Expand All @@ -222,7 +222,7 @@
return txn.Commit(ctx)
}

log.Debug(ctx, "Opened a new DB, needs full initialization")
log.DebugContext(ctx, "Opened a new DB, needs full initialization")

// init meta data
// collection sequence
Expand Down Expand Up @@ -261,16 +261,16 @@
// Close is called when we are shutting down the database.
// This is the place for any last minute cleanup or releasing of resources (i.e.: Badger instance).
func (db *db) Close() {
log.Info(context.Background(), "Closing DefraDB process...")
log.Info("Closing DefraDB process...")
if db.events.Updates.HasValue() {
db.events.Updates.Value().Close()
}

err := db.rootstore.Close()
if err != nil {
log.ErrorE(context.Background(), "Failure closing running process", err)
log.ErrorE("Failure closing running process", err)
}
log.Info(context.Background(), "Successfully closed running process")
log.Info("Successfully closed running process")
}

func printStore(ctx context.Context, store datastore.DSReaderWriter) error {
Expand All @@ -286,7 +286,7 @@
}

for r := range results.Next() {
log.Info(ctx, "", logging.NewKV(r.Key, r.Value))
log.InfoContext(ctx, "", r.Key, r.Value)

Check warning on line 289 in db/db.go

View check run for this annotation

Codecov / codecov/patch

db/db.go#L289

Added line #L289 was not covered by tests
}

return results.Close()
Expand Down
2 changes: 1 addition & 1 deletion db/subscriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
for evt := range pub.Event() {
txn, err := db.NewTxn(ctx, false)
if err != nil {
log.Error(ctx, err.Error())
log.ErrorContext(ctx, err.Error())

Check warning on line 58 in db/subscriptions.go

View check run for this annotation

Codecov / codecov/patch

db/subscriptions.go#L58

Added line #L58 was not covered by tests
continue
}

Expand Down
26 changes: 25 additions & 1 deletion docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,28 @@ https://docs.libp2p.io/concepts/addressing/

Enable libp2p's Circuit relay transport protocol. Defaults to `false`.

https://docs.libp2p.io/concepts/circuit-relay/
https://docs.libp2p.io/concepts/circuit-relay/

## `log.level`

Log level to use. Options are `debug`, `info`, `error`, `fatal`. Defaults to `info`.

## `log.output`

Log output path. Options are `stderr` or `stdout`. Defaults to `stderr`.

## `log.format`

Log format to use. Options are `text` or `json`. Defaults to `text`.

## `log.stacktrace`

Include stacktrace in error and fatal logs. Defaults to `false`.

## `log.source`

Include source location in logs. Defaults to `false`.

## `log.overrides`

Logger config overrides. Format `<name>,<key>=<val>,...;<name>,...`.
Loading
Loading