From 6e853491bbd774e6e96f82b0773c706c5f50743b Mon Sep 17 00:00:00 2001 From: Kevin Tayah Date: Sun, 15 Oct 2023 15:46:01 -0400 Subject: [PATCH 1/6] Add ability to expand_all_maps from requested_attributes --- config/config.exs | 6 +++-- lib/open_telemetry_decorator.ex | 5 +++- lib/open_telemetry_decorator/attributes.ex | 24 +++++++++++++++---- .../attributes_test.exs | 15 ++++++++++++ 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/config/config.exs b/config/config.exs index 41984ff..2f4050b 100644 --- a/config/config.exs +++ b/config/config.exs @@ -7,8 +7,10 @@ # General application configuration import Config -config :open_telemetry_decorator, attr_prefix: "" -config :open_telemetry_decorator, attr_joiner: "." +config :open_telemetry_decorator, + attr_prefix: "", + attr_joiner: ".", + expand_all_maps: false # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. diff --git a/lib/open_telemetry_decorator.ex b/lib/open_telemetry_decorator.ex index e48ac51..bff0cf8 100644 --- a/lib/open_telemetry_decorator.ex +++ b/lib/open_telemetry_decorator.ex @@ -14,6 +14,8 @@ defmodule OpenTelemetryDecorator do alias OpenTelemetryDecorator.Attributes alias OpenTelemetryDecorator.Validator + @default_expand_all_maps Application.compile_env(:open_telemetry_decorator, :expand_all_maps) + def trace(span_name, opts \\ [], body, context), do: with_span(span_name, opts, body, context) @doc """ @@ -40,6 +42,7 @@ defmodule OpenTelemetryDecorator do """ def with_span(span_name, opts \\ [], body, context) do include = Keyword.get(opts, :include, []) + expand_all_maps = Keyword.get(opts, :expand_maps, @default_expand_all_maps) Validator.validate_args(span_name, include) quote location: :keep do @@ -51,7 +54,7 @@ defmodule OpenTelemetryDecorator do input_params = Kernel.binding() - |> Attributes.get(unquote(include)) + |> Attributes.get(unquote(include), unquote(expand_all_maps)) |> Keyword.delete(:result) Attributes.set(input_params) diff --git a/lib/open_telemetry_decorator/attributes.ex b/lib/open_telemetry_decorator/attributes.ex index f250a39..ddced8b 100644 --- a/lib/open_telemetry_decorator/attributes.ex +++ b/lib/open_telemetry_decorator/attributes.ex @@ -27,16 +27,32 @@ defmodule OpenTelemetryDecorator.Attributes do set(Tracer.current_span_ctx(), attributes) end - def get(all_attributes, requested_attributes) do + def get(all_attributes, requested_attributes, expand_maps \\ false) do Enum.reduce(requested_attributes, [], fn requested_attribute, taken_attributes -> - case get_attribute(all_attributes, requested_attribute) do + case get_attribute(all_attributes, requested_attribute, expand_maps) do + attributes when is_list(attributes) -> taken_attributes ++ attributes {name, value} -> Keyword.put(taken_attributes, name, value) _ -> taken_attributes end end) end - defp get_attribute(attributes, [attribute_name | nested_keys]) do + defp get_attribute(attributes, attribute_name, true) do + requested_attribute = recursive_get_in(attributes, List.wrap(attribute_name)) + + if is_map(requested_attribute) do + requested_attribute + |> as_map() + |> Map.keys() + |> Enum.map(&get_attribute(attributes, List.wrap(attribute_name) ++ [&1], true)) + |> Enum.reject(&(&1 == nil)) + |> List.flatten() + else + get_attribute(attributes, attribute_name, false) + end + end + + defp get_attribute(attributes, [attribute_name | nested_keys], false) do requested_obj = attributes |> Keyword.get(attribute_name) |> as_map() if value = recursive_get_in(requested_obj, nested_keys) do @@ -44,7 +60,7 @@ defmodule OpenTelemetryDecorator.Attributes do end end - defp get_attribute(attributes, attribute_name) do + defp get_attribute(attributes, attribute_name, false) do if value = Keyword.get(attributes, attribute_name) do {derived_name(attribute_name), to_otlp_value(value)} end diff --git a/test/open_telemetry_decorator/attributes_test.exs b/test/open_telemetry_decorator/attributes_test.exs index d3eb269..28ef16b 100644 --- a/test/open_telemetry_decorator/attributes_test.exs +++ b/test/open_telemetry_decorator/attributes_test.exs @@ -156,12 +156,27 @@ defmodule OpenTelemetryDecorator.AttributesTest do assert Attributes.get([obj: %{id: 1}], [[:obj, :id]]) == [obj_id: 1] end + test "handles expanding map" do + attributes = Attributes.get([obj: %{id: 1, foo: 2}], [:obj], true) + assert {:obj_id, 1} in attributes + assert {:obj_foo, 2} in attributes + + attributes = Attributes.get([obj: %{id: 1, foo: 2, foop: %{id: 3}}, obj_b: %{id: 4}], [:obj, :obj_b], true) + assert {:obj_id, 1} in attributes + assert {:obj_foo, 2} in attributes + assert {:obj_foop_id, 3} in attributes + assert {:obj_b_id, 4} in attributes + end + test "handles nested structs" do one_level = %SomeStruct{beep: "boop"} assert Attributes.get([obj: one_level], [[:obj, :beep]]) == [obj_beep: "boop"] two_levels = %SomeStruct{failed: %SomeStruct{count: 3}} assert Attributes.get([obj: two_levels], [[:obj, :failed, :count]]) == [obj_failed_count: 3] + + # Use expand_all_maps functionality + assert Attributes.get([obj: two_levels], [:obj], true) == [obj_failed_count: 3] end test "handles invalid requests for nested structs" do From 66fa9957e5f6947b96e6caf80eecdb9f8717a23d Mon Sep 17 00:00:00 2001 From: Kevin Tayah Date: Sun, 15 Oct 2023 16:03:00 -0400 Subject: [PATCH 2/6] Add test for with_span w/ expand_map option --- lib/open_telemetry_decorator.ex | 2 +- test/open_telemetry_decorator_test.exs | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/open_telemetry_decorator.ex b/lib/open_telemetry_decorator.ex index bff0cf8..41b7bd3 100644 --- a/lib/open_telemetry_decorator.ex +++ b/lib/open_telemetry_decorator.ex @@ -65,7 +65,7 @@ defmodule OpenTelemetryDecorator do attrs = Kernel.binding() |> Keyword.put(:result, result) - |> Attributes.get(unquote(include)) + |> Attributes.get(unquote(include), unquote(expand_all_maps)) |> Keyword.merge(input_params) |> Enum.map(fn {k, v} -> {Atom.to_string(k), v} end) diff --git a/test/open_telemetry_decorator_test.exs b/test/open_telemetry_decorator_test.exs index 279e292..f6a24fc 100644 --- a/test/open_telemetry_decorator_test.exs +++ b/test/open_telemetry_decorator_test.exs @@ -39,6 +39,20 @@ defmodule OpenTelemetryDecoratorTest do end end + @decorate with_span("Example.find_expand", include: [:user], expand_maps: true) + def find_expand(id) do + _even = rem(id, 2) == 0 + user = %{id: id, name: "my user"} + + case id do + 1 -> + {:ok, user} + + error -> + {:error, error} + end + end + @decorate with_span("Example.parse_params", include: [[:params, "id"]]) def parse_params(params) do %{"id" => id} = params @@ -106,6 +120,12 @@ defmodule OpenTelemetryDecoratorTest do assert %{"user_name" => "my user"} = get_span_attributes(attrs) end + test "handles nested attributes when expand_maps is set" do + Example.find_expand(1) + assert_receive {:span, span(name: "Example.find_expand", attributes: attrs)} + assert %{"user_id" => 1, "user_name" => "my user"} = get_span_attributes(attrs) + end + test "handles maps with string keys" do Example.parse_params(%{"id" => 12}) assert_receive {:span, span(name: "Example.parse_params", attributes: attrs)} From c759d36631bb5154043852bc17dd41bff01bb19a Mon Sep 17 00:00:00 2001 From: Kevin Tayah Date: Sun, 15 Oct 2023 19:41:33 -0400 Subject: [PATCH 3/6] Add documentation --- README.md | 21 +++++++++++++++++++++ config/config.exs | 2 +- lib/open_telemetry_decorator.ex | 8 ++++---- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a50a76d..30d494f 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,13 @@ config :open_telemetry_decorator, attr_joiner: "." ``` Thanks to @benregn for the examples and inspiration for these two options! + +### Changing the default behavior for expanding nested maps +By default, nested attributes need to be explicitly selected in the `with_span/3` macro (See additional examples to see this in action). However if you want the default behavior across your application to always expand variables that are maps/structs, you can specify the following config +```elixir +config :open_telemetry_decorator, expand_maps: true +``` + ### Additional Examples @@ -176,6 +183,20 @@ defmodule MyApp.Worker do end ``` +Grab all nested map/struct properties: + +```elixir +defmodule MyApp.Worker do + use OpenTelemetryDecorator + + @decorate with_span("my_app.worker.do_work", include: [:arg1, :arg2], expand_maps: true) + def do_work(arg1, arg2) do + total = some_calculation(arg1.count, arg2.count) + {:ok, total} + end +end +``` + ```elixir defmodule MyApp.Worker do use OpenTelemetryDecorator diff --git a/config/config.exs b/config/config.exs index 2f4050b..5a9d51a 100644 --- a/config/config.exs +++ b/config/config.exs @@ -10,7 +10,7 @@ import Config config :open_telemetry_decorator, attr_prefix: "", attr_joiner: ".", - expand_all_maps: false + expand_maps: false # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. diff --git a/lib/open_telemetry_decorator.ex b/lib/open_telemetry_decorator.ex index 41b7bd3..5525870 100644 --- a/lib/open_telemetry_decorator.ex +++ b/lib/open_telemetry_decorator.ex @@ -14,7 +14,7 @@ defmodule OpenTelemetryDecorator do alias OpenTelemetryDecorator.Attributes alias OpenTelemetryDecorator.Validator - @default_expand_all_maps Application.compile_env(:open_telemetry_decorator, :expand_all_maps) + @default_expand_maps Application.compile_env(:open_telemetry_decorator, :expand_all_maps) def trace(span_name, opts \\ [], body, context), do: with_span(span_name, opts, body, context) @@ -42,7 +42,7 @@ defmodule OpenTelemetryDecorator do """ def with_span(span_name, opts \\ [], body, context) do include = Keyword.get(opts, :include, []) - expand_all_maps = Keyword.get(opts, :expand_maps, @default_expand_all_maps) + expand_maps = Keyword.get(opts, :expand_maps, @default_expand_maps) Validator.validate_args(span_name, include) quote location: :keep do @@ -54,7 +54,7 @@ defmodule OpenTelemetryDecorator do input_params = Kernel.binding() - |> Attributes.get(unquote(include), unquote(expand_all_maps)) + |> Attributes.get(unquote(include), unquote(expand_maps)) |> Keyword.delete(:result) Attributes.set(input_params) @@ -65,7 +65,7 @@ defmodule OpenTelemetryDecorator do attrs = Kernel.binding() |> Keyword.put(:result, result) - |> Attributes.get(unquote(include), unquote(expand_all_maps)) + |> Attributes.get(unquote(include), unquote(expand_maps)) |> Keyword.merge(input_params) |> Enum.map(fn {k, v} -> {Atom.to_string(k), v} end) From 012c8a240c74e657d470771357b1b08b38bb5545 Mon Sep 17 00:00:00 2001 From: Kevin Tayah Date: Sun, 15 Oct 2023 19:42:56 -0400 Subject: [PATCH 4/6] Fix error introduced --- lib/open_telemetry_decorator.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_telemetry_decorator.ex b/lib/open_telemetry_decorator.ex index 5525870..fd779bd 100644 --- a/lib/open_telemetry_decorator.ex +++ b/lib/open_telemetry_decorator.ex @@ -14,7 +14,7 @@ defmodule OpenTelemetryDecorator do alias OpenTelemetryDecorator.Attributes alias OpenTelemetryDecorator.Validator - @default_expand_maps Application.compile_env(:open_telemetry_decorator, :expand_all_maps) + @default_expand_maps Application.compile_env(:open_telemetry_decorator, :expand_maps) def trace(span_name, opts \\ [], body, context), do: with_span(span_name, opts, body, context) From 299d0f6adf78b4b24cc4d14af1a8d58a9e25b203 Mon Sep 17 00:00:00 2001 From: Kevin Tayah Date: Sun, 15 Oct 2023 19:42:59 -0400 Subject: [PATCH 5/6] Format --- test/open_telemetry_decorator/attributes_test.exs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/open_telemetry_decorator/attributes_test.exs b/test/open_telemetry_decorator/attributes_test.exs index 28ef16b..0a05f82 100644 --- a/test/open_telemetry_decorator/attributes_test.exs +++ b/test/open_telemetry_decorator/attributes_test.exs @@ -161,7 +161,13 @@ defmodule OpenTelemetryDecorator.AttributesTest do assert {:obj_id, 1} in attributes assert {:obj_foo, 2} in attributes - attributes = Attributes.get([obj: %{id: 1, foo: 2, foop: %{id: 3}}, obj_b: %{id: 4}], [:obj, :obj_b], true) + attributes = + Attributes.get( + [obj: %{id: 1, foo: 2, foop: %{id: 3}}, obj_b: %{id: 4}], + [:obj, :obj_b], + true + ) + assert {:obj_id, 1} in attributes assert {:obj_foo, 2} in attributes assert {:obj_foop_id, 3} in attributes From b37f1b1c37e61b5d5adad8743194048b8927874a Mon Sep 17 00:00:00 2001 From: Kevin Tayah Date: Sat, 21 Oct 2023 11:34:58 -0400 Subject: [PATCH 6/6] Added default to expand_maps --- config/config.exs | 7 +++---- lib/open_telemetry_decorator.ex | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/config/config.exs b/config/config.exs index 5a9d51a..9205912 100644 --- a/config/config.exs +++ b/config/config.exs @@ -7,10 +7,9 @@ # General application configuration import Config -config :open_telemetry_decorator, - attr_prefix: "", - attr_joiner: ".", - expand_maps: false +config :open_telemetry_decorator, attr_prefix: "" +config :open_telemetry_decorator, attr_joiner: "." +config :open_telemetry_decorator, expand_maps: false # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. diff --git a/lib/open_telemetry_decorator.ex b/lib/open_telemetry_decorator.ex index fd779bd..019cdf2 100644 --- a/lib/open_telemetry_decorator.ex +++ b/lib/open_telemetry_decorator.ex @@ -14,7 +14,7 @@ defmodule OpenTelemetryDecorator do alias OpenTelemetryDecorator.Attributes alias OpenTelemetryDecorator.Validator - @default_expand_maps Application.compile_env(:open_telemetry_decorator, :expand_maps) + @default_expand_maps Application.compile_env(:open_telemetry_decorator, :expand_maps, false) def trace(span_name, opts \\ [], body, context), do: with_span(span_name, opts, body, context)