Skip to content
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

Should OTLP Exporter Error enum be public and visible to the processor? #2561

Open
lalitb opened this issue Jan 24, 2025 · 1 comment
Open

Comments

@lalitb
Copy link
Member

lalitb commented Jan 24, 2025

More for discussion.

The OTLP Exporter crate defines a public Error enum that is currently accessible throughout the crate and to external consumers. The processor, however, interacts with these errors through the ExportError trait, which abstracts away the specific details of the underlying Error enum.

  • Should the Error enum and its variants remain public, given that they are primarily relevant to the OTLP Exporter crate itself?
  • Is there a use case for the processor or other consumers to directly match on or handle specific variants of the Error enum, or is the abstraction provided by ExportError sufficient?
  • Does the processor need to take any action based on specific OTLP Error enum variants?
  • Should the processor be designed to react differently to specific errors (e.g., transport errors, invalid URIs, etc.), or does it suffice to treat all exporter errors uniformly?

In the current design, the processor wraps errors using LogError::ExportFailed, which stores a boxed ExportError:

LogError::ExportFailed(Box<dyn ExportError>)

To handle specific OTLP Error variants, the consumer(i.e, processor) must perform a manual downcast:

if let LogError::ExportFailed(export_error) = log_error {
    if let Some(otlp_error) = export_error.downcast_ref::<otlp::Error>() {
        match otlp_error {
            otlp::Error::Transport(err) => println!("Transport error: {}", err),
            otlp::Error::InvalidUri(err) => println!("Invalid URI: {}", err),
            otlp::Error::Status { code, message } => {
                println!("gRPC Status - Code: {:?}, Message: {}", code, message);
            }
            _ => println!("Other OTLP error"),
        }
    }
}

However, in that case, processor should handle such downcast and error handling for all other exporters (if they have any such error enum).

cc @scottgerring , @cijothomas

@lalitb lalitb changed the title Should OTLP Exporter Error Enum Be Public and Visible to the Processor? Should OTLP Exporter Error enum be public and visible to the processor? Jan 24, 2025
@cijothomas
Copy link
Member

My thoughts:
No need of exposing specific error codes from OTLP. It should do internal logging with details of the error, and then propagate ExportError enum back to processor.
ExportError can have timeout(Duration),Other(String) and maybe AlreadyShutdown()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants