ethresearch / sharding-p2p-poc

Proof of Concept of Ethereum Serenity Peer-to-Peer Layer on libp2p PubSub System
40 stars 19 forks source link

Try go-log and refactor error propagation #56

Closed NIC619 closed 5 years ago

NIC619 commented 5 years ago

What was wrong?

go-log is ipfs's library for logging which also supports APIs similar to opentracing's. The APIs are mainly wrappers over the original opentracing APIs(though not all of them) with some extra functionalities, see doc.

Differences between go-log and opentracing:

  1. go-log generates spans from context object(e.g., logger.Start(ctx, "span name")) while in opentracing it's optional(opentracing supports opentracing.StartSpan("span name", childOf(parentSpan), opentracing.StartSpanFromConext(ctx, "span name", childOf(parentSpan)))
    • This makes it easier for go-log to pass along parent span.
    • Though opentracing also supports injecting/extracting span into/from context object(e.g. opentracing.ContextWithSpan and opentracing.SpanFromContext).
  2. go-log does not support span.setBaggageItem which helps propagating shared information(the information shared by span and it's ancestor) to all descendant spans.
    • Though these information could be passed along via context object in go-log.
  3. go-log does not support Inject and Extract which helps propagating shared information across processes(i.e., interprocess tracing).
    • Though these information could also be passed between sender and receiver as long as they are on the same page which also means that the tracing framework used can not be abstracted away on either side.

How was it fixed?

Cute Animal Picture

mhchia commented 5 years ago

Just a note that we probably can use go-log to classify logs, for easier parsing. Refer to https://github.com/ethresearch/sharding-p2p-poc/issues/15

NIC619 commented 5 years ago

@mhchia Rebased and applied PR feedbacks(Remove ctx argument in goroutine in connectShardNodes, Remove return value of Unsubscribe* functions and Remove number in Node struct). Also added an example script that makes each rpc call in sequence.