Skip to content

Commit bcae95d

Browse files
committed
chore(review): address PR review feedback
- Simplify transaction_root? assignment by removing intermediate variable - Rename is_server_span?, is_http_server_span?, is_liveview_span? to remove is_ prefix per Elixir convention - Replace map_keys helper functions with direct &Map.keys/1 calls
1 parent 4f35d86 commit bcae95d

3 files changed

Lines changed: 16 additions & 25 deletions

File tree

lib/sentry/opentelemetry/live_view_propagator.ex

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() and
114114
Sentry.OpenTelemetry.Propagator.extract(
115115
:otel_ctx.get_current(),
116116
carrier,
117-
&map_keys/1,
117+
&Map.keys/1,
118118
&map_getter/2,
119119
[]
120120
)
@@ -144,8 +144,6 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() and
144144
defp has_valid_span?(span_ctx(trace_id: trace_id)) when trace_id != 0, do: true
145145
defp has_valid_span?(_), do: false
146146

147-
defp map_keys(carrier), do: Map.keys(carrier)
148-
149147
defp map_getter(key, carrier) do
150148
case Map.fetch(carrier, key) do
151149
{:ok, value} -> value

lib/sentry/opentelemetry/span_processor.ex

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,23 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do
3737
end
3838

3939
defp process_span(span_record) do
40-
is_transaction_root =
40+
transaction_root? =
4141
cond do
4242
# No parent = definitely a root
4343
span_record.parent_span_id == nil ->
4444
true
4545

4646
# Has a parent - check if it's local or remote
47+
has_local_parent_span?(span_record.parent_span_id) ->
48+
# Parent exists locally - this is a child span, not a transaction root
49+
false
50+
4751
true ->
48-
has_local_parent = has_local_parent_span?(span_record.parent_span_id)
49-
50-
if has_local_parent do
51-
# Parent exists locally - this is a child span, not a transaction root
52-
false
53-
else
54-
# Parent is remote (distributed tracing) - treat server spans as transaction roots
55-
is_server_span?(span_record)
56-
end
52+
# Parent is remote (distributed tracing) - treat server spans as transaction roots
53+
server_span?(span_record)
5754
end
5855

59-
if is_transaction_root do
56+
if transaction_root? do
6057
build_and_send_transaction(span_record)
6158
else
6259
true
@@ -68,19 +65,19 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do
6865
end
6966

7067
# Check if it's an HTTP server request span or a LiveView span
71-
defp is_server_span?(%{kind: :server} = span_record) do
72-
is_http_server_span?(span_record) or is_liveview_span?(span_record)
68+
defp server_span?(%{kind: :server} = span_record) do
69+
http_server_span?(span_record) or liveview_span?(span_record)
7370
end
7471

75-
defp is_server_span?(_), do: false
72+
defp server_span?(_), do: false
7673

77-
defp is_http_server_span?(%{kind: :server, attributes: attributes}) do
74+
defp http_server_span?(%{kind: :server, attributes: attributes}) do
7875
Map.has_key?(attributes, to_string(HTTPAttributes.http_request_method()))
7976
end
8077

8178
# Check if span name matches LiveView lifecycle patterns
82-
defp is_liveview_span?(%{origin: "opentelemetry_phoenix"}), do: true
83-
defp is_liveview_span?(_), do: false
79+
defp liveview_span?(%{origin: "opentelemetry_phoenix"}), do: true
80+
defp liveview_span?(_), do: false
8481

8582
defp build_and_send_transaction(span_record) do
8683
child_span_records = SpanStorage.get_child_spans(span_record.span_id)

lib/sentry/plug/live_view_context.ex

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() and
136136
Sentry.OpenTelemetry.Propagator.extract(
137137
ctx,
138138
carrier,
139-
&map_keys/1,
139+
&Map.keys/1,
140140
&map_getter/2,
141141
[]
142142
)
@@ -187,10 +187,6 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() and
187187
end
188188
end
189189

190-
defp map_keys(carrier) do
191-
Map.keys(carrier)
192-
end
193-
194190
defp map_getter(key, carrier) do
195191
case Map.fetch(carrier, key) do
196192
{:ok, value} -> value

0 commit comments

Comments
 (0)