ddtracer/tracer: marshalled span values in encode span

[dianashevchenko]

Illegal characters, such as newlines, were not escaped in encodeSpan function, which ruined json structure. Closes #791

This, in turn, corrupted log collection

[jamestelfer]

I'm interested in this fix since it's affecting me too: I appreciate that it's being worked on!

Should this fix be widened such that the same technique is used in all cases where WriteString is being called with input that is not a string literal? json.Marshal will also deal properly with embedded quotes and other control characters that would otherwise render invalid JSON.

The spec has details of the cases to cover, and the Marshal docs have further information.

Lastly, should this use an explicit Encoder and opt out of HTML escaping with SetHTMLEscape(false)? Since this output isn't destined for an HTML context, this would slightly decrease the encode/decode time for the same data. I'm assuming (without any background in this codebase) that performance was one of the reasons that this didn't use a JSON encoder originally.

[gbbr]

@jamestelfer you're right. We should use this everywhere where we write strings that are user input and aren't numeric (which get converted easily), such as s.Name, s.Resource and s.Service (I think that's it). Also agree with the HTML escaping... No need for it.

It'll likely help to create a helper function marshalString and use that where it applies.

[jamestelfer]

@gbbr / @dianashevchenko LMK if you'd like me to pitch in here! I'm currently blocked on this and would be happy to.

[gbbr]

@jamestelfer all comments welcome and help is much appreciated!

[jamestelfer]

This is looking great, thanks @dianashevchenko! I'm going to pull this branch and test it against the service I have that's failing.

[jamestelfer]

I forked this branch and merged v1, then pulled that version into my service using go mod edit -replace. I could see the trace written to a single line in CloudWatch, and the error trace appeared correctly in the Datadog UI. I'll look to pull the official version in as soon as it's released.

Looks good to me, and thanks again!

[gbbr]

LGTM!

[gbbr]

@jamestelfer thanks for confirming that this fixes your issue.