-
Notifications
You must be signed in to change notification settings - Fork 492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OTLP Exporter builder to return more specific Error type #2654
base: main
Are you sure you want to change the base?
OTLP Exporter builder to return more specific Error type #2654
Conversation
@scottgerring @lalitb #2571 principles are followed, please review. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2654 +/- ##
=====================================
Coverage 79.1% 79.2%
=====================================
Files 120 119 -1
Lines 22556 22553 -3
=====================================
+ Hits 17857 17864 +7
+ Misses 4699 4689 -10 ☔ View full report in Codecov by Sentry. |
@@ -92,6 +93,42 @@ impl Default for ExportConfig { | |||
} | |||
} | |||
|
|||
#[derive(Error, Debug)] | |||
/// Errors that can occur while building an exporter. | |||
// TODO: Refine and polish this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😇
// TODO: Refine and polish this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! couple of minor comments inline
Signed-off-by: tison <[email protected]>
Co-authored-by: Braden Steffaniak <[email protected]> Co-authored-by: Cijo Thomas <[email protected]> Co-authored-by: Lalit Kumar Bhasin <[email protected]>
…2663) Co-authored-by: Cijo Thomas <[email protected]> Co-authored-by: Lalit Kumar Bhasin <[email protected]>
Fixes #2561
Also removes now unused Error enum from Log SDK. Another set of cleanup required to clean Metrics, Traces completely.
Also, the newly introduced enum
ExporterBuildError
is not fully refined, this can be done in follow ups.Note: This is a breaking change for OTLP Exporter, however, the impact is minimal (most users/examples simply unwrap/expect the Result instead of matching on it.), as described in the changelog, so I propose to make the change even though OTLP Exporter is marked RC.