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

Add tracing(Opentracing + Appdash) #33

Merged
merged 30 commits into from
Aug 14, 2018
Merged

Conversation

NIC619
Copy link
Collaborator

@NIC619 NIC619 commented Jul 31, 2018

How was it fixed?

Add some basic tracing setup(Opentracing + Appdash) to keep track of the RPC calls between different nodes in the network:

  • There's one Appdash server which collects span data(i.e., logs) from different nodes and visualize them via a web UI server.
  • There are relationships between spans, for example, an AddPeer span or SubscribeShard span would be the ChildOf an RPC Server span.
  • Whenever a node is started, a new trace is started which includes an RPC Server span and multiple RPC call spans(e.g., AddPeer).
  • Each trace and spans has it's own unique ID and one can attach extra information to the span by SetTag.

  • Add an RPC server Stop call to turn down the RPC server so the main span(span with name RPC Server) can terminate normally.

WIP: Look into libp2p codebase to see how they set up and utilize tracing
WIP: survey for different/better visualization tools

Cute Animal Picture

put a cute animal picture link inside the parentheses

@mhchia
Copy link
Collaborator

mhchia commented Jul 31, 2018

Reference to #27

@NIC619 NIC619 force-pushed the master branch 2 times, most recently from e315d8a to b05997f Compare August 7, 2018 13:44
@mhchia
Copy link
Collaborator

mhchia commented Aug 8, 2018

TODOs

  • Avoid type conflict due to gx-ed repo between opentracing and gx/ipfs/.../opentracing
    • the temporary solution is, after doing gx-go rw, we "unwrite" the "gx/ipfs/.../opentracing" imports to "github.com/opentracing/opentracing"
  • fix "proto: invalid utf-8 string"
    • It occurs in addpeer and broadcastcollation, the solution is to ensure we don't store any non utf-8 string in the type string in protobuf(or, just change the type string to bytes in those use cases)
  • fix "duplicate proto type registered: basictracer_go.wire.TracerState"

After every calls to `gx-go rw`
Add commands in Makefile: gx-partial-rw, gx-rw, gx-uw
@mhchia
Copy link
Collaborator

mhchia commented Aug 12, 2018

I merged this branch with master, and modified partial-gx-uw.py to print the unwritten imports. Let's see what errors occur in the CI now

@mhchia
Copy link
Collaborator

mhchia commented Aug 13, 2018

Ah...I just removed fstring. Hope we can get rid of the error. Thanks a lot @ChihChengLiang

@NIC619
Copy link
Collaborator Author

NIC619 commented Aug 14, 2018

Seems like we forgot to fetch the dependency packages recursively while the command go test -v ./… will run the tests recursively(hence it would run into the cannot find package of XXX error.

As for the previous undefined: proto.GoGoProtoPackageIsVersion1 error, the reason seems to be that the wrong version of package gogo/protobuf/proto is fetched by gx and it's currently fixed by gx unwrite the specified packages.

@mhchia
Copy link
Collaborator

mhchia commented Aug 14, 2018

So we need to pass ./... to go get and go test, to build all packages under the directory, and pass . to go get in docker, since we only need the exact main package in the directory to make it build?
Btw., CI passed!

Copy link
Collaborator

@mhchia mhchia left a comment

Choose a reason for hiding this comment

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

LGTM

@mhchia mhchia merged commit 017a6ec into ethresearch:master Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants