apmotel span.RecordError
should handle nil errors, as per OpenTelemetry's implementation
#1565
Labels
span.RecordError
should handle nil errors, as per OpenTelemetry's implementation
#1565
Is your feature request related to a problem? Please describe.
The
apmotel
bridge has proved extremely helpful in migrating to OpenTelemetry spans/tracers, but there is one noticeable difference inside theSpan
implementation:RecordError
.The OpenTelemetry SDK implementation of
span.RecordError()
includes a nil check for theerror
that is passed in.In contrast, the
apmotel
implementation does no such check. While this is understandable, and arguably more idiomatic, it introduces an issue where you cannot seamlessly drop in the bridge and usespan.RecordError
how you might expect to with the OpenTelemetry SDK.Describe the solution you'd like
I believe
apmotel
'sspan.RecordError
should do the same nil check to improve interoperability with the official OpenTelemetry SDK.Describe alternatives you've considered
With the official SDK, you might do something like:
For this code to coexist with the bridge, we'd then have to add a nil check in before the second line, and we'd have to do so everywhere we've already written a
recordError
usage like it.While we could do this, and indeed have opted to so far, we've noticed it just leaves us open to missed cases each time we attempt to adopt the bridge in a service to start its migration to OpenTelemetry.
edit: I have attached a draft PR that implements the requested change
The text was updated successfully, but these errors were encountered: