Handle exceptions in Instrumenter.start() and Instrumenter.end() methods

[steverao]

Resolved #16376

[laurit]

Let's assume that there was an exception thrown in start and we return the parent context. Now in end we'll end up closing the span from parent context which isn't ideal.

[steverao]

Thanks, how about using a unique ContextKey to mark the Context that "start failed". If end() detects this mark, skip it directly to avoid erroneously operating on the parent span?

[laurit]

I doubt this would work.

[steverao]

I'm wondering if your concern is about the edge case where the span is created but an exception is thrown afterwards (e.g., in OperationListener.onStart()).
In that case, the created span won't be properly ended. How about a solution that wraps the span creation and listener calls in try-catch to ensure the span is ended if an exception occurs after creation or do you have any suggestion?

Span span = null;
try {
    span = spanBuilder.startSpan();
    // ... listeners ...
    return spanSuppressor.storeInContext(context, spanKind, span);
} catch (Throwable t) {
    if (span != null) span.end();
    throw t;
}

[laurit]

I don't follow. If you rethrow the exception from start won't it defeat the purpose of catching the exception in first place?
Instead of trying to wrap the whole method in try catch you could add try catch around individual parts that are more likely to throw e.g. attribute extraction.