Skip to content

Commit

Permalink
Do not override span_id with otel value by default
Browse files Browse the repository at this point in the history
This is done to prevent breaking of behavior for people that have both set and already rely for regular span_id to be logged to DD.
  • Loading branch information
AndrewDryga committed Oct 18, 2022
1 parent 314b173 commit cc8d932
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/logger_json/formatters/datadog_logger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ defmodule LoggerJSON.Formatters.DatadogLogger do
# To connect logs and traces, span_id and trace_id keys are respectively dd.span_id and dd.trace_id
# https://docs.datadoghq.com/tracing/faq/why-cant-i-see-my-correlated-logs-in-the-trace-id-panel/?tab=jsonlogs
defp convert_tracing_keys(output, md) do
fields = %{

This comment has been minimized.

Copy link
@AndrewDryga

AndrewDryga Oct 18, 2022

Author Member

@btkostner I made a small change here: we should use a list instead of a map. With a map, we won't be able to control key priorities (so there would be no way to ensure otel fields do not override regular ones). This is to ensure that there are no breaking changes for people that use it currently and do not expect otel_span_id to override span_id, etc.

I pushed a new release and please tell me if this behavior doesn't work for you - we can expose priorities as a configuration.

span_id: ["dd.span_id", & &1],
trace_id: ["dd.trace_id", & &1],
otel_span_id: ["dd.span_id", &convert_otel_field/1],
otel_trace_id: ["dd.trace_id", &convert_otel_field/1]
}

Enum.reduce(fields, output, fn {key, [new_key, transformer]}, acc ->
# Notice: transformers can override each others but the last one in this list wins
[
otel_span_id: {"dd.span_id", &convert_otel_field/1},
otel_trace_id: {"dd.trace_id", &convert_otel_field/1},
span_id: {"dd.span_id", & &1},
trace_id: {"dd.trace_id", & &1}
]
|> Enum.reduce(output, fn {key, {new_key, transformer}}, acc ->
if Keyword.has_key?(md, key) do
new_value = transformer.(Keyword.get(md, key))
Map.put(acc, new_key, new_value)
Expand Down
19 changes: 19 additions & 0 deletions test/unit/logger_json/formatters/datadog_logger_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,25 @@ defmodule LoggerJSONDatadogTest do
assert Map.has_key?(log, "span_id") == false
end

test "trace_id/span_id has greater priority than otel_trace_id/otel_span_id" do
Logger.configure_backend(LoggerJSON, metadata: :all)

otel_id =
<<98, 56, 49, 48, 100, 98, 97, 50, 57, 56, 48, 51, 101, 101, 54, 49, 101, 55, 99, 55, 49, 102, 102, 48, 99, 50,
99, 57, 53, 97, 57, 100>>

Logger.metadata(otel_trace_id: otel_id, trace_id: "1", span_id: "2", otel_span_id: otel_id)

log =
fn -> Logger.debug("hello") end
|> capture_log()
|> Jason.decode!()

assert %{"dd.trace_id" => "1", "dd.span_id" => "2"} = log
assert Map.has_key?(log, "trace_id") == false
assert Map.has_key?(log, "span_id") == false
end

test "convert otel_trace_id/otel_span_id binary to expected datadog keys" do
Logger.configure_backend(LoggerJSON, metadata: :all)

Expand Down

0 comments on commit cc8d932

Please sign in to comment.