-
Notifications
You must be signed in to change notification settings - Fork 344
Is final class
necessary?
#330
Comments
The usual reasons: removing final extends the API surface, making future maintenance and refactoring potentially breaking changes. Maybe you can explain what kind of higher level logic you want to add. Since it's a mock class, maybe those changes can be upstreamed. |
Ok, I see. The reason I'd like to inherit from MockTracer is this: The SpecialAgent project has a JUnit runner named AgentRunner that injects an instance of MockTracer to each test method (example). The MockTracer is the OpenTracing standard for JUnit tests (I believe), so it is reasonable to couple this standard into the JUnit tests that run with AgentRunner (as opposed to creating a replica of MockTracer in the SpecialAgent project itself). We have a requirement to be able to test the AgentRunner JUnit tests against a real tracer, as opposed to just the MockTracer. Since the AgentRunner JUnit tests are coupled to the MockTracer (as they should be, I believe), it would be necessary to rewrite each AgentRunner JUnit test to accept the Tracer interface instead of the MockTracer implementation specifically. Doing so would allow us to inject real tracers instead of the MockTracer for each test. But, the tests rely on assertions made against the MockTracer's To avoid reimplementing the MockTracer, I would like to be able to extend the MockTracer itself. Doing so would allow me to create a "ProxyMockTracer", where I would be able to "filter/pipe/tee" API calls through the MockTracer's API, and to any other Tracer implementation. This would allow a seamless integration of real Tracer providers to AgentRunner JUnit tests. |
Read your explanation twice, but still not sure what the issue is. If the tracer API required by your unit tests is more than OT, could you not simply use MockTracer via composition? It sounds like you'd need to do that anyway with a real tracer in order to provide an equivalent of finishedSpans() accessor (since it's not just the method itself, but the data model it returns as well that becomes your dependency). |
To accomplish my goal, I need to create a “Proxy*” version of every class in the OT API, on top of the MockTracer’s implementation of the API interfaces. Only this way, will a ProxyMockTracer be able to meet the full scope of functionality of the MockTracer as well as the Tracer being “proxied”. |
My question is: don't you need to do that anyway? How else can you make a real tracer satisfy the mock tracer API? Incidentally, this could be helpful https://github.com/opentracing-contrib/java-api-extensions/ |
I'm not quite following what you're referring to with "don't you need to do that anyway". But thank you for the reference to java-api-extensions -- I did not know this existed. Though it's useful to be aware of, it still does not fully meet our requirements. Apologies if my explanation of my use-case is not great. I'll try to itemize the individual points:
As far as I can see, this is not possible with the current class model of the MockTracer, because of Please correct my thinking here if I'm overseeing something. I attempted to implement this ProxyMockTracer subclass of MockTracer, and got stuck at the |
^ this is the part I was referring to. It sounds like you want MockTracer subclass to act as a wrapper around another real tracer (just like api-extensions thing does). Can you show example of what that looks like? E.g. what happens when the agent/instrumentation calls tracer.startSpan() ? |
@yurishkuro, I have added a comment for you in the |
@yurishkuro, I had left a comment for you in the |
Dear OpenTracing-Mock developers,
I am attempting to wrap the MockTracer with higher-level logic, while preserving the full scope of functionality offered by the MockTracer itself.
I am not able to achieve this, however, due to
final class
spec on:MockSpan
,MockSpan.MockContext
, andMockTracer.SpanBuilder
.Before I request to drop the
final
, I want to make sure I'm not missing something.Is there an underlying reason for the
final class
spec?The text was updated successfully, but these errors were encountered: