1165 create initial conduit binary #1167

AlgoStephenAkiki · 2022-08-05T18:08:28Z

Resolves #1165

Creates an initial framework for conduit binary production. Introduces
a data directory flag as well as init and shutdown (basic)
functionality.

Instructions:

Type make in project root. Conduit binary should now be located in cmd/conduit
Run from shell: mkdir -p /tmp/conduit_data_dir && touch /tmp/conduit_data_dir/conduit.yml
Insert the following into /tmp/conduit_data_dir/conduit.yml:

LogLevel: info
Importer:
  Name: "algod"
  Config:
    netaddr: "http://127.0.0.1:8080"
    token: "e36c01fc77e490f23e61899c0c22c6390d0fff1443af2c95d056dc5ce4e61302"
Processors:
  - Name: "noop"
    Config:
      catchpoint: "7560000#3OUX3TLXZNOK6YJXGETKRRV2MHMILF5CCIVZUOJCT6SLY5H2WWTQ"
Exporter:
  Name: "noop"
  Config:
    connectionstring: ""

start algod instance
Run cmd/conduit/conduit -d /tmp/conduit_data_dir/

Resolves #1165 Creates an initial framework for conduit binary production. Introduces a data directory flag as well as init and shutdown (basic) functionality.

Eric-Warehime

Nice, I think it's a great start! Left some comments/questions--hope they help.

cmd/conduit/main.go

conduit/errors.go

exporters/noop/noop_exporter.go

conduit/init_provider.go

conduit/pipeline.go

data/block_export_data.go

Eric-Warehime

LGTM for a first pass. Let's continue the discussion around moving Genesis data to the Importer interface. Will also be good to get Will's opinion on this when he has time.

Eric-Warehime · 2022-08-08T20:13:27Z

conduit/logging.go

+	entry.Data["__type"] = f.Type
+	entry.Data["_name"] = f.Name


Why use 1 underscore in one key and 2 in the other?

It's to force the order when outputting the json to the screen to be type then name. Like this:

{"__type":"Conduit","_name":"main","level":"info","msg":"Log level set to: warn","time":"2022-08-09T08:48:44-04:00"}

If one is looking at the logs manually it is a little easier to look at.

Looks like you can provide a sorting function for this kind of thing without messing around with the names: https://pkg.go.dev/github.com/sirupsen/logrus#TextFormatter.SortingFunc

I think that is only for TextFormatter not JSONFormatter though. Do we care if we lose the JSON data format? I figured the structured logging would be useful for log aggregating tools.

Eric-Warehime

I'd like to merge just to keep development moving forward. Left some comments for some things.

Also, we need to add conduit/pipeline_test.go, conduit/config_test.go, and probably processors/noop/noop_processor_test.go (even though I know it does nothing).

cmd/conduit/main.go

conduit/init_provider.go

Eric-Warehime · 2022-08-09T18:15:00Z

importers/algod/algod_importer_test.go

+			rnd, _ := strconv.Atoi(path.Base(r.URL.Path))
+			blk := rpcs.EncodedBlockCert{Block: bookkeeping.Block{BlockHeader: bookkeeping.BlockHeader{Round: basics.Round(rnd)}}}
+			var blockbytes []byte
+			w.WriteHeader(http.StatusOK)
+			response := struct {
+				Block bookkeeping.Block `codec:"block"`
+			}{
+				Block: blk.Block,
+			}
+			enc := codec.NewEncoderBytes(&blockbytes, protocol.CodecHandle)
+			enc.Encode(response)
+			w.Write(blockbytes)


Nit: I think we could simplify this with something like the following, but I haven't tested it.

Suggested change

rnd, _ := strconv.Atoi(path.Base(r.URL.Path))

blk := rpcs.EncodedBlockCert{Block: bookkeeping.Block{BlockHeader: bookkeeping.BlockHeader{Round: basics.Round(rnd)}}}

var blockbytes []byte

w.WriteHeader(http.StatusOK)

response := struct {

Block bookkeeping.Block `codec:"block"`

}{

Block: blk.Block,

}

enc := codec.NewEncoderBytes(&blockbytes, protocol.CodecHandle)

enc.Encode(response)

w.Write(blockbytes)

rnd, _ := strconv.Atoi(path.Base(r.URL.Path))

blk := rpcs.EncodedBlockCert{Block: bookkeeping.Block{BlockHeader: bookkeeping.BlockHeader{Round: basics.Round(rnd)}}}

blockbytes := protocol.EncodeJSON(*blk)

w.WriteHeader(http.StatusOK)

w.Write(blockbytes)

importers/algod/algod_importer.go

cmd/conduit/.gitignore

winder · 2022-08-09T19:45:17Z

cmd/conduit/main.go

