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.

Sign up for GitHub

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

Jump to bottom

Capture breadcrumbs for OkHttp network requests #1361

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Capture breadcrumbs for OkHttp network requests #1361

merged 1 commit into from
Sep 13, 2021

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Sep 9, 2021 •
edited
Loading

Goal

Captures breadcrumbs for OkHttp network requests using the BugsnagOkHttpPlugin.

This changeset includes some TODOs relating to tests - these will be addressed in a separate PR for ease of review.

Changeset

BugsnagOkHttpPlugin now uses EventListener callbacks to track information about each request in a ConcurrentHashMap. When a call fails or ends a breadcrumb is recorded using the captured data.

Additional changes include:

  • Copied redactedKeys property onto ImmutableConfig
  • Redacted any sensitive keys in breadcrumb metadata

Testing

  • Added unit tests to verify the behaviour of BugsnagOkHttpPlugin when OkHttp callbacks are invoked
  • Updated integration test to verify that a breadcrumb is captured for successful requests
  • Updated E2E test to make a request to google. This will be completed more fully once mazerunner functionality containing arbitrary endpoints is in place to facilitate testing

@fractalwrench fractalwrench force-pushed the PLAT-7216/capture-network-breadcrumbs branch from 8420637 to f930ef9 Compare September 9, 2021 16:08
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Sep 9, 2021 •
edited
Loading

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1272.13 1159.37
arm64_v8a 405.9 295.31
armeabi -492.59 -21.55
armeabi_v7a 377.23 266.64
x86 446.84 336.25
x86_64 430.47 319.87

Generated by 🚫 Danger

@fractalwrench fractalwrench force-pushed the PLAT-7216/capture-network-breadcrumbs branch from f930ef9 to 265cc5a Compare September 10, 2021 09:18
@fractalwrench fractalwrench marked this pull request as ready for review September 10, 2021 09:35
@fractalwrench fractalwrench force-pushed the PLAT-7216/capture-network-breadcrumbs branch 2 times, most recently from 9004a94 to 0795763 Compare September 10, 2021 09:45
lemnik
lemnik approved these changes Sep 10, 2021
View reviewed changes
Copy link
Contributor

@lemnik lemnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM With some minor comments

bugsnag-plugin-android-okhttp/src/main/java/com/bugsnag/android/okhttp/BugsnagOkHttpPlugin.kt Outdated Show resolved Hide resolved
bugsnag-plugin-android-okhttp/src/main/java/com/bugsnag/android/okhttp/BugsnagOkHttpPlugin.kt Show resolved Hide resolved
@fractalwrench fractalwrench force-pushed the PLAT-7216/capture-network-breadcrumbs branch from 0795763 to a30ba64 Compare September 13, 2021 08:47
@fractalwrench fractalwrench force-pushed the PLAT-7216/capture-network-breadcrumbs branch from a30ba64 to 88d73b1 Compare September 13, 2021 09:18
@fractalwrench fractalwrench merged commit 7387208 into integration/ROAD-740/network-breadcrumbs Sep 13, 2021
@fractalwrench fractalwrench deleted the PLAT-7216/capture-network-breadcrumbs branch September 13, 2021 09:57
This was referenced Sep 22, 2021
Merged
Merged
@imjoehaines imjoehaines mentioned this pull request Sep 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lemnik lemnik lemnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fractalwrench @bugsnagbot @lemnik