opentracing / opentracing-java

OpenTracing API for Java. 🛑 This library is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io
Apache License 2.0
1.68k stars 344 forks source link

Don't override the return type in buildSpan(). #372

Closed mmimica closed 4 years ago

mmimica commented 4 years ago

This makes it possible to override buildSpan(String op) of MockTracer and return another impl of Tracer.SpanBuilder. A use-case for this would be, for example, to supply an instance of SpanBuilder that delegates to original MockTracer.SpanBuilder, but also performs additional logging.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 76.586% when pulling 6bc1d8a46997b1227ca11f58dd92f4d50637ab29 on mmimica:master into a8557d7c7a56210cbcf6bc518fe24b32923aa66f on opentracing:master.

whiskeysierra commented 4 years ago

The proper way of doing this would be composition, not inheritance. You can take a look at https://github.com/zalando/opentracing-toolbox/tree/master/opentracing-proxy to see an example.

mmimica commented 4 years ago

The proper way of doing this would be composition, not inheritance.

Indeed.

mmimica commented 4 years ago

if you extend MockTracer.SpanBuilder, then you do not need this change.

My extension to SpanBuilder is generic and has nothing to do with mocks, so I'm not going do that.

yurishkuro commented 4 years ago

if you're using MockTracer, you should be extending its subcomponents.

You're proposing a breaking change.