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

No way to construct LogData #1217

Closed
FSMaxB opened this issue Aug 22, 2023 · 5 comments · Fixed by #1222
Closed

No way to construct LogData #1217

FSMaxB opened this issue Aug 22, 2023 · 5 comments · Fixed by #1222

Comments

@FSMaxB
Copy link
Contributor

FSMaxB commented Aug 22, 2023

There is currently no way to construct LogData directly, but I need it to feed a LogExporter with.

The issue is that there is no constructor and constructing it directly is prevented by the #[non_exhaustive] attribute.

There's a workaround, but it's absolutely less than pretty (the workaround is to create a fake LogProcessor and abuse a LoggerProvider to construct the LogData for us):

fn log_data(log_record: LogRecord) -> LogData {
    let (log_processor, log_data_receiver) = FakeLogProcesor::new();
    let provider = LoggerProvider::builder()
        .with_config(opentelemetry::sdk::logs::Config { resource: Cow::Owned(resource()) })
        .with_log_processor(log_processor)
        .build();

    let logger = provider.logger("logger name");

    logger.emit(log_record);

    log_data_receiver.recv().unwrap()
}

fn resource() -> Resource {
    Resource::new([KeyValue::new("service.name", "service-name")])
}

#[derive(Debug)]
struct FakeLogProcesor(std::sync::mpsc::Sender<LogData>);

impl FakeLogProcesor {
    pub fn new() -> (Self, std::sync::mpsc::Receiver<LogData>) {
        let (sender, receiver) = std::sync::mpsc::channel();
        (Self(sender), receiver)
    }
}

impl LogProcessor for FakeLogProcesor {
    fn emit(&self, data: LogData) {
        let _ = self.0.send(data);
    }

    fn force_flush(&self) -> LogResult<()> {
        Ok(())
    }

    fn shutdown(&mut self) -> LogResult<()> {
        Ok(())
    }
}
@lalitb
Copy link
Member

lalitb commented Aug 22, 2023

Can you elaborate on your use case - are you writing a log exporter and want to construct it for testing? The LogData is supposed to be constructed only within the SDK, and not part of the user-facing API/SDK.

@FSMaxB
Copy link
Contributor Author

FSMaxB commented Aug 22, 2023

My use case is manually sending out logs and traces and handle errors individually.

I'm building a service that sends logs and traces to our customers, each of which can configure their own endpoints (only OTLP for now). What this means is that I'm multiplexing between customer endpoints and need to explicitly handle errors when sending the data to a customer.

Basically my service exposes a REST API, that is called with a piece of telemetry data and the configuration of the customer endpoint and I need to respond with a success or error HTTP response depending on whether the telemetry data was successfully delivered to that customer.

I can't rely on the regular Logger trait for that, because it is a) not asynchronous and b) doesn't provide any error reporting, that's why I'm using the LogExporter trait instead, which requires LogData.

@FSMaxB
Copy link
Contributor Author

FSMaxB commented Aug 22, 2023

Note that SpanData, which I'm also using together with the SpanExporter trait, doesn't have the issue because it doesn't have the #[non_exhaustive] attribute.

@lalitb
Copy link
Member

lalitb commented Aug 22, 2023

Ok thanks for the details. Feel free to raise a PR to remove the #[non_exhaustive] attributes from LogData. Also, need to be aware that the internal structure/implementation can change as Log signal is still not stable.

@FSMaxB
Copy link
Contributor Author

FSMaxB commented Aug 23, 2023

Ok thanks for the details. Feel free to raise a PR to remove the #[non_exhaustive] attributes from LogData.

Done.

Also, need to be aware that the internal structure/implementation can change as Log signal is still not stable.

Yeah, that is fine, I can adapt when changes happen.

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

Successfully merging a pull request may close this issue.

2 participants