Simplify capturing errors from Spans

[savaki]

It's very useful to know when a Span has failed and to capture the error. However, with the current mechanism, log.Error, one ends up with code that looks like this:

span, ctx := opentracing.StartSpanFromContext(ctx, "operationName")
defer span.Finish()

if err := boom(); err != nil {
  err := errors.Wrapf(err, "unable to do %v to %v", x, y)
  ext.Error.Set(span, true)
  return nil, err
}

Instead, have you considered adding Error/Errorf or Wrap/Wrapf funcs to span similar to Dave Cheney's error package? The previous code could be reduced to:

span, ctx := opentracing.StartSpanFromContext(ctx, "operationName")
defer span.Finish()

if err := boom(); err != nil {
  return nil, span.Errorf(err, "unable to do %v to %v", x, y)
}

[savaki]

I put together an opentracing implementation for Google's stackdriver, github.com/savaki/stackdriver that integrates with Stackdriver Trace, Logging, and Error Reporting. Being able to capture errors with the context of the Span is incredibly useful, but currently quite cumbersome.

[tj]

👍 would be cool to have something like .Finish(&err) for the defer

[yurishkuro]

@savaki it does not need to be a span method (which would be a breaking change), can be just a util function:

if err := boom(); err != nil {
  return nil, opentracing.Errorf(span, "unable to do %v to %v: %w", x, y, err)
}

@tj I think .Finish(&err) would promote poor coding practice where err needs to be declared prior to defer, potentially far away from where it's initialized, and forcing it to leak to the outer scope (compared to the if statement above).

[tj]

@yurishkuro I don't actually use defer much in practice, not sure why I suggested that haha but yeah I agree