Log out important directories for validator client #5653

ShawkiS · 2020-04-27T20:09:27Z

Resolves #5617

Description
Write why you are making the changes in this pull request

I want to print the .db and validator keys location at the beginning of the process.

Write a summary of the changes you are making:
-Logging the DB path in validator client in NewValidatorClient in validator/node/node.go
-Logging the Keystore path in validator client in NewKeystore function in validator/keymanager/direct_keystore.go

terencechain

We do log this today for beacon node

[2020-04-27 13:04:49]  INFO node: Checking DB database-path=/Users/terencetsao/Library/Eth2/beaconchaindata

codecov · 2020-04-27T21:37:37Z

Codecov Report

Merging #5653 into master will decrease coverage by 21.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #5653       +/-   ##
==========================================
- Coverage   26.80%   5.62%   -21.18%     
==========================================
  Files         241     116      -125     
  Lines       21193    9328    -11865     
==========================================
- Hits         5681     525     -5156     
+ Misses      14423    8702     -5721     
+ Partials     1089     101      -988

0xKiwi · 2020-04-27T23:27:03Z

slasher/db/kv/kv.go

@@ -76,6 +77,7 @@ func NewKVStore(dirPath string, cfg *Config) (*Store, error) {
 	if err := os.MkdirAll(dirPath, 0700); err != nil {
 		return nil, err
 	}
+	logrus.Infof(".db & validator keys dir is: %v", dirPath)


This isn't needed, the slasher already logs out the datadir.

0xKiwi · 2020-04-27T23:27:50Z

validator/db/db.go

@@ -68,6 +68,7 @@ func NewKVStore(dirPath string, pubKeys [][48]byte) (*Store, error) {
 	if err := os.MkdirAll(dirPath, 0700); err != nil {
 		return nil, err
 	}
+	log.Infof(".db & validator keys dir is: %v", dirPath)


This is not correct, the validator keys are not in this directory. You will need another log to declare where they are. Look at how the Checking DB log is put out and mirror that for this one.

prestonvanloon · 2020-04-28T01:03:50Z

validator/db/db.go

@@ -68,6 +69,8 @@ func NewKVStore(dirPath string, pubKeys [][48]byte) (*Store, error) {
 	if err := os.MkdirAll(dirPath, 0700); err != nil {
 		return nil, err
 	}
+	log.WithField("database-path", dirPath).Info("Checking DB")
+	log.WithField("validator-keys-path", cmd.DataDirFlag.Name).Info("Checking Validator Keys")


Wouldn't this print validator-keys-path=datadir?

By this i mean, you are printing the flag name, not the value of the flag.

Oh sorry, What is the property I have to use to get the datadir?

Hi @ShawkiS typically we can use something like cliCtx.GlobalString(cmd.DataDirFlag.Name)

rauljordan · 2020-04-28T18:23:26Z

validator/node/node.go

@@ -101,6 +101,7 @@ func NewValidatorClient(ctx *cli.Context) (*ValidatorClient, error) {
 		if err := clearDB(dataDir, pubkeys, forceClearFlag); err != nil {
 			return nil, err
 		}
+		log.WithField("database-path and validator-keys-path", dataDir).Info("Checking DB and Validator Keys")


Suggested change

log.WithField("database-path and validator-keys-path", dataDir).Info("Checking DB and Validator Keys")

log.WithField("path", dataDir).Info("Checking DB and validator keystore path")

Ok, thank you. I changed it.

0xKiwi · 2020-04-28T19:38:50Z

validator/node/node.go

@@ -101,6 +101,7 @@ func NewValidatorClient(ctx *cli.Context) (*ValidatorClient, error) {
 		if err := clearDB(dataDir, pubkeys, forceClearFlag); err != nil {
 			return nil, err
 		}
+		log.WithField("path", dataDir).Info("Checking DB path")


Can you mirror this to the beacon node log? So Checking DB and the field name databasePath.

0xKiwi · 2020-04-28T19:39:22Z

validator/keymanager/direct_keystore.go

@@ -45,6 +45,7 @@ func NewKeystore(input string) (KeyManager, string, error) {
 	if opts.Path == "" {
 		opts.Path = accounts.DefaultValidatorDir()
 	}
+	log.WithField("path", opts.Path).Info("Checking validator keystore path")


Same here, can you change the text to "Checking validator keys" and the field to "keystorePath"?

0xKiwi

Meant to just comment, my bad

terencechain

I don't understand this. Validator client only logs the DB directory path if clear-db is applied? I don't think this is intension given the description. Can you also update the description please?

ShawkiS added 3 commits April 27, 2020 23:00

.

b9472d9

print in beacon-chain, validator and slasher

4f77791

add source and build

d11e01e

ShawkiS requested a review from a team as a code owner April 27, 2020 20:09

ShawkiS requested review from farazdagi, 0xKiwi and nisdas April 27, 2020 20:09

terencechain requested changes Apr 27, 2020

View reviewed changes

fix

4be13e5

ShawkiS requested a review from terencechain April 27, 2020 22:24

0xKiwi suggested changes Apr 27, 2020

View reviewed changes

ShawkiS changed the title ~~Display key paths at startup~~ Log out important directories for validator client Apr 27, 2020

ShawkiS added 2 commits April 28, 2020 03:30

fix

046224f

fix

896383e

prestonvanloon reviewed Apr 28, 2020

View reviewed changes

ShawkiS added 2 commits April 28, 2020 21:01

fix

c8c25f7

run gazelle

008194b

rauljordan reviewed Apr 28, 2020

View reviewed changes

ShawkiS added 3 commits April 28, 2020 21:26

change log

c27235b

change log file

00686bd

change log file

5895b23

ShawkiS requested a review from 0xKiwi April 28, 2020 19:29

0xKiwi approved these changes Apr 28, 2020

View reviewed changes

0xKiwi suggested changes Apr 28, 2020

View reviewed changes

ShawkiS added 2 commits April 28, 2020 22:57

change log message

ce01981

change log message

2a89080

ShawkiS requested review from 0xKiwi and rauljordan April 28, 2020 20:57

prestonvanloon previously approved these changes Apr 28, 2020

View reviewed changes

Merge branch 'master' into Display-key-paths-at-startup

ba9690d

terencechain reviewed Apr 28, 2020

View reviewed changes

change the logging code place

f08a525

ShawkiS dismissed prestonvanloon’s stale review via f08a525 April 29, 2020 09:34

ShawkiS and others added 2 commits April 29, 2020 12:48

change datadir to out of scoop

a8388f4

Merge branch 'master' into Display-key-paths-at-startup

e6c4796

prestonvanloon approved these changes Apr 29, 2020

View reviewed changes

terencechain approved these changes Apr 29, 2020

View reviewed changes

Merge branch 'master' into Display-key-paths-at-startup

afc73c8

0xKiwi approved these changes Apr 30, 2020

View reviewed changes

0xKiwi added the OK to merge label Apr 30, 2020

prylabs-bulldozer bot merged commit e1ac1d3 into prysmaticlabs:master Apr 30, 2020

0xKiwi mentioned this pull request May 13, 2020

Add Subcommand to Retrieve Data Directory #5526

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log out important directories for validator client #5653

Log out important directories for validator client #5653

ShawkiS commented Apr 27, 2020 •

edited

Loading

terencechain left a comment

codecov bot commented Apr 27, 2020 •

edited

Loading

0xKiwi Apr 27, 2020

0xKiwi Apr 27, 2020

prestonvanloon Apr 28, 2020

prestonvanloon Apr 28, 2020

ShawkiS Apr 28, 2020

rauljordan Apr 28, 2020

rauljordan Apr 28, 2020 •

edited

Loading

ShawkiS Apr 28, 2020

0xKiwi Apr 28, 2020

ShawkiS Apr 28, 2020

0xKiwi Apr 28, 2020

ShawkiS Apr 28, 2020

0xKiwi left a comment

terencechain left a comment

	log.WithField("database-path and validator-keys-path", dataDir).Info("Checking DB and Validator Keys")
	log.WithField("path", dataDir).Info("Checking DB and validator keystore path")

Log out important directories for validator client #5653

Log out important directories for validator client #5653

Conversation

ShawkiS commented Apr 27, 2020 • edited Loading

terencechain left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 27, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rauljordan Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xKiwi left a comment

Choose a reason for hiding this comment

terencechain left a comment

Choose a reason for hiding this comment

ShawkiS commented Apr 27, 2020 •

edited

Loading

codecov bot commented Apr 27, 2020 •

edited

Loading

rauljordan Apr 28, 2020 •

edited

Loading