Skip to content

Commit

Permalink
Refactor TraceNavigation
Browse files Browse the repository at this point in the history
Based on discussions in #1140 , a decision was made to, instead of using
the URL as the span name for navigation spans, use the word 'navigation'
to identify the action, following the same pattern as the rest of the
spans. Due to this, and because a navigation span will always have that
fixed span name, it is no longer necessary to accept the span name as
input for the method.
  • Loading branch information
ka3de committed Dec 18, 2023
1 parent 5e38a3f commit fde592f
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 8 deletions.
3 changes: 1 addition & 2 deletions common/frame_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,8 +756,7 @@ func (fs *FrameSession) onFrameNavigated(frame *cdp.Frame, initial bool) {
}

_, fs.mainFrameSpan = TraceNavigation(
fs.ctx, fs.targetID.String(), frame.URL,
trace.WithAttributes(attribute.String("navigation.url", frame.URL)),
fs.ctx, fs.targetID.String(), trace.WithAttributes(attribute.String("navigation.url", frame.URL)),
)

var (
Expand Down
6 changes: 3 additions & 3 deletions common/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type Tracer interface {
ctx context.Context, targetID string, spanName string, opts ...trace.SpanStartOption,
) (context.Context, trace.Span)
TraceNavigation(
ctx context.Context, targetID string, url string, opts ...trace.SpanStartOption,
ctx context.Context, targetID string, opts ...trace.SpanStartOption,
) (context.Context, trace.Span)
TraceEvent(
ctx context.Context, targetID string, eventName string, spanID string, opts ...trace.SpanStartOption,
Expand All @@ -37,10 +37,10 @@ func TraceAPICall(
// calls its TraceNavigation implementation. If the Tracer is not present in the given
// ctx, it returns a noopSpan and the given context.
func TraceNavigation(
ctx context.Context, targetID string, url string, opts ...trace.SpanStartOption,
ctx context.Context, targetID string, opts ...trace.SpanStartOption,
) (context.Context, trace.Span) {
if tracer := GetTracer(ctx); tracer != nil {
return tracer.TraceNavigation(ctx, targetID, url, opts...)
return tracer.TraceNavigation(ctx, targetID, opts...)
}
return ctx, browsertrace.NoopSpan{}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestTracing(t *testing.T) {
js: fmt.Sprintf("page.goto('%s')", ts.URL),
spans: []string{
"page.goto",
fmt.Sprintf("%s/", ts.URL), // Navigation span
"navigation",
},
},
{
Expand Down
4 changes: 2 additions & 2 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (t *Tracer) TraceAPICall(
// Posterior calls to TraceEvent or TraceAPICall given the same targetID will try to
// associate new spans for these actions to the liveSpan created in this call.
func (t *Tracer) TraceNavigation(
ctx context.Context, targetID string, url string, opts ...trace.SpanStartOption,
ctx context.Context, targetID string, opts ...trace.SpanStartOption,
) (context.Context, trace.Span) {
t.liveSpansMu.Lock()
defer t.liveSpansMu.Unlock()
Expand All @@ -96,7 +96,7 @@ func (t *Tracer) TraceNavigation(

opts = append(opts, trace.WithAttributes(t.metadata...))

ls.ctx, ls.span = t.Start(ctx, url, opts...)
ls.ctx, ls.span = t.Start(ctx, "navigation", opts...)
t.liveSpans[targetID] = ls

return ls.ctx, ls.span
Expand Down

0 comments on commit fde592f

Please sign in to comment.