Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLAT-8819] Add leaveNetworkRequestBreadcrumbForTask:metrics: #1472

Merged
merged 5 commits into from
Sep 7, 2022

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Aug 24, 2022

Goal

Facilitate leaving network request breadcrumbs without relying on the plugin's Objective-C method swizzling.

Design

+[Bugsnag leaveNetworkRequestBreadcrumbForTask:metrics:] provides a public API for leaving a network request breadcrumb based on an NSURLSessionTask and NSURLSessionTaskMetrics.

Changeset

Adds +[Bugsnag leaveNetworkRequestBreadcrumbForTask:metrics:] and -[BugsnagClient leaveNetworkRequestBreadcrumbForTask:metrics:].

Moves breadcrumb message & metadata computation from BSGURLSessionTracingDelegate to BSGNetworkBreadcrumb.m, called from -[BugsnagClient leaveNetworkRequestBreadcrumbForTask:metrics:].

Calls -[BugsnagClient leaveNetworkRequestBreadcrumbForTask:metrics:] from -[BSGURLSessionTracingDelegate URLSession:task:didFinishCollectingMetrics:].

Testing

Migrates and modifies existing unit tests to continue passing.

Verified by existing E2E scenario.

@github-actions
Copy link

github-actions bot commented Aug 24, 2022

Bugsnag.framework binary size increased by 1,200 bytes from 757,472 to 758,672

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +1.2% +2.70Ki  +1.2% +2.70Ki    __TEXT,__text
  +0.4%    +624  +0.4%    +624    String Table
  +0.4%    +480  +0.4%    +480    Symbol Table
  +1.3%    +340  +1.3%    +340    __TEXT,__objc_methname
  +1.1%    +224  +1.1%    +224    __DATA,__cfstring
  +0.8%    +146  +0.8%    +146    __TEXT,__cstring
  +2.2%    +144  +2.2%    +144    __DATA,__objc_selrefs
  +4.1%     +48  +4.1%     +48    Export Info
  +0.1%     +48  +0.1%     +48    __DATA,__objc_const
  +1.4%     +40  +1.4%     +40    Binding Info
  +0.4%     +12  +0.4%     +12    __TEXT,__unwind_info
  +0.5%      +8  +0.5%      +8    Function Start Addresses
  +1.3%      +8  +1.3%      +8    __DATA,__objc_classrefs
  -3.0%    -424  -9.4%    -424    [__DATA]
  [ = ]       0  -9.5% -1.17Ki    [__LINKEDIT]
 -11.6% -3.19Ki -11.6% -3.19Ki    [__TEXT]
  +0.2% +1.17Ki  [ = ]       0    TOTAL

Generated by 🚫 Danger

@nickdowell nickdowell requested review from kstenerud and removed request for kstenerud September 6, 2022 07:40
@nickdowell nickdowell force-pushed the nickdowell/leave-network-request-breadcrumb branch from 151d16d to 3fb9442 Compare September 7, 2022 07:49
@nickdowell nickdowell merged commit 5cae396 into next Sep 7, 2022
@nickdowell nickdowell deleted the nickdowell/leave-network-request-breadcrumb branch September 7, 2022 08:20
@nickdowell nickdowell mentioned this pull request Sep 14, 2022
@eseay
Copy link

eseay commented Sep 14, 2022

Thank you for doing this! A great addition!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants