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.

Sign up for GitHub

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 gRPC communication between agent and collector #1165

Merged
merged 14 commits into from
Nov 13, 2018
Merged

Add gRPC communication between agent and collector #1165

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e238edf
Add gRPC communication between agent and collector
pavolloffay Nov 8, 2018
a5b633d
uncomment make and fix comment
pavolloffay Nov 8, 2018
3eeab86
Fix grpc handler initialization
pavolloffay Nov 8, 2018
582e4d7
Copy process if missing
pavolloffay Nov 8, 2018
6afafb0
log only warn and error from grpc
pavolloffay Nov 8, 2018
ba542e5
Fix review comments
pavolloffay Nov 9, 2018
b530e87
Add todo
pavolloffay Nov 9, 2018
0066cdd
Add Roundrobin grpc load balancer
pavolloffay Nov 12, 2018
0d198be
Fix comment
pavolloffay Nov 12, 2018
86e4765
Fix review comments
pavolloffay Nov 12, 2018
dff8e52
Change grpc target name
pavolloffay Nov 12, 2018
9a89d4d
Defaul return error
pavolloffay Nov 12, 2018
b78e314
Fix back tchannel collector addr
pavolloffay Nov 12, 2018
955ab84
Remove collector from grpc reporter flag
pavolloffay Nov 13, 2018
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
Prev Previous commit
Next Next commit
Add todo 
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
  • Loading branch information
@pavolloffay
pavolloffay committed Nov 13, 2018
commit b530e878fcccdfc172b3743a765ec7a4f1f66c6a
1 change: 1 addition & 0 deletions cmd/agent/app/reporter/grpc/reporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func NewReporter(conn *grpc.ClientConn, logger *zap.Logger) *Reporter {

// EmitBatch implements EmitBatch() of Reporter
func (r *Reporter) EmitBatch(b *thrift.Batch) error {
// TODO pass process to r.send() - do not convert it for every span
spans := jConverter.ToDomain(b.Spans, b.Process)
Copy link
Member

Choose a reason for hiding this comment

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

it seems redundant to pass Process (that presumably will be added to each span) and then dedupe that same process in send(). I suggest passing Process directly to send(), and do the index-0 trick in the Zipkin path only.

Copy link
Member Author

Choose a reason for hiding this comment

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

Had a quick look there is no to domain without process. I would prefer a separate PR this is already quite long.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add todo in the code and action in the PR comment

return r.send(spans)
}
Expand Down