apm.Error implements error interface

[psrebniak]

Is your feature request related to a problem? Please describe.
At the moment there is no possibility to pass Error as error interface.
We have to redeclare type as type ApmError apm.Error and define methods there.

Describe the solution you'd like
func (e *Error) Error() string should be implemented

Is there any reason, why Error does not implement error interface?
I can prepare pull request for that feature if you're interested in.

[axw]

@psrebniak apm.Error wasn't designed to be passed around as an error, but I'm interested to know why you would want to do that. It's only meant for capturing an error to be sent to Elastic APM.

[psrebniak]

Ok, so let's define two usecases:

1. http -> mux.Router -> someGetMethod -> Service (here we got error) -> [call to external api]
2. worker -> Service (here we got error) -> [call to external api]

In first case, all errors should be handled
In second case, some errors should be ommited but returned, and we should have a possibility to raport other errors (so we cannot just disabled all raporting).

If we could pass apm.Error as error, we could return that from service, and outer function should decide what to do with this.

We use own package to check type of error, if it is ApmError - send it, otherwise - wrap as ApmError and Submit

We define errors inside Service to set apm.Span on that apm.Error.

[axw]

OK, so in summary you would like to be able to defer the decision to call (*apm.Error).Send to your business logic layer (worker) while retaining the connection between the reported error and the inner span (e.g. for Service).

To answer your question "Is there any reason, why Error does not implement error interface?", only that I had not yet considered this scenario. Your suggestion seems reasonable to me.

I can prepare pull request for that feature if you're interested in.

That would be great, I'd be happy to review your pull request.

One request I have is that the *apm.Error should also implement the following interface, to support github.com/pkg/errors.Cause:

type causer interface {
    Cause() error
}

This would enable the handling code to inspect the underlying error. At the moment, neither Tracer.NewError nor Tracer.NewErrorLog record the original error value; they will need to be changed.

[psrebniak]

Ok, should I keep original error in Error or ErrorData?
I'm not sure because ErrorData is purged after Error.Send() and I think original error should not be removed after sending error.
I would prefer to keep it directly in Error but wanted to ask beforehand.

[axw]

I agree, putting it directly in Error seems best.