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

Nested Spans don't work correctly in JSON logs #704

Open
sazzer opened this issue May 3, 2020 · 1 comment
Open

Nested Spans don't work correctly in JSON logs #704

sazzer opened this issue May 3, 2020 · 1 comment

Comments

@sazzer
Copy link

@sazzer sazzer commented May 3, 2020

Bug Report

Version

-> % cargo tree | grep tracing
├── tracing v0.1.13
│   ├── tracing-attributes v0.1.7
│   └── tracing-core v0.1.10
├── tracing-futures v0.2.4
│   └── tracing v0.1.13 (*)
├── tracing-log v0.1.1
│   └── tracing-core v0.1.10 (*)
├── tracing-subscriber v0.2.5
│   ├── tracing-core v0.1.10 (*)
│   ├── tracing-log v0.1.1 (*)
│   └── tracing-serde v0.1.1
│       └── tracing-core v0.1.10 (*)

Platform

-> % uname -a
Darwin xxxxx.local 19.4.0 Darwin Kernel Version 19.4.0: Wed Mar  4 22:28:40 PST 2020; root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64

Crates

  • tracing-subscriber I believe

Description

Nesting spans - specifically I'm using #[instrument] but I assume any method is the same - works great when using the normal text subscriber. But when using the JSON subscriber, only the innermost span details get returned.

For example, in the following:

#[tracing::instrument]
fn outer(a: u16) {
    inner(a);
}

#[tracing::instrument]
fn inner(b: u16) {
    log::info!("Hello from Log");
    tracing::info!("Hello from Tracing");
}

If I call this from a main function as follows:

fn main() {
    tracing_subscriber::fmt().init();
    outer(1);
}

Then the log outputs are:

May 03 17:57:47.336  INFO outer{a=1}:inner{b=1}: xxx: Hello from Log    
May 03 17:57:47.338  INFO outer{a=1}:inner{b=1}: xxx: Hello from Tracing

Which is exactly as expected. But if I instead use this:

fn main() {
    tracing_subscriber::fmt().json().flatten_event(false).init();
    outer(1);
}

Then the output becomes:

{"timestamp":"May 03 17:58:33.534","level":"INFO","span":{"b":"1","name":"inner"},"target":"xxx","fields":{"message":"Hello from Log","log.target":"xxx","log.module_path":"xxx","log.file":"src/main.rs","log.line":64}}
{"timestamp":"May 03 17:58:33.535","level":"INFO","span":{"b":"1","name":"inner"},"target":"xxx","fields":{"message":"Hello from Tracing"}}

Note that the span element contains "inner" but not "outer".

Since JSON logging is especially useful in production environments where you might want to have tooling running off of the structured logs, this is especially frustrating.

Cheers

@hawkw
Copy link
Member

@hawkw hawkw commented May 3, 2020

I think that fixing this should be fairly straightforward. We would want to update this code here:

let id = ctx.ctx.current_span();
let id = id.id();
if let Some(id) = id {
if let Some(span) = ctx.ctx.span(id) {
let ext = span.extensions();
let data = ext
.get::<FormattedFields<N>>()
.expect("Unable to find FormattedFields in extensions; this is a bug");
// TODO: let's _not_ do this, but this resolves
// https://github.com/tokio-rs/tracing/issues/391.
// We should probably rework this to use a `serde_json::Value` or something
// similar in a JSON-specific layer, but I'd (david)
// rather have a uglier fix now rather than shipping broken JSON.
let mut fields: Value = serde_json::from_str(&data)?;
fields["name"] = json!(span.metadata().name());
serializer.serialize_entry("span", &fields).unwrap_or(());
}
}

and make it behave more like the default formatter, visiting all spans in the trace:
self.ctx.visit_spans(|span| {
write!(f, "{}", bold.paint(span.metadata().name()))?;
seen = true;
let ext = span.extensions();
let fields = &ext
.get::<FormattedFields<N>>()
.expect("Unable to find FormattedFields in extensions; this is a bug");
if !fields.is_empty() {
write!(f, "{}{}{}", bold.paint("{"), fields, bold.paint("}"))?;
}
f.write_char(':')
})?;

I think it would probably make sense to format the spans as a JSON array, in order?

We may want to make this configurable.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.