search

Nebo15 / logger_json

JSON logger formatter with support for Google Cloud, DataDog and other for Elixir.
https://nebo15.github.io/logger_json/
MIT License
237 stars 92 forks source link

Prefix span_id and trace_id keys with "dd." to connect logs and traces #84

Closed davidjulien closed 2 years ago

davidjulien commented 2 years ago

Otherwise datadog is unable to link a log entry to the corresponding trace. See https://docs.datadoghq.com/tracing/faq/why-cant-i-see-my-correlated-logs-in-the-trace-id-panel/?tab=jsonlogs : "ensure the name of the logs attribute that contains the trace id is dd.trace_id"

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.5%) to 75.309% when pulling 40b537707269bd93032bdfc8e2b897834e8d49af on WTTJ:fix-span-id-and-trace-id-keys-for-datadog into 640e86a09ad0ebca913b291768da0b121ec1ea6d on Nebo15:master.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.5%) to 75.309% when pulling 40b537707269bd93032bdfc8e2b897834e8d49af on WTTJ:fix-span-id-and-trace-id-keys-for-datadog into 640e86a09ad0ebca913b291768da0b121ec1ea6d on Nebo15:master.

AndrewDryga commented 2 years ago

@davidjulien Hello, this change looks great, thank you! Who is setting span_id and trace_id in the Logger metadata? I'm thinking if we can make this change in some backwards-compatible way so that we won't break existing filters for people that use this

davidjulien commented 2 years ago

Hi !

I'm using LoggerJSON with Spandex.

I think it's normal to have span_id and trace_id keys in metadata when you log a message. However, according to the documentation, Datadog requires dd.span_id and dd.trace_id keys to connect your traces with your logs. It doesn't work when you use span_id and trace_id keys. All examples in other languages use dd.span_id and dd.trace_id: java, ruby ...

But you're right. To keep backward compatibility, we can keep span_id and trace_id keys and add two new keys (dd.trace_id and dd.span_id). Is it ok for you ?

AndrewDryga commented 2 years ago

@davidjulien I think we can break the compatibility if it's set by Spandex, since it's just for DataDog. We can release a major version for that so that people check this change before upgrading.

AndrewDryga commented 2 years ago

Please try the master branch and tell me if everything goes smooth (or not), I'll release a hex version a bit later.

davidjulien commented 2 years ago

Thank you @AndrewDryga . It's working as expected. I have an other PR to submit before you build a new release.