Unify Errors

[jtescher]

Currently the metrics module uses a MetricsError type while the trace module was created before the spec allowed for error results and therefore does not have any result types. Exporters are using variations of std::error::Error. We should consider introducing an opentelemetry::Error type and update the exporters to use it. global::handle_error should also accept this type, and the tracing module should call this function internally instead of dropping errors.

[cpcloud]

Would it be appropriate to use thiserror library-wide?

[jtescher]

@cpcloud yeah I think that probably makes the most sense

[jtescher]

@cpcloud it could also be useful to re-evalutate the proper level of granularity, i.e. as a consumer of these APIs, what level of error information is worth responding to? Could for example go down to effectively two enum values, Trace and Metrics, and have data within those be opaque, or have some categories within those like an initialization error vs a runtime error, etc

[cpcloud]

The way I would want to handle errors from this library (and related workspace crates) is:

1. opentelemetry::Error enum with as-specific-as-possible variants for all errors that are not specific to a framework (what are effectively the workspace crates)
2. <workspace crate>::Error enum with as-specific-as-possible variants for all errors pertaining to the particular framework (e.g., opentelemetry_jaeger::Error, etc)

That way, I can easily choose (as long as my crate is using thiserror!) to propagate errors up, or handle specific variants.

[jtescher]

I'd be happy to review a PR to that effect, are there good example crates that define errors well with respect to granularity that you've found?

[jtescher]

#263 would likely be a part of this

[danburkert]

Perhaps at the risk of piling on here, I'm finding opentelemetry-jaeger to be difficult to work with because the error type it returns Box<dyn std::error::Error + ...> doesn't impl std::error::Error. If this isn't clear, see this compile failure: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f14fe95aeec58829b465ddad594f1e41

[frigus02]

Hm, that doesn't compile for me because dyn Error isn't sized. When I change this to &dyn Error and call as_ref() on the boxed error, it compiles successfully (updated playground). I thought this is how it's supposed to work. But then again, my experience with writing good errors in Rust is quite limited.

[TommyCpp]

close via #371