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

Expose fmt::format::Writer::new() #2512

Closed
kaifastromai opened this issue Mar 18, 2023 · 1 comment
Closed

Expose fmt::format::Writer::new() #2512

kaifastromai opened this issue Mar 18, 2023 · 1 comment

Comments

@kaifastromai
Copy link
Contributor

kaifastromai commented Mar 18, 2023

Feature Request

Make the constructer of Writer from tracing_subscriber available outside of the crate. I'm not sure I should file this issue in a different repo since tracing_subscriber is in it's own repo.

Crates

tracing-subscriber

Motivation

I have a server that needs to be logged remotely. What I am trying to do is add a Layer that copies the formatted output from fmt and sends it down a tokio channel (and subsequently a grpc stream), but I can't just use with_writer since I also want to attach the log level and other context data. I managed to implement my own Layer based upon the example:

pub struct JobLogLayer<E> {
    pub channel: tokio::sync::mpsc::UnboundedSender<Result<JobHostEvent, E>>,
}
pub struct DefaultVisitor {
    pub string: String,
}
impl tracing_subscriber::field::Visit for DefaultVisitor {
    fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) {
        if field.name() == "message" {
            self.string.push_str(&format!("{value:?}"));
        } else {
            self.string
                .push_str(&format!("\n{}: {value:?} ", field.name()));
        }
    }
}

impl<S, E> tracing_subscriber::Layer<S> for JobLogLayer<E>
where
    S: tracing::Subscriber,
    E: 'static,
{
    fn on_event(
        &self,
        _event: &tracing::Event<'_>,
        _ctx: tracing_subscriber::layer::Context<'_, S>,
    ) {
        //send the event to the channel
        let mut v = DefaultVisitor {
            string: String::new(),
        };
        _event.record(&mut v);
        //get the log level
        let level = _event.metadata().level();
        let job_level = match level.as_str() {
            "TRACE" => crate::job::JobLogLevel::Trace,
            "DEBUG" => crate::job::JobLogLevel::Debug,
            "INFO" => crate::job::JobLogLevel::Info,
            "WARN" => crate::job::JobLogLevel::Warn,
            "ERROR" => crate::job::JobLogLevel::Error,
            _ => unreachable!(),
        };
        let job_log = crate::job::JobLog {
            message: v.string,
            level: job_level,
        };
        let json = serde_json::to_string(&job_log)
            .context("Could not serialize job log")
            .unwrap();
        let log = JobHostEvent {
            event_type: JobHostEventType::JobLog as i32,
            json,
        };
        let res = self.channel.send(Ok(log));
        match res {
            Ok(_) => {}
            Err(e) => {
                print!("Error sending log: {e}");
            }
        }
    }
}

and that allowed me get access to this data, but I'd rather not try to reimplement (and at present have no idea how one would implement) tracing-subscriber's fmt stuff. After digging through the source code some more, I found the formatter layer that seems to have mosts things already configured for me:
(this is ripped directly from the documentation in the source code)

pub struct MoscaEventFormatter {}
impl<S, N> FormatEvent<S, N> for MoscaEventFormatter
where
    S: Subscriber + for<'a> LookupSpan<'a>,
    N: for<'a> FormatFields<'a> + 'static,
{
    fn format_event(
        &self,
        ctx: &FmtContext<'_, S, N>,
        mut writer: format::Writer<'_>,
        event: &Event<'_>,
    ) -> std::fmt::Result {
        // Format values from the event's's metadata:
        let metadata = event.metadata();
        write!(&mut writer, "{} {}: ", metadata.level(), metadata.target())?;
        use std::fmt::Write;
        
        // Format all the spans in the event's span context.
        if let Some(scope) = ctx.event_scope() {
            for span in scope.from_root() {
                write!(writer, "{}", span.name())?;

                // `FormattedFields` is a formatted representation of the span's
                // fields, which is stored in its extensions by the `fmt` layer's
                // `new_span` method. The fields will have been formatted
                // by the same field formatter that's provided to the event
                // formatter in the `FmtContext`.
                let ext = span.extensions();
                let fields = &ext
                    .get::<fmt::FormattedFields<N>>()
                    .expect("will never be `None`");
                
                // Skip formatting the fields if the span had no fields.
                if !fields.is_empty() {
                    write!(writer, "{{{}}}", fields)?;
                }
                write!(writer, ": ")?;
            }
        }

        // Write fields on the event
        ctx.field_format().format_fields(writer.by_ref(), event)?;
        writeln!(writer)
        
    }
}

However, I now need to access the data in the writer, which is not possible. I initially thought I could just ignore the parameter and create my own writer, but the Writer::new(...) is private. I did there was a comment to(I believe) @hawkw about making this public and making the necessary considerations.

I am very new to tracing (and tokio for that matter) so I'm not sure this is the best way to accomplish my outlined goals.

Proposal

Remove the pub(crate) and just make it pub.
There comments about considerating creating an explicit from_string() constructor or some such.

Alternatives

I have no idea

goodspark added a commit to goodspark/rust-tracing that referenced this issue Mar 21, 2023
Solves tokio-rs#2512

I'm writing a custom formatting layer and want to use UtcTime and format_time to format the time into a string.
@TmLev
Copy link
Sponsor

TmLev commented Mar 30, 2023

Also related: #2223

@hawkw hawkw closed this as completed in 99e0377 Oct 11, 2023
hawkw added a commit that referenced this issue Oct 12, 2023
## Motivation

As seen here #2512 and #2223. Previously pr'ed here #2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct

## Solution

Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.

Closes  #2512
Closes #2223

Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
davidbarsky pushed a commit that referenced this issue Oct 12, 2023
## Motivation

As seen here #2512 and #2223. Previously pr'ed here #2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct

## Solution

Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.

Closes  #2512
Closes #2223

Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
kaffarell pushed a commit to kaffarell/tracing that referenced this issue Nov 21, 2023
## Motivation

As seen here tokio-rs#2512 and tokio-rs#2223. Previously pr'ed here tokio-rs#2525, but no progress has
been made there for quite some. I have applied the suggested changes. Not sure
the formatting of the doc is sufficient or otherwise correct

## Solution

Make the `format::Writer::new()` function public. I don't see any obvious
trade-offs, but I am not familiar with the larger direction of
tracing/subscriber, so I may be wrong.

Closes  tokio-rs#2512
Closes tokio-rs#2223

Co-authored-by: Cephas Storm <cephas.storm@borisfx.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
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