From 2c0149fc3308e0ab323f30280794882465aef604 Mon Sep 17 00:00:00 2001 From: Vince Foley Date: Tue, 24 Dec 2024 11:23:19 -0800 Subject: [PATCH 1/2] Add spansaction for regular DT --- .../phx_example/test/phx_example_test.exs | 13 +++++++ .../apps/test_support/lib/test_support.ex | 7 ++++ lib/new_relic/span/event.ex | 4 +- lib/new_relic/transaction/complete.ex | 39 +++---------------- 4 files changed, 28 insertions(+), 35 deletions(-) diff --git a/examples/apps/phx_example/test/phx_example_test.exs b/examples/apps/phx_example/test/phx_example_test.exs index ceffe09e..f6dd2e9f 100644 --- a/examples/apps/phx_example/test/phx_example_test.exs +++ b/examples/apps/phx_example/test/phx_example_test.exs @@ -53,6 +53,19 @@ defmodule PhxExampleTest do assert event[:status] == 200 end + test "Phoenix spans generated" do + TestSupport.restart_harvest_cycle(Collector.SpanEvent.HarvestCycle) + {:ok, %{body: body}} = request("/phx/home", unquote(server)) + assert body =~ "Some content" + + span_events = TestSupport.gather_harvest(Collector.SpanEvent.Harvester) + + tx_span = TestSupport.find_span(span_events, "/Phoenix/PhxExampleWeb.HomeLive/index") + process_span = TestSupport.find_span(span_events, "Transaction Root Process") + + assert process_span[:parentId] == tx_span[:guid] + end + @tag :capture_log test "Phoenix error" do TestSupport.restart_harvest_cycle(Collector.Metric.HarvestCycle) diff --git a/examples/apps/test_support/lib/test_support.ex b/examples/apps/test_support/lib/test_support.ex index b4dea2be..5dd25370 100644 --- a/examples/apps/test_support/lib/test_support.ex +++ b/examples/apps/test_support/lib/test_support.ex @@ -34,6 +34,13 @@ defmodule TestSupport do end) end + def find_span(spans, name) do + Enum.find_value(spans, fn + [%{name: ^name} = span, _, _] -> span + _span -> false + end) + end + def simulate_agent_enabled(_context) do Process.whereis(Harvest.TaskSupervisor) || NewRelic.EnabledSupervisor.start_link(:ok) diff --git a/lib/new_relic/span/event.ex b/lib/new_relic/span/event.ex index c3bb223d..7e334ca4 100644 --- a/lib/new_relic/span/event.ex +++ b/lib/new_relic/span/event.ex @@ -72,8 +72,8 @@ defmodule NewRelic.Span.Event do component: category[:component] || "component", "span.kind": "client" }) - |> NewRelic.Util.coerce_attributes() |> Map.merge(custom) + |> NewRelic.Util.coerce_attributes() end def merge_category_attributes(%{category: "datastore"} = span, category_attributes) do @@ -89,8 +89,8 @@ defmodule NewRelic.Span.Event do component: category[:component] || "component", "span.kind": "client" }) - |> NewRelic.Util.coerce_attributes() |> Map.merge(custom) + |> NewRelic.Util.coerce_attributes() end def merge_category_attributes(span, category_attributes), diff --git a/lib/new_relic/transaction/complete.ex b/lib/new_relic/transaction/complete.ex index 2f51cd80..43f1a7b8 100644 --- a/lib/new_relic/transaction/complete.ex +++ b/lib/new_relic/transaction/complete.ex @@ -189,7 +189,7 @@ defmodule NewRelic.Transaction.Complete do defp extract_span_events(:sampling, %{sampled: true} = tx_attrs, pid, spawns, exits) do spawned_process_span_events(tx_attrs, spawns, exits) - |> add_root_process_span_event(tx_attrs, pid) + |> add_spansactions(tx_attrs, pid) end defp extract_span_events(_trace_mode, _tx_attrs, _pid, _spawns, _exits) do @@ -208,31 +208,6 @@ defmodule NewRelic.Transaction.Complete do Util.Apdex.calculate(duration_s, apdex_t()) end - defp add_root_process_span_event(spans, tx_attrs, pid) do - [ - %NewRelic.Span.Event{ - trace_id: tx_attrs[:traceId], - transaction_id: tx_attrs[:guid], - sampled: tx_attrs[:sampled], - priority: tx_attrs[:priority], - category: "generic", - name: "Transaction Root Process", - guid: DistributedTrace.generate_guid(pid: pid), - parent_id: tx_attrs[:parentSpanId], - timestamp: tx_attrs[:start_time], - duration: tx_attrs[:duration_s], - entry_point: true, - category_attributes: - %{ - pid: inspect(pid) - } - |> maybe_add(:tracingVendors, tx_attrs[:tracingVendors]) - |> maybe_add(:trustedParentId, tx_attrs[:trustedParentId]) - } - | spans - ] - end - @spansaction_exclude_attrs [ :guid, :traceId, @@ -253,7 +228,7 @@ defmodule NewRelic.Transaction.Complete do trace_id: tx_attrs[:traceId], parent_id: tx_attrs[:parentSpanId], name: tx_attrs[:name], - category: "Transaction", + category: "generic", entry_point: true, timestamp: tx_attrs[:start_time], duration: tx_attrs[:duration_s], @@ -261,8 +236,10 @@ defmodule NewRelic.Transaction.Complete do tx_attrs |> Map.drop(@spansaction_exclude_attrs) |> Map.merge(NewRelic.Config.automatic_attributes()) - |> maybe_add(:tracingVendors, tx_attrs[:tracingVendors]) - |> maybe_add(:trustedParentId, tx_attrs[:trustedParentId]) + |> Map.merge(%{ + tracingVendors: tx_attrs[:tracingVendors], + trustedParentId: tx_attrs[:trustedParentId] + }) }, %NewRelic.Span.Event{ guid: DistributedTrace.generate_guid(pid: pid), @@ -279,10 +256,6 @@ defmodule NewRelic.Transaction.Complete do ] end - def maybe_add(attrs, _key, nil), do: attrs - def maybe_add(attrs, _key, ""), do: attrs - def maybe_add(attrs, key, value), do: Map.put(attrs, key, value) - defp spawned_process_span_events(tx_attrs, process_spawns, process_exits) do process_spawns |> collect_process_segments(process_exits) From f1b5cb0e5c1206c2ccd9a2aa7b93f8a22eba6ecc Mon Sep 17 00:00:00 2001 From: Vince Foley Date: Thu, 26 Dec 2024 17:05:46 -0800 Subject: [PATCH 2/2] Update tests --- lib/new_relic/transaction/complete.ex | 4 ++++ lib/new_relic/util.ex | 3 +++ test/infinite_tracing_test.exs | 2 +- test/instrumented/task_test.exs | 14 +++++++------- test/other_transaction_test.exs | 6 +++--- test/sidecar_test.exs | 4 ++-- test/span_event_test.exs | 12 ++++++++++-- 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/lib/new_relic/transaction/complete.ex b/lib/new_relic/transaction/complete.ex index 43f1a7b8..4cc0eea3 100644 --- a/lib/new_relic/transaction/complete.ex +++ b/lib/new_relic/transaction/complete.ex @@ -228,6 +228,8 @@ defmodule NewRelic.Transaction.Complete do trace_id: tx_attrs[:traceId], parent_id: tx_attrs[:parentSpanId], name: tx_attrs[:name], + sampled: tx_attrs[:sampled], + priority: tx_attrs[:priority], category: "generic", entry_point: true, timestamp: tx_attrs[:start_time], @@ -247,6 +249,8 @@ defmodule NewRelic.Transaction.Complete do trace_id: tx_attrs[:traceId], category: "generic", name: "Transaction Root Process", + sampled: tx_attrs[:sampled], + priority: tx_attrs[:priority], parent_id: tx_attrs[:guid], timestamp: tx_attrs[:start_time], duration: tx_attrs[:duration_s], diff --git a/lib/new_relic/util.ex b/lib/new_relic/util.ex index 138942cf..1f572a73 100644 --- a/lib/new_relic/util.ex +++ b/lib/new_relic/util.ex @@ -72,6 +72,9 @@ defmodule NewRelic.Util do {_key, nil} -> [] + {_key, ""} -> + [] + {key, value} when is_number(value) when is_boolean(value) -> [{key, value}] diff --git a/test/infinite_tracing_test.exs b/test/infinite_tracing_test.exs index 5213655f..f4c3cd91 100644 --- a/test/infinite_tracing_test.exs +++ b/test/infinite_tracing_test.exs @@ -139,7 +139,7 @@ defmodule InfiniteTracingTest do tx_span = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) tx_root_process_span = diff --git a/test/instrumented/task_test.exs b/test/instrumented/task_test.exs index 41fb8814..a0c3d1b8 100644 --- a/test/instrumented/task_test.exs +++ b/test/instrumented/task_test.exs @@ -67,7 +67,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] @@ -108,7 +108,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] @@ -168,7 +168,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] @@ -250,7 +250,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] @@ -322,7 +322,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] @@ -385,7 +385,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] @@ -414,7 +414,7 @@ defmodule InstrumentedTaskTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) refute spansaction.attributes[:not_instrumented] diff --git a/test/other_transaction_test.exs b/test/other_transaction_test.exs index ad3d5d32..64f27ba5 100644 --- a/test/other_transaction_test.exs +++ b/test/other_transaction_test.exs @@ -79,7 +79,7 @@ defmodule OtherTransactionTest do ) span_events = TestHelper.gather_harvest(Collector.SpanEvent.Harvester) - assert length(span_events) == 3 + assert length(span_events) == 4 TestHelper.pause_harvest_cycle(Collector.TransactionEvent.HarvestCycle) TestHelper.pause_harvest_cycle(Collector.TransactionTrace.HarvestCycle) @@ -200,7 +200,7 @@ defmodule OtherTransactionTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" && attr[:name] == "Test/Error" + attr[:"nr.entryPoint"] == true && attr[:name] == "Test/Error" end) assert spansaction.attributes[:error] @@ -241,7 +241,7 @@ defmodule OtherTransactionTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" && attr[:name] == "Test/ExpectedError" + attr[:"nr.entryPoint"] == true && attr[:name] == "Test/ExpectedError" end) refute spansaction.attributes[:error] diff --git a/test/sidecar_test.exs b/test/sidecar_test.exs index 2d198d88..a03d9456 100644 --- a/test/sidecar_test.exs +++ b/test/sidecar_test.exs @@ -264,7 +264,7 @@ defmodule SidecarTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" + attr[:"nr.entryPoint"] == true end) assert spansaction.attributes[:root] == "YES" @@ -352,7 +352,7 @@ defmodule SidecarTest do spansaction = Enum.find(spans, fn %{attributes: attr} -> - attr[:category] == "Transaction" && attr[:name] == "Test/double_connect_test" + attr[:"nr.entryPoint"] == true && attr[:name] == "Test/double_connect_test" end) assert spansaction.attributes[:root] == "YES" diff --git a/test/span_event_test.exs b/test/span_event_test.exs index a09ea8d1..f362ddbb 100644 --- a/test/span_event_test.exs +++ b/test/span_event_test.exs @@ -193,9 +193,12 @@ defmodule SpanEventTest do span_events = TestHelper.gather_harvest(Collector.SpanEvent.Harvester) - [tx_root_process_event, _, _] = + [spansaction_event, _, _] = Enum.find(span_events, fn [ev, _, _] -> ev[:"nr.entryPoint"] == true end) + [tx_root_process_event, _, _] = + Enum.find(span_events, fn [ev, _, _] -> ev[:parentId] == spansaction_event[:guid] end) + [request_process_event, _, _] = Enum.find(span_events, fn [ev, _, _] -> ev[:parentId] == tx_root_process_event[:guid] end) @@ -220,6 +223,7 @@ defmodule SpanEventTest do assert tx_event[:parentId] == "7d3efb1b173fecfa" assert tx_event[:traceId] == "d6b4ba0c3a712ca" + assert spansaction_event[:traceId] == "d6b4ba0c3a712ca" assert tx_root_process_event[:traceId] == "d6b4ba0c3a712ca" assert request_process_event[:traceId] == "d6b4ba0c3a712ca" assert function_event[:traceId] == "d6b4ba0c3a712ca" @@ -227,6 +231,7 @@ defmodule SpanEventTest do assert task_event[:traceId] == "d6b4ba0c3a712ca" assert nested_external_event[:traceId] == "d6b4ba0c3a712ca" + assert spansaction_event[:transactionId] == tx_event[:guid] assert tx_root_process_event[:transactionId] == tx_event[:guid] assert request_process_event[:transactionId] == tx_event[:guid] assert function_event[:transactionId] == tx_event[:guid] @@ -235,6 +240,7 @@ defmodule SpanEventTest do assert nested_external_event[:transactionId] == tx_event[:guid] assert tx_event[:sampled] == true + assert spansaction_event[:sampled] == true assert tx_root_process_event[:sampled] == true assert request_process_event[:sampled] == true assert function_event[:sampled] == true @@ -243,6 +249,7 @@ defmodule SpanEventTest do assert nested_external_event[:sampled] == true assert tx_event[:priority] == 0.987654 + assert spansaction_event[:priority] == 0.987654 assert tx_root_process_event[:priority] == 0.987654 assert request_process_event[:priority] == 0.987654 assert function_event[:priority] == 0.987654 @@ -255,7 +262,8 @@ defmodule SpanEventTest do assert function_event[:"tracer.reductions"] |> is_number assert function_event[:"tracer.reductions"] > 1 - assert tx_root_process_event[:parentId] == "5f474d64b9cc9b2a" + assert spansaction_event[:parentId] == "5f474d64b9cc9b2a" + assert tx_root_process_event[:parentId] == spansaction_event[:guid] assert request_process_event[:parentId] == tx_root_process_event[:guid] assert function_event[:parentId] == request_process_event[:guid] assert nested_function_event[:parentId] == function_event[:guid]