Fix unwrap methods to return cast delegates rather than cast 'this'

[tjquinno]

Description

Resolves #8200

This PR basically does these things:

1. Remove the default interface implementation of unwrap from Tracer, Span, and Span.Builder. The code would cast this but the classes were package-private and inaccessible to developers.
2. Implement proper logic to cast and return the delegate rather than this in the various implementations.
3. Slightly revise the asParent implementations which used unwrap and now just simply cast.
4. Add tests.

Documentation

Bug fix; no doc impact

[tomas-langer]

There is a reason it is calling unwrap - the implementation that we receive may behave differently from what you expect, and provide the correct type, or maybe throw a more relevant exception than just ClassCastException

[tjquinno]

I guess I do not understand what you mean.

The unwrap method has always been declared as public so, presumably, intended to be of use to app developers.

Prior to this PR the default implementation of unwrap defined at Span.Builder(Class<T> type) returned type.cast(this) which is OK for Helidon code in the same package because this is always a package-private Helidon type such as OpenTelemetrySpanBuilder or OpenTracingSpanBuilder. But returning this from there is not useful for app developers because they cannot access those package-private Helidon classes.

So this PR, in line with other unwrap implementations (such as in Tracer), changes unwrap so it now returns the delegate instead of this (cast of course to the requested type). The delegate type is accessible to app developers--because the delegate is a public OpenTracing or OpenTelemetry type accessible to Helidon code-- and therefore of use to an app developer.

Because that revision changes the behavior of unwrap, these prior internal uses of unwrap which you pointed to would need to change. But the net effect of the changed code you linked to is exactly the same as before: the old use of the old unwrap method would cast the span builder's this to the specified type, and that's precisely what the revised code in the PR does at those places except the revised asParent code just does the casting itself.

[tjquinno]

(after private DM exchange)

I have revised the PR so the implementations of unwrap do their own type checking and exception throwing. I think that's the revision you were asking for, although I am not sure I agree it's really needed or desirable.

(Sorry for the long message which follows. I think we are very close to agreeing on this but do differ on one point.)

I do not see what assumption you think the PR code is making about the types. Both in main and in the PR the caller can pass any type to unwrap. If the type is compatible, an object is cast to the type and returned; otherwise the code throws an exception.

And in both cases ultimately the code in Class.cast runs which does its own type check and exception throwing. The PR code simply delegates that work to Class.cast rather than duplicating the type check that Class.cast will do anyway and throwing our own IllegalArgumentException if the type is incompatible. Maybe that is what you are seeing as an assumption, but in the original PR code the unwrap implementations did not assume anything; that code simply attempts to do what the caller asks.

Your comment on the PR suggested "maybe throw a more relevant exception than just ClassCastException. TBH I do not see why IllegalArgumentException would be more relevant; in fact it's more general than ClassCastException--the caller is truly asking to have an object cast to an incompatible class. For the IAE to be clear, its message must describe the class mismatch as the underlying reason anyway.

Even so, now in the updated PR our unwrap methods do their own type checking and throw their own exceptions.

All that aside, the main problem was that the Span and Span.Builder interfaces provide default implementations of unwrap which checks and casts this but only OpenTracingSpan provided its own overriding implementation which checks and casts the delegate. The following classes have delegate instances inside them and should have--but did not--override the default implementation of unwrap to work with the delegate:

• OpenTracingSpanBuilder
• OpenTelemetrySpan
• OpenTelemetrySpanBuilder

The main point of the PR is to update those classes so they override the default implementation of unwrap so they use their delegates.

[tomas-langer]

Same here - unwrap I think is reasonable here