Fine-tune bootstrap logging

[jpkrohling]

Signed-off-by: Juraci Paixão Kröhling [email protected]

Which problem is this PR solving?

• The Jaeger Java Client is quite verbose during the bootstrap:

15:56:05,271 INFO  [io.jaegertracing.internal.senders.SenderResolver] (ServerService Thread Pool -- 7) Using sender UdpSender(udpTransport=ThriftUdpTransport(socket=java.net.DatagramSocket@c90e6f0, receiveBuf=null, receiveOffSet=-1, receiveLength=0))
15:56:05,452 INFO  [io.jaegertracing.Configuration] (ServerService Thread Pool -- 7) Initialized tracer=JaegerTracer(version=Java-0.30.5, serviceName=8dbe9483-4f76-42e9-8fcd-05421892257f.war, reporter=RemoteReporter(queueProcessor=RemoteReporter.QueueProcessor(open=true), sender=UdpSender(udpTransport=ThriftUdpTransport(socket=java.net.DatagramSocket@c90e6f0, receiveBuf=null, receiveOffSet=-1, receiveLength=0)), closeEnqueueTimeout=1000), sampler=RemoteControlledSampler(maxOperations=2000, serviceName=8dbe9483-4f76-42e9-8fcd-05421892257f.war, manager=HttpSamplingManager(hostPort=localhost:5778), metrics=io.jaegertracing.internal.metrics.Metrics@42ae2e51, sampler=ProbabilisticSampler(positiveSamplingBoundary=9223372036854776, negativeSamplingBoundary=-9223372036854776, samplingRate=0.001, tags={sampler.type=probabilistic, sampler.param=0.001})), ipv4=-1062731769, tags={hostname=caju.kroehling.de, jaeger.version=Java-0.30.5, ip=192.168.0.7}, zipkinSharedRpcSpan=false, baggageSetter=io.jaegertracing.internal.baggage.BaggageSetter@2d728fb9, expandExceptionLogs=false, objectFactory=io.jaegertracing.internal.JaegerObjectFactory@20cecb76)

Short description of the changes

• Downgraded regular, non-exceptional messages to debug from info level. Users who wish to see those messages can set log4j to display debug messages for io.jaegertracing classes.
• Changed ToString annotation to not use exclusions at this annotation level, but rather, at the field level, so that future changes are more conscious about the fact that new properties end up in the #toString()

[codecov]

Codecov Report

Merging #531 into master will decrease coverage by 0.05%.
The diff coverage is 84.61%.

@@             Coverage Diff              @@
##             master     #531      +/-   ##
============================================
- Coverage     88.42%   88.36%   -0.06%     
  Complexity      505      505              
============================================
  Files            66       66              
  Lines          1883     1883              
  Branches        239      239              
============================================
- Hits           1665     1664       -1     
- Misses          142      143       +1     
  Partials         76       76

Impacted Files	Coverage Δ	Complexity Δ
...tracing/internal/samplers/RateLimitingSampler.java	66.66% <0%> (ø)	6 <0> (ø)	⬇️
...n/java/io/jaegertracing/internal/JaegerTracer.java	90.38% <100%> (ø)	24 <1> (ø)	⬇️
...jaegertracing/internal/senders/SenderResolver.java	96.77% <100%> (ø)	9 <0> (ø)	⬇️
...egertracing/internal/reporters/RemoteReporter.java	82.14% <100%> (-1.2%)	7 <1> (ø)
.../src/main/java/io/jaegertracing/Configuration.java	94.6% <100%> (ø)	42 <0> (ø)	⬇️
...ing/internal/samplers/RemoteControlledSampler.java	89.53% <100%> (ø)	17 <1> (ø)	⬇️
...tracing/internal/samplers/HttpSamplingManager.java	100% <100%> (ø)	6 <3> (ø)	⬇️
...racing/internal/samplers/ProbabilisticSampler.java	81.81% <100%> (ø)	12 <1> (ø)	⬇️
...ain/java/io/jaegertracing/zipkin/ZipkinSender.java	65.67% <50%> (ø)	17 <1> (ø)	⬇️

---

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 8b35787...6548879. Read the comment docs.

[vprithvi]

I'm of the opinion that bootstrap can be verbose because it allows for easy checks on what the actual configuration of the tracer is on production, etc.
Are these lines printed more than once per session?

[jpkrohling]

They are printed only once, but when tracing is a "small" part of a bigger ecosystem, it might not be desirable to have them. I'm quite sure I left the first message at INFO because of local tests (... Using sender UdpSender...)

The second is too verbose to be useful "all the time", which is why I think it should be suppressed. They are indeed useful when debugging why something is wrong, though.

[jpkrohling]

I just added another commit with some changes to the @ToString used by all classes. The first change was to use @ToString.Exclude close to the field. With that, I hope that future changes won't automatically add fields to the ToString "implicitly". We had a bunch of those, like JaegerTracer#objectFactory, resulting in a string like objectFactory=io.jaegertracing.internal.JaegerObjectFactory@20cecb76, which brings no added value.

Here's how it looks like before this PR:

JaegerTracer(
    version=Java-0.30.5, 
    serviceName=8dbe9483-4f76-42e9-8fcd-05421892257f.war, 
    reporter=RemoteReporter(
        queueProcessor=RemoteReporter.QueueProcessor(open=true), 
        sender=UdpSender(
            udpTransport=ThriftUdpTransport(
                socket=java.net.DatagramSocket@c90e6f0, 
                receiveBuf=null, 
                receiveOffSet=-1, 
                receiveLength=0)
            ), 
        closeEnqueueTimeout=1000
    ), 
    sampler=RemoteControlledSampler(
        maxOperations=2000, 
        serviceName=8dbe9483-4f76-42e9-8fcd-05421892257f.war, 
        manager=HttpSamplingManager(hostPort=localhost:5778), 
        metrics=io.jaegertracing.internal.metrics.Metrics@42ae2e51, 
        sampler=ProbabilisticSampler(
            positiveSamplingBoundary=9223372036854776, 
            negativeSamplingBoundary=-9223372036854776, 
            samplingRate=0.001, 
            tags={sampler.type=probabilistic, sampler.param=0.001}
        )
    ), 
    ipv4=-1062731769, 
    tags={hostname=caju.kroehling.de, jaeger.version=Java-0.30.5, ip=192.168.0.7}, 
    zipkinSharedRpcSpan=false, 
    baggageSetter=io.jaegertracing.internal.baggage.BaggageSetter@2d728fb9, 
    expandExceptionLogs=false, 
    objectFactory=io.jaegertracing.internal.JaegerObjectFactory@20cecb76
)

And here's how it looks after:

JaegerTracer(
    version=Java-0.30.6-SNAPSHOT, 
    serviceName=50349613-e60a-487d-b3d6-f0fd2241cc17.war, 
    reporter=RemoteReporter(sender=UdpSender(), closeEnqueueTimeout=1000), 
    sampler=RemoteControlledSampler(
        maxOperations=2000, 
        manager=HttpSamplingManager(hostPort=localhost:5778), 
        sampler=ProbabilisticSampler(tags={sampler.type=probabilistic, sampler.param=0.001})
    ), 
    tags={hostname=caju.kroehling.de, jaeger.version=Java-0.30.6-SNAPSHOT, ip=192.168.0.7}, 
    zipkinSharedRpcSpan=false, 
    expandExceptionLogs=false
)

[pavolloffay]

I would like more adding @ToString.Exclude directly on top of the excluded field without creating a new block for excluded fields. And keep the field at the original place

private final boolean zipkinSharedRpcSpan
@ToString.Exclude
private final ScopeManager scopeManager;

[jpkrohling]

That's a matter of taste, I guess. I'll keep it like that, unless other reviewers prefer to have the annotation in a different line.

The reason I prefer it this way is that it makes it easy to spot what is and what is not in the toString. More importantly, it makes future decisions more conscious about whether or not the field should be included in the toString, instead of just "copy and paste".

[objectiser]

Not directly related to this PR, but to improve clarity of error reporting, just wondering if we should add reporting of these specific scenarios:

• multiple factories but JAEGER_SENDER_FACTORY not specified to select
• multiple factories found, but JAEGER_SENDER_FACTORY value not found in the list
• no sender factories found

[jpkrohling]

Created #532

[jpkrohling]

Seriously...

[jpkrohling]

@vprithvi, could you please re-review this one? If this change is acceptable, I'll merge and do a new release, so that I can consume it on WildFly.

[vprithvi]

@jpkrohling LGTM! I like the approach of applying the exclude annotations on the specific fields. Lot less fragile.