feat: extra fields

[hseeberger]

Ref #1531.

Currently the trace ID, added to Format, is only serialized for Json.

[davidbarsky]

Few notes:

• In general, please point PRs against the master branch. We'll handle backports to v0.1.x.
• I'm not sure we want to add additional dependencies to tracing-subscriber, even behind a feature flag.
• It's not clear to me what the behavior of this API would if the tracing-opentelemetry layer is not setup. What will happen in that case?
• I'm not really convinced on the utility of a method (in that I think creating a span with the ID is fine, but maybe given the number of support questions, it's not?), but if we are going to add non-span data to a formatter, I think I'd rather it be called .with_extra_fields or something and we'd provide an example of how to add, say, an OpenTelemetry trace ID.

[hseeberger]

Thanks, @davidbarsky, for the quick review!

In general, please point PRs against the master branch. We'll handle backports to v0.1.x.

Sure, will do. My motivation was to get a demo up and running with the PR, and this demo has dependencies which are not avaliable for master0.2.

I'm not sure we want to add additional dependencies to tracing-subscriber, even behind a feature flag.
I'm not really convinced on the utility of a method (in that I think creating a span with the ID is fine, but maybe given the number of support questions, it's not?), but if we are going to add non-span data to a formatter, I think I'd rather it be called .with_extra_fields or something and we'd provide an example of how to add, say, an OpenTelemetry trace ID.

I agree with you wrt the dependency (we do not need it) and that I should revert to what the original ticket was asking for, i.e. .with_extra_fields. That's equally easy to use for someone who wants to add the trace ID, but more general, i.e. also supports other use cases and does not pull in the otel dependency.

It's not clear to me what the behavior of this API would if the tracing-opentelemetry layer is not setup. What will happen in that case?
I think the trace ID will be invalid, i.e. lots of zeroes, but I'll add a demo/test for that.

Stay tuned for updates.

[hseeberger]

@davidbarsky, I think I have addressed all of the above.

If you are interested in using this new feature, check out https://github.com/hseeberger/hello-tracing-rs/blob/main/hello-tracing-backend/src/main.rs#L70.

[hseeberger]

This is how the new method can be used:

fmt::layer()
    .json()
    .with_span_list(false)
    .with_extra_fields(Box::new(|extensions| {
        extensions
            .get::<OtelData>()
            .map(|otel_data| {
                let trace_id = otel_data.parent_cx.span().span_context().trace_id();
                HashMap::from_iter(iter::once((
                    "trace_id".to_string(),
                    trace_id.to_string(),
                )))
            })
            .unwrap_or_default()
    })),

Notice calling with_span_list(false). If not called (i.e. using the default value true), the trace ID can easily be added by modifiying the root span, but that adds a lot of overhead to each log event. If one does not want the span list, this PR allows for seamless log-trace integration by tracing backends like Grafana or DataDog.

[hseeberger]

@davidbarsky, I know I haven't yet pointed this PR against master, because I wanted to have a working demo with this PR. I am pretty happy with how it is working. Please let me know if you consider this feature for worth merging and I can point the PR against master.

[hseeberger]

Hey @davidbarsky and @hawkw, I know you are busy, but nevertheless I'd really love to get a thumbs up (or down) to pointing this PR against master. Hoping to hear from you ...

[hseeberger]

@davidbarsky, OK, I have targeted these changes against the latest master in #2733, hence closing this one now.