Skip to content

Commit

Permalink
Fix the navigation span's starting point
Browse files Browse the repository at this point in the history
The navigation span was started a lot later than it should have. This
meant that child spans (such as web vitals) looked incorrect since
the values of those child spans were longer than the navigation span.

Now the navigation span is started a lot earlier, and this means that
the child span values are safely within the permitted navigation span.

We still have a navigation span start from onFrameNavigated which is
needed when the iteration starts on a new page, since
onFrameStartedLoading isn't called when a new page is called and it
navigates to about:blank.
  • Loading branch information
ankur22 committed Sep 10, 2024
1 parent 6a5a314 commit 934378e
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions common/frame_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ type FrameSession struct {
// Keep a reference to the main frame span so we can end it
// when FrameSession.ctx is Done
mainFrameSpan trace.Span
// The initial navigation when a new page is created navigates to about:blank.
// We want to make sure that the the navigation span is created for this in
// onFrameNavigated, but subsequent calls to onFrameNavigated in the same
// mainframe never again create a navigation span.
initialNavDone bool
}

// NewFrameSession initializes and returns a new FrameSession.
Expand Down Expand Up @@ -782,6 +787,16 @@ func (fs *FrameSession) onFrameNavigated(frame *cdp.Frame, initial bool) {
frame.URL+frame.URLFragment, err)
}

// If this is true then a new navigation span was already started in
// onFrameStartedLoading.
if fs.initialNavDone {
return
}

fs.initialNavDone = true

// This should only ever be called when a new page is created and it
// navigates to about:blank.
fs.processNavigationSpan(frame.URL, frame.ID)
}

Expand Down Expand Up @@ -850,6 +865,11 @@ func (fs *FrameSession) onFrameStartedLoading(frameID cdp.FrameID) {
"sid:%v tid:%v fid:%v",
fs.session.ID(), fs.targetID, frameID)

frame, ok := fs.manager.getFrameByID(frameID)
if ok {
fs.processNavigationSpan(frame.URL(), frame.id)
}

fs.manager.frameLoadingStarted(frameID)
}

Expand Down

0 comments on commit 934378e

Please sign in to comment.