call finish() on a finished span

[tutufool]

Hi,

I'm curious about expected behaviors of below code:

....
span.finish();

//should below be ignored?
span.setTag("k1", "v1");
span.finish();

I saw the javadoc mentioned finish() should be the last call. But what if some user call finish() again?

Should this finish() call be silently ignored or throw some exception to inform user (from implementation perspective)?

Thanks

Leon

[yurishkuro]

the behavior of the tracer in this case is undefined. Some tracers might ignore the second call to Finish(), but doing this second call is a clear bug in the instrumentation, so making the noop behavior required by the API would only encourage malformed instrumentation.

[tutufool]

Yes, I prefer to inform user "this is wrong usage" rather than ignore it. But from the javadoc, this API seems to accept a noop behavior.

Should we introduce something like "SpanAlreadyFinishedException"? Or just let implementation make the call?

Thanks

Leon

[pavolloffay]

Should we introduce something like "SpanAlreadyFinishedException"? Or just let implementation make the call?

Tracing code should not crash the app. MockTracer throws an exception in this case, but that is designed for testing.

[tutufool]

So I think the ideal impl would be:

Ignore the second call to finish() and log some information to let user know this is a malformed operation.

What do you think?