From eedadc6e23cc5a4c193525a3a54ba2d47db5109b Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Mon, 4 Jun 2018 12:13:10 -0400 Subject: [PATCH] allowing for non-inheriting private meta (#54) Resolves #50 --- lib/datadog/api_server.ex | 35 +++++---- lib/plug/end_trace.ex | 2 +- lib/span.ex | 122 ++++++++++++++++++------------- lib/span/error.ex | 9 --- lib/span/http.ex | 13 ---- lib/span/sql_query.ex | 10 --- test/datadog/api_server_test.exs | 2 +- 7 files changed, 92 insertions(+), 101 deletions(-) delete mode 100644 lib/span/error.ex delete mode 100644 lib/span/http.ex delete mode 100644 lib/span/sql_query.ex diff --git a/lib/datadog/api_server.ex b/lib/datadog/api_server.ex index 760b27f..36b65b0 100644 --- a/lib/datadog/api_server.ex +++ b/lib/datadog/api_server.ex @@ -179,9 +179,7 @@ defmodule Spandex.Datadog.ApiServer do response = traces |> Enum.map(fn trace -> - trace - |> Enum.map(&format/1) - |> Enum.sort_by(&Map.get(&1, :start)) + Enum.map(trace, &format/1) end) |> encode() |> push(state) @@ -233,9 +231,9 @@ defmodule Spandex.Datadog.ApiServer do defp add_error_data(meta, %{error: error}) do meta - |> add_error_type(error.exception) - |> add_error_message(error.exception) - |> add_error_stacktrace(error.stacktrace) + |> add_error_type(error[:exception]) + |> add_error_message(error[:exception]) + |> add_error_stacktrace(error[:stacktrace]) end @spec add_error_type(map, Exception.t() | nil) :: map @@ -259,14 +257,14 @@ defmodule Spandex.Datadog.ApiServer do defp add_http_data(meta, %{http: http}) do status_code = - if http.status_code do - to_string(http.status_code) + if http[:status_code] do + to_string(http[:status_code]) end meta - |> Map.put("http.url", http.url) + |> Map.put("http.url", http[:url]) |> Map.put("http.status_code", status_code) - |> Map.put("http.method", http.method) + |> Map.put("http.method", http[:method]) end @spec add_sql_data(map, Spandex.Span.t()) :: map @@ -274,9 +272,9 @@ defmodule Spandex.Datadog.ApiServer do defp add_sql_data(meta, %{sql_query: sql}) do meta - |> Map.put("sql.query", sql.query) - |> Map.put("sql.rows", sql.rows) - |> Map.put("sql.db", sql.db) + |> Map.put("sql.query", sql[:query]) + |> Map.put("sql.rows", sql[:rows]) + |> Map.put("sql.db", sql[:db]) end @spec add_tags(map, Spandex.Span.t()) :: map @@ -286,9 +284,16 @@ defmodule Spandex.Datadog.ApiServer do Map.merge(meta, Enum.into(tags, %{})) end - @spec error(nil | Spandex.Span.Error.t()) :: integer + @spec error(nil | Keyword.t()) :: integer defp error(nil), do: 0 - defp error(_), do: 1 + + defp error(keyword) do + if Enum.any?(keyword, fn {_, v} -> not is_nil(v) end) do + 1 + else + 0 + end + end @spec encode(data :: term) :: iodata | no_return defp encode(data), diff --git a/lib/plug/end_trace.ex b/lib/plug/end_trace.ex index 680fcfc..93974ab 100644 --- a/lib/plug/end_trace.ex +++ b/lib/plug/end_trace.ex @@ -37,7 +37,7 @@ defmodule Spandex.Plug.EndTrace do Keyword.merge([http: [status_code: conn.status]], tracer_opts) else Keyword.merge( - [http: [status_code: conn.status], error: %Spandex.Span.Error{}], + [http: [status_code: conn.status], error: [error?: true]], tracer_opts ) end diff --git a/lib/span.ex b/lib/span.ex index ed86781..e60fdc2 100644 --- a/lib/span.ex +++ b/lib/span.ex @@ -10,6 +10,7 @@ defmodule Spandex.Span do :id, :name, :parent_id, + :private, :resource, :service, :sql_query, @@ -19,27 +20,22 @@ defmodule Spandex.Span do :type ] - @nested_opt_structs %{ - http: %Spandex.Span.Http{}, - sql_query: %Spandex.Span.SqlQuery{}, - error: %Spandex.Span.Error{} - } - - @nested_opts Map.keys(@nested_opt_structs) + @nested_opts [:error, :http, :sql_query] @type timestamp :: term @type t :: %__MODULE__{ completion_time: timestamp, env: String.t(), - error: Spandex.Span.Error.t() | nil, + error: Keyword.t() | nil, id: term, name: String.t(), parent_id: term, + private: Keyword.t(), resource: String.t(), service: atom, - http: Spandex.Span.Http.t() | nil, - sql_query: Spandex.Span.SqlQuery.t() | nil, + http: Keyword.t() | nil, + sql_query: Keyword.t() | nil, start: timestamp, trace_id: term, tags: Keyword.t() | nil, @@ -51,22 +47,24 @@ defmodule Spandex.Span do id: :any, trace_id: :any, name: :string, - http: [:keyword, {:struct, Spandex.Span.Http}], - error: [:keyword, {:struct, Spandex.Span.Error}], + http: :keyword, + error: :keyword, completion_time: :any, env: :string, parent_id: :any, + private: :keyword, resource: [:atom, :string], service: :atom, services: :keyword, - sql_query: [:keyword, {:struct, Spandex.Span.SqlQuery}], + sql_query: :keyword, start: :any, type: :atom, tags: :keyword ], defaults: [ tags: [], - services: [] + services: [], + private: [] ], required: [ :id, @@ -94,6 +92,36 @@ defmodule Spandex.Span do Update an existing span. #{Optimal.Doc.document(Map.put(@span_opts, :required, []))} + + ## Special Meta + + ```elixir + [ + http: [ + url: "my_website.com?foo=bar", + status_code: "400", + method: "GET", + query_string: "foo=bar", + user_agent: "Mozilla/5.0...", + request_id: "special_id" + ], + error: [ + exception: ArgumentError.exception("foo"), + stacktrace: System.stacktrace(), + error?: true # Used for specifying that a span is an error when there is no exception or stacktrace. + ], + sql_query: [ + rows: 100, + db: "my_database", + query: "SELECT * FROM users;" + ], + # Private has the same structure as the outer meta structure, but private metadata does not + # transfer from parent span to child span. + private: [ + ... + ] + ] + ``` """ def update(span, opts, schema \\ Map.put(@span_opts, :required, [])) do opts_without_nils = Enum.reject(opts, fn {_key, value} -> is_nil(value) end) @@ -102,21 +130,7 @@ defmodule Spandex.Span do span |> Map.take(schema.opts) |> Enum.reject(fn {_key, value} -> is_nil(value) end) - |> Keyword.merge(opts_without_nils, fn key, v1, v2 -> - case key do - k when k in @nested_opts -> - left = struct_to_keyword(v1) - right = struct_to_keyword(v2) - - merge_non_nils(left, right) - - :tags -> - Keyword.merge(v1 || [], v2 || []) - - _ -> - v2 - end - end) + |> merge_retaining_nested(opts_without_nils) with_type = case {starting_opts[:type], starting_opts[:services]} do @@ -130,6 +144,32 @@ defmodule Spandex.Span do validate_and_merge(span, with_type, schema) end + @spec merge_retaining_nested(Keyword.t(), Keyword.t()) :: Keyword.t() + defp merge_retaining_nested(left, right) do + Keyword.merge(left, right, fn key, v1, v2 -> + case key do + k when k in @nested_opts -> + left = struct_to_keyword(v1) + right = struct_to_keyword(v2) + + merge_non_nils(left, right) + + :tags -> + Keyword.merge(v1 || [], v2 || []) + + :private -> + if v1 && v2 do + merge_retaining_nested(v1, v2) + else + v1 || v2 + end + + _ -> + v2 + end + end) + end + @spec merge_non_nils(Keyword.t(), Keyword.t()) :: Keyword.t() defp merge_non_nils(left, right) do Keyword.merge(left, right, fn _k, v1, v2 -> @@ -145,35 +185,13 @@ defmodule Spandex.Span do defp validate_and_merge(span, opts, schema) do case Optimal.validate(opts, schema) do {:ok, opts} -> - non_composite_opts = Keyword.drop(opts, @nested_opts) - - span = struct(span, non_composite_opts) - - opts - |> Keyword.take(@nested_opts) - |> Enum.reduce(span, fn {key, opts}, span -> - struct = Map.get(span, key) || @nested_opt_structs[key] - - value = do_merge(struct, opts) - - Map.put(span, key, value) - end) + struct(span, opts) {:error, errors} -> {:error, errors} end end - @spec do_merge(struct, struct | Keyword.t()) :: struct - defp do_merge(struct, keyword_or_opts) do - struct_name = struct.__struct__ - - case keyword_or_opts do - %^struct_name{} -> Map.merge(struct, keyword_or_opts) - keyword -> struct(struct, keyword) - end - end - def child_of(%{id: parent_id} = parent, name, id, start, opts) do child = %{parent | id: id, name: name, start: start, parent_id: parent_id} diff --git a/lib/span/error.ex b/lib/span/error.ex deleted file mode 100644 index e44922b..0000000 --- a/lib/span/error.ex +++ /dev/null @@ -1,9 +0,0 @@ -defmodule Spandex.Span.Error do - @moduledoc "Contains error related metadata." - defstruct [:exception, :stacktrace] - - @type t :: %__MODULE__{ - exception: Exception.t() | nil, - stacktrace: [] | nil - } -end diff --git a/lib/span/http.ex b/lib/span/http.ex deleted file mode 100644 index a4800f9..0000000 --- a/lib/span/http.ex +++ /dev/null @@ -1,13 +0,0 @@ -defmodule Spandex.Span.Http do - @moduledoc "Contains http related metadata." - defstruct [:url, :status_code, :method, :query_string, :user_agent, :request_id] - - @type t :: %__MODULE__{ - url: String.t() | nil, - status_code: non_neg_integer() | nil, - method: String.t() | nil, - query_string: String.t() | nil, - user_agent: String.t() | nil, - request_id: term | nil - } -end diff --git a/lib/span/sql_query.ex b/lib/span/sql_query.ex deleted file mode 100644 index 380d1fd..0000000 --- a/lib/span/sql_query.ex +++ /dev/null @@ -1,10 +0,0 @@ -defmodule Spandex.Span.SqlQuery do - @moduledoc "Contains sql query related metadata." - defstruct [:rows, :db, :query] - - @type t :: %__MODULE__{ - rows: non_neg_integer() | nil, - db: String.t() | nil, - query: String.t() | nil - } -end diff --git a/test/datadog/api_server_test.exs b/test/datadog/api_server_test.exs index fa20974..62f2430 100644 --- a/test/datadog/api_server_test.exs +++ b/test/datadog/api_server_test.exs @@ -48,7 +48,7 @@ defmodule Spandex.Datadog.ApiServerTest do { :ok, [ - spans: [span_2, span_1], + spans: [span_1, span_2], url: "localhost:8126/v0.3/traces", state: %ApiServer{ asynchronous_send?: false,