Follow-up improvements from #2060

[jpkrohling]

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

Which problem is this PR solving?

• Addresses open comments from Refactor collector mains #2060

[yurishkuro]

lgtm, but I really think the repetition of grpclog is unnecessary. We already initialize the Zap logged in Service, we should do the same for grpclog (and perhaps in the future change it to go through Zap instead of going directly to stdout).

[yurishkuro]

why not do it somewhere in svc?

[jpkrohling]

In fact, looks like it's there already!

jaeger/cmd/flags/service.go

Lines 96 to 100 in e631338

	grpcZap.ReplaceGrpcLogger(logger.WithOptions(
	// grpclog is not consistent with the depth of call tree before it's dispatched to zap,
	// but Skip(2) still shows grpclog as caller, while Skip(3) shows actual grpc packages.
	zap.AddCallerSkip(3),
	))

Just need to remove it from the mains then.

[codecov]

Codecov Report

Merging #2088 into master will decrease coverage by 0.03%.
The diff coverage is 63.15%.

@@            Coverage Diff             @@
##           master    #2088      +/-   ##
==========================================
- Coverage   96.33%   96.29%   -0.04%     
==========================================
  Files         214      214              
  Lines       10537    10535       -2     
==========================================
- Hits        10151    10145       -6     
- Misses        328      331       +3     
- Partials       58       59       +1

Impacted Files	Coverage Δ
cmd/collector/app/server/zipkin.go	0% <0%> (ø)	⬆️
cmd/flags/service.go	0% <0%> (ø)	⬆️
cmd/collector/app/server/http.go	0% <0%> (ø)	⬆️
cmd/collector/app/server/grpc.go	66.66% <100%> (-1.2%)	⬇️
cmd/collector/app/collector.go	72.15% <100%> (-1.02%)	⬇️
cmd/query/app/server.go	91.78% <0%> (-2.74%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 913ab1c...7c9afc5. Read the comment docs.

[jpkrohling]

The Cassandra integration test is failing, but isn't apparently related to this PR. Master was showing the build as passing, but it failed with the same reason once I started the job again:

https://travis-ci.org/jaegertracing/jaeger/jobs/655134449

[yurishkuro]

Cassandra issues should be fixed in master