Revisit error handling in trace

[TommyCpp]

Current approaches

Currently, most of the errors in trace have been returned via Box<dyn std::error::Error + Send + Sync + 'static>. This approach gives users good flexibility to return basically any errors. But it totally depends on user to return meaningful messages in errors to help end user to understand the issue.

Proposed approaches

As discussed in #286, we decided to use thiserror library wide to allow multiple level of errors throughout the whole process to send span.

1. We define a TraceError enum to represent any possible errors from trace module. It provides some predefined variantion, and we should add more variantions as needed. It also provides two generic variantions Other and Boxed. Other could hold any string and Boxed could hold any Box<dyn std::error::Error + Send + Sync + 'static>. It allows the user to adapt their currently implementation easily.

2. We define a ExporterError trait, which contains one method to return the name of the exporter. Exporter implementations are engouraged to define their own Error type and implement ExporterError. This allow any exporter implementations to pass their custom errors back to SDK. This should also help narrow down which exporter causes the error when errors being returned.

3. Change function signature to return TraceError or ExporterError when possible. And use global error handler to handle the rest of errors

[codecov]

Codecov Report

Merging #371 (47836ea) into master (ac8e4e5) will decrease coverage by 0.50%.
The diff coverage is 19.60%.

@@            Coverage Diff             @@
##           master     #371      +/-   ##
==========================================
- Coverage   54.00%   53.50%   -0.51%     
==========================================
  Files          71       72       +1     
  Lines        6051     6117      +66     
==========================================
+ Hits         3268     3273       +5     
- Misses       2783     2844      +61     

Impacted Files	Coverage Δ
opentelemetry-otlp/src/span.rs	2.67% <0.00%> (ø)
opentelemetry/src/api/metrics/mod.rs	0.00% <0.00%> (ø)
opentelemetry/src/api/mod.rs	0.00% <0.00%> (ø)
opentelemetry/src/api/trace/mod.rs	0.00% <0.00%> (ø)
opentelemetry/src/exporter/trace/mod.rs	56.52% <0.00%> (-7.68%)	⬇️
opentelemetry/src/exporter/trace/stdout.rs	17.64% <0.00%> (-5.43%)	⬇️
opentelemetry/src/global/error_handler.rs	0.00% <0.00%> (ø)
opentelemetry/src/sdk/metrics/processors/basic.rs	0.00% <0.00%> (ø)
opentelemetry/src/testing/trace.rs	65.06% <31.57%> (-10.70%)	⬇️
opentelemetry/src/sdk/trace/span_processor.rs	78.36% <72.22%> (-0.49%)	⬇️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac8e4e5...47836ea. Read the comment docs.

[jtescher]

Nice start, couple of thoughts as the draft progresses

[jtescher]

Looking good! only a couple of small comments, also looks like the msrv tests are now broken by an async std dependency, maybe we could feature flag those tests so they only run if async-std feature is used?

[jtescher]

Or maybe just remove async-std from msrv job in

opentelemetry-rust/.github/workflows/ci.yml

Line 55 in ac8e4e5

cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,async-std,serde,http,tonic,reqwest &&

if that fixes it

[jtescher]

small nits left but looking good, ready whenever out of draft state

[jtescher]

Looks good!