-
Notifications
You must be signed in to change notification settings - Fork 49
feat(analytics-browser): add "autocapture.networkTracking" plugin #1055
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
feat(analytics-browser): add "autocapture.networkTracking" plugin #1055
Conversation
…/Amplitude-TypeScript into AMP-125616/network-package
…/Amplitude-TypeScript into AMP-125616/network-package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new "autocapture.networkTracking" plugin to support network request tracking in the analytics-browser package.
- Introduces a new networkTracking flag to the AutocaptureOptions interface.
- Implements helper functions (isNetworkTrackingEnabled and getNetworkTrackingConfig) in default-tracking.ts.
- Updates BrowserConfig and browser-client.ts to wire up the network tracking plugin.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/analytics-core/src/types/browser-config.ts | Adds networkTracking flag to autocapture configuration. |
packages/analytics-browser/src/default-tracking.ts | Adds functions to enable and configure network tracking. |
packages/analytics-browser/src/config.ts | Updates BrowserConfig to include networkTrackingOptions. |
packages/analytics-browser/src/browser-client.ts | Integrates the new network tracking plugin based on configuration. |
Files not reviewed (1)
- packages/analytics-browser/package.json: Language not supported
Co-authored-by: Copilot <[email protected]>
- @amplitude/[email protected] - @amplitude/analytics-client-common@2.3.19-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/[email protected] - @amplitude/plugin-autocapture-browser@1.2.1-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/plugin-global-user-properties@1.2.52-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/plugin-network-capture-browser@1.0.1-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/plugin-page-view-tracking-browser@2.3.21-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/plugin-session-replay-browser@1.16.2-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/plugin-web-attribution-browser@2.1.45-usenetworkcaptureinanalyticsbrowser-1.0 - @amplitude/session-replay-browser@1.22.2-usenetworkcaptureinanalyticsbrowser-1.0
This reverts commit be04090.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for network tracking by introducing a new "networkTracking" option to the browser configuration and integrating it in the core tracking and plugin initialization code. Key changes include:
- Adding the "networkTracking" property to the AutocaptureOptions interface.
- Implementing the getNetworkTrackingConfig function and related tests.
- Updating the BrowserConfig and browser-client to register the network tracking plugin.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/analytics-core/src/types/browser-config.ts | Adds the "networkTracking" option in the AutocaptureOptions interface. |
packages/analytics-browser/test/default-tracking.test.ts | Introduces tests for the network tracking configuration. |
packages/analytics-browser/test/browser-client.test.ts | Adds a test to ensure the network tracking plugin is called when enabled. |
packages/analytics-browser/src/default-tracking.ts | Implements getNetworkTrackingConfig and isNetworkTrackingEnabled for network tracking. |
packages/analytics-browser/src/config.ts | Passes networkTrackingOptions into the BrowserConfig constructor. |
packages/analytics-browser/src/browser-client.ts | Adds registration of the network tracking plugin when enabled. |
packages/analytics-browser/README.md | Updates the snippet code, reflecting minor adjustments. |
Files not reviewed (1)
- packages/analytics-browser/package.json: Language not supported
…/Amplitude-TypeScript into AMP-125616/use-network-capture-in-analytics-browser
@@ -154,6 +157,13 @@ export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient { | |||
await this.add(autocapturePlugin(getElementInteractionsConfig(this.config))).promise; | |||
} | |||
|
|||
// TODO: the "this.config.networkTrackingOptions" should be taken out once we have | |||
// beta tested network tracking | |||
if (isNetworkTrackingEnabled(this.config.autocapture) && !!this.config.networkTrackingOptions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is taken out, then we all users that turn auto-capture on will start tracking network requests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is taken out, then all users who have autocapture=true
or autocapture.networkTracking=true
will start tracking network requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will they be able to set autocapture.networkTracking = false
at the minimum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, setting autocapture.networkTracking
to false
will disable it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome thanks for confirming!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one question
ca244f9
into
AMP-125616/network-autocapture
Summary
Checklist