+		Args:  cobra.NoArgs,
+		RunE: func(cmd *cobra.Command, args []string) error {
+
+			// Tie cobra variables into local go-lang variables


Are we sure we want to use environment variables? How does that work if there is more than one level in the YAML file?

I think we need to modify it anyway since the function uses the INDEXER prefix which isn't what we want.

For this level, we would only be setting the data directory via environmental variables. All other (nested) configurations would be handled by the supplied config file conduit.yml and wouldn't be affected by environmental variables.

cmd/conduit/main.go

conduit/pipeline.go

exporters/exporter_factory.go

winder · 2022-08-09T20:10:11Z

conduit/pipeline.go

+	importer   *importers.Importer
+	processors []*processors.Processor
+	exporter   *exporters.Exporter


I would consider making all of these slices and generate errors when creating the things below if it isn't supported yet.

Could you explain more on that? So would it become:

importer []*importers.Importer processors []*processors.Processor exporter []*exporters.Exporter

?

Right. This way you can use the same pattern for initializing all of them, only the error handling would change. I don't think this was my most important bit of feedback, just something to consider.

conduit/pipeline.go

codecov · 2022-08-10T01:26:48Z

Codecov Report

❗ No coverage uploaded for pull request base (conduit@3d2720c). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             conduit    #1167   +/-   ##
==========================================
  Coverage           ?   58.84%           
==========================================
  Files              ?       66           
  Lines              ?     8904           
  Branches           ?        0           
==========================================
  Hits               ?     5240           
  Misses             ?     3196           
  Partials           ?      468

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Makefile

importers/algod/algod_importer.go

Makefile

winder · 2022-08-12T15:12:39Z

cmd/conduit/main.go

+}
+
+// runConduitCmdWithConfig run the main logic with a supplied conduit config
+func runConduitCmdWithConfig(cfg *conduit.Config) error {


having viper/pflag stuff in the run function might make it more difficult to use this in daemon later on. I think this can just accept a single dataDir string parameter.

Should we manually check CONDUIT_DATA_DIR since it's the only special case?

winder · 2022-08-12T15:29:59Z

conduit/logging.go

+}
+
+// Format allows this to be used as a logrus formatter
+func (f PluginLogFormatter) Format(entry *log.Entry) ([]byte, error) {


I finally realized that you're implementing the Format interface for logrus. Neat trick 👍

winder · 2022-08-12T15:31:22Z

conduit/pipeline.go

+}
+
+// Valid validates pipeline config
+func (cfg *PipelineConfig) Valid() error {


This can be private

It might also be possible to set defaults and have some of it be automatic:
https://github.com/spf13/viper#establishing-defaults

winder · 2022-08-12T15:39:10Z

conduit/pipeline.go

+			},
+		)
+
+		err := (*processor).Init(p.ctx, *p.initProvider, "")


Is this missing the configuration and logger?

winder · 2022-08-12T15:40:54Z

conduit/pipeline.go

+	var initProvider data.InitProvider = &PipelineInitProvider{
+		currentRound: &currentRound,
+		genesis:      genesis,
+	}
+
+	p.initProvider = &initProvider


Suggested change

var initProvider data.InitProvider = &PipelineInitProvider{

currentRound: &currentRound,

genesis: genesis,

}

p.initProvider = &initProvider

p.initProvider = &PipelineInitProvider{

currentRound: &currentRound,

genesis: genesis,

}

Also, consider renaming to pluginProvider if we're using the same one for all plugin types.

winder · 2022-08-12T15:44:40Z

conduit/pipeline.go

+	)
+
+	// TODO modify/fix ?
+	jsonEncode := string(json.Encode(p.cfg.Importer.Config))


You could put a String() function on the p.cfg.Importer.Config object to streamline this a bit.

winder · 2022-08-12T15:45:16Z

conduit/pipeline.go

+		},
+	)
+
+	err = (*p.exporter).Init("", p.logger)


This one is also missing the config.

winder · 2022-08-12T15:47:13Z

conduit/pipeline.go

+	exporter   *exporters.Exporter
+}
+
+func (p *pipelineImpl) Start() error {


consider renaming to initialize, since I'm sure there is going to be a lot more to do for Start in the next iteration.

winder

I think this is in a good spot, we can followup on any remaining tasks in later PRs.

AlgoStephenAkiki added 2 commits August 5, 2022 10:02

Initial files

9ba8fd0

Initial Conduit Binary

da7bbb8

Resolves #1165 Creates an initial framework for conduit binary production. Introduces a data directory flag as well as init and shutdown (basic) functionality.

AlgoStephenAkiki requested review from winder, Eric-Warehime, shiqizng and algoganesh August 5, 2022 18:08

AlgoStephenAkiki self-assigned this Aug 5, 2022

linting

d022d8c

Eric-Warehime reviewed Aug 8, 2022

View reviewed changes

PR comments

06e8d06

Eric-Warehime reviewed Aug 8, 2022

View reviewed changes

AlgoStephenAkiki added 3 commits August 9, 2022 12:32

init via importer

27e6cd1

Passing test

369ad8d

e2e test passing

6cd16a6

Eric-Warehime approved these changes Aug 9, 2022

View reviewed changes