Integrate experiments with firebase analytics internal #9278
Conversation
|
athiramanu
changed the title
Size Report 1Affected Products
Test Logs |
athiramanu
force-pushed
the
web-exp-ga
branch
from
f4f7ce2 to
b210bbb
Compare
athiramanu
force-pushed
the
web-exp-ga
branch
from
b210bbb to
66b104b
Compare
Size Analysis Report 1This report is too large (2,214,516 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
athiramanu
requested review from
ashish-kothari,
hsubox76 and
dsjadaun-google
| experimentId: string, | ||
| variantId: string | null | ||
| ): Promise<void> { | ||
| const analytics = await this.analyticsProvider.get(); |
There was a problem hiding this comment.
Are there cases where the provider doesn't return a valid value which we should handle?
| const customProperty = { | ||
| [experimentId]: variantId | ||
| }; | ||
| analytics.setUserProperties({ properties: customProperty }); |
There was a problem hiding this comment.
Does this method allow setting multiple user properties in one shot? I'm wondering if it might be a better idea to instead set all properties in one call given each such call would hit their public endpoint over the network?
Maybe we could carve out setUserPropertiesInAnalytics as a separate method where all properties updates could be bundled?
| experimentId: string | ||
| ): Promise<void> { | ||
| void this.addExperimentToAnalytics(experimentId, null); | ||
| } |
There was a problem hiding this comment.
This function seems redundant to me. If we're reusing addExperimentToAnalytics, then we could directly call it in removeInactiveExperiments instead.
This change is part of the feature to support Web Experiments in Remote Config. The change is split into 3 PRs where:
This is 3/3 mentioned above
Design doc (internal): go/experiments-web