Add manual feature exposure recording (#4120)

This commit is contained in:
Travis Long 2021-05-20 14:02:33 -05:00 коммит произвёл GitHub
Родитель a91e108df4
Коммит 209cc53e4f
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
5 изменённых файлов: 166 добавлений и 24 удалений

Просмотреть файл

@ -79,8 +79,23 @@ interface NimbusInterface {
*/
fun getExperimentBranches(experimentId: String): List<Branch>? = listOf()
/**
* Get the variables needed to configure the feature given by `featureId`.
*
* @param featureId The string feature id that identifies to the feature under experiment.
*
* @param recordExposureEvent Passing `true` to this parameter will record the exposure event
* automatically if the client is enrolled in an experiment for the given [featureId].
* Passing `false` here indicates that the application will manually record the exposure
* event by calling the `recordExposureEvent` function at the time of the exposure to the
* feature.
*
* See [recordExposureEvent] for more information on manually recording the event.
*
* @return a [Variables] object used to configure the feature.
*/
@AnyThread
fun getVariables(featureId: String): Variables = NullVariables.instance
fun getVariables(featureId: String, recordExposureEvent: Boolean = true): Variables = NullVariables.instance
/**
* Open the database and populate the SDK so as make it usable by feature developers.
@ -156,6 +171,34 @@ interface NimbusInterface {
*/
fun resetTelemetryIdentifiers() = Unit
/**
* Records the `exposure` event in telemetry.
*
* This is a manual function to accomplish the same purpose as passing `true` as the
* `recordExposureEvent` property of the [getVariables] function. It is intended to be used
* when requesting feature variables must occur at a different time than the actual user's
* exposure to the feature within the app.
*
* Examples:
* * If the [Variables] are needed at a different time than when the exposure to the feature
* actually happens, such as constructing a menu happening at a different time than the user
* seeing the menu.
* * If [getVariables] is required to be called multiple times for the same feature and it is
* desired to only record the exposure once, such as if [getVariables] were called with every
* keystroke.
*
* In the case where the use of this function is required, then the [getVariables] function
* should be called with `false` so that the exposure event is not recorded when the variables
* are fetched.
*
* This function is safe to call even when there is no active experiment for the feature. The SDK
* will ensure that an event is only recorded for active experiments.
*
* @param featureId string representing the id of the feature for which to record the exposure
* event.
*/
fun recordExposureEvent(featureId: String) = Unit
/**
* Control the opt out for all experiments at once. This is likely a user action.
*/
@ -319,12 +362,14 @@ open class Nimbus(
}
override fun getExperimentBranch(experimentId: String): String? {
recordExposure(experimentId)
return nimbusClient.getExperimentBranch(experimentId)
}
override fun getVariables(featureId: String): Variables =
override fun getVariables(featureId: String, recordExposureEvent: Boolean): Variables =
getFeatureConfigVariablesJson(featureId)?.let { json ->
if (recordExposureEvent) {
recordExposure(featureId)
}
JSONVariables(context, json)
}
?: NullVariables.instance
@ -470,6 +515,10 @@ open class Nimbus(
}
}
override fun recordExposureEvent(featureId: String) {
recordExposure(featureId)
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun recordExperimentTelemetry(experiments: List<EnrolledExperiment>) {
// Call Glean.setExperimentActive() for each active experiment.
@ -510,9 +559,9 @@ open class Nimbus(
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun recordExposure(experimentId: String) {
internal fun recordExposure(featureId: String) {
dbScope.launch {
recordExposureOnThisThread(experimentId)
recordExposureOnThisThread(featureId)
}
}
@ -520,9 +569,9 @@ open class Nimbus(
// for a "control" branch) is applied or shown to the user.
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
@WorkerThread
internal fun recordExposureOnThisThread(experimentId: String) = withCatchAll {
internal fun recordExposureOnThisThread(featureId: String) = withCatchAll {
val activeExperiments = getActiveExperiments()
activeExperiments.find { it.slug == experimentId }?.also { experiment ->
activeExperiments.find { it.featureIds.contains(featureId) }?.also { experiment ->
NimbusEvents.exposure.record(mapOf(
NimbusEvents.exposureKeys.experiment to experiment.slug,
NimbusEvents.exposureKeys.branch to experiment.branchSlug,

Просмотреть файл

@ -16,6 +16,7 @@ import mozilla.components.service.glean.config.Configuration
import mozilla.components.service.glean.net.ConceptFetchHttpUploader
import mozilla.components.service.glean.testing.GleanTestRule
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
@ -166,10 +167,13 @@ class NimbusTest {
// because recordExposure checks for active experiments before recording.
nimbus.setUpTestExperiments(packageName, appInfo)
// Record the exposure event in Glean
nimbus.recordExposureOnThisThread("test-experiment")
// Assert that there are no events to start with
assertFalse("There must not be any pre-existing events", NimbusEvents.exposure.testHasValue())
// Use the Glean test API to check the recorded event
// Record a valid exposure event in Glean that matches the featureId from the test experiment
nimbus.recordExposureOnThisThread("about_welcome")
// Use the Glean test API to check that the valid event is present
assertTrue("Event must have a value", NimbusEvents.exposure.testHasValue())
val enrollmentEvents = NimbusEvents.exposure.testGetValue()
assertEquals("Event count must match", enrollmentEvents.count(), 1)
@ -177,6 +181,20 @@ class NimbusTest {
assertEquals("Experiment slug must match", "test-experiment", enrollmentEventExtras["experiment"])
assertEquals("Experiment branch must match", "test-branch", enrollmentEventExtras["branch"])
assertNotNull("Experiment enrollment-id must not be null", enrollmentEventExtras["enrollment_id"])
// Attempt to record an event for a non-existent or feature we are not enrolled in an
// experiment in to ensure nothing is recorded.
nimbus.recordExposureOnThisThread("not-a-feature")
// Verify the invalid event was ignored by checking again that the valid event is still the only
// event, and that it hasn't changed any of its extra properties.
assertTrue("Event must have a value", NimbusEvents.exposure.testHasValue())
val enrollmentEventsTryTwo = NimbusEvents.exposure.testGetValue()
assertEquals("Event count must match", enrollmentEventsTryTwo.count(), 1)
val enrollmentEventExtrasTryTwo = enrollmentEventsTryTwo.first().extra!!
assertEquals("Experiment slug must match", "test-experiment", enrollmentEventExtrasTryTwo["experiment"])
assertEquals("Experiment branch must match", "test-branch", enrollmentEventExtrasTryTwo["branch"])
assertNotNull("Experiment enrollment-id must not be null", enrollmentEventExtrasTryTwo["enrollment_id"])
}
private fun Nimbus.setUpTestExperiments(appId: String, appInfo: NimbusAppInfo) {
@ -234,7 +252,7 @@ class NimbusTest {
}
@Test
fun `Smoke test receiving JSON features`() {
fun `Smoke test receiving JSON features`() {
nimbus.setUpTestExperiments(packageName, appInfo)
// The test experiment has exactly one branch with 100% enrollment
// We should be able to get feature variables for the feature in this

Просмотреть файл

@ -47,10 +47,18 @@ private extension Nimbus {
}
// Glean integration
internal extension Nimbus {
func recordExposure(experimentId: String) {
extension Nimbus: NimbusTelemetryConfiguration {
public func recordExposureEvent(featureId: String) {
// First we need a list of the active experiments that are enrolled.
let activeExperiments = getActiveExperiments()
if let experiment = activeExperiments.first(where: { $0.slug == experimentId }) {
// Next, we search for any experiment that has a matching featureId. This depends on the
// fact that we can only be enrolled in a single experiment per feature, so there should
// only ever be zero or one experiments for a given featureId.
if let experiment = activeExperiments.first(where: { $0.featureIds.contains(featureId) }) {
// Finally, if we do have an experiment for the given featureId, we will record the
// exposure event in Glean. This is to protect against accidentally recording an event
// for an experiment without an active enrollment.
GleanMetrics.NimbusEvents.exposure.record(extra: [
.experiment: experiment.slug,
.branch: experiment.branchSlug,
@ -59,7 +67,7 @@ internal extension Nimbus {
}
}
func postEnrollmentCalculation(_ events: [EnrollmentChangeEvent]) {
internal func postEnrollmentCalculation(_ events: [EnrollmentChangeEvent]) {
// We need to update the experiment enrollment annotations in Glean
// regardless of whether we recieved any events. Calling the
// `setExperimentActive` function multiple times with the same
@ -77,7 +85,7 @@ internal extension Nimbus {
}
}
func recordExperimentTelemetry(_ experiments: [EnrolledExperiment]) {
internal func recordExperimentTelemetry(_ experiments: [EnrolledExperiment]) {
for experiment in experiments {
Glean.shared.setExperimentActive(
experimentId: experiment.slug,
@ -87,7 +95,7 @@ internal extension Nimbus {
}
}
func recordExperimentEvents(_ events: [EnrollmentChangeEvent]) {
internal func recordExperimentEvents(_ events: [EnrollmentChangeEvent]) {
for event in events {
switch event.change {
case .enrollment:
@ -184,10 +192,15 @@ extension Nimbus: NimbusFeatureConfiguration {
}
}
public func getVariables(featureId: String) -> Variables {
public func getVariables(featureId: String, recordExposureEvent: Bool = true) -> Variables {
guard let json = getFeatureConfigVariablesJson(featureId: featureId) else {
return NilVariables.instance
}
if recordExposureEvent {
self.recordExposureEvent(featureId: featureId)
}
return JSONVariables(with: json)
}
}
@ -296,7 +309,7 @@ public extension NimbusDisabled {
return nil
}
func getVariables(featureId _: String) -> Variables {
func getVariables(featureId _: String, recordExposureEvent _: Bool) -> Variables {
return NilVariables.instance
}
@ -316,6 +329,8 @@ public extension NimbusDisabled {
func resetTelemetryIdentifiers() {}
func recordExposureEvent(featureId _: String) {}
func getExperimentBranches(_: String) -> [Branch]? {
return nil
}

Просмотреть файл

@ -26,9 +26,46 @@ public protocol NimbusFeatureConfiguration {
/// Get the variables needed to configure the feature given by `featureId`.
///
/// - Parameter featureId The string feature id that identifies to the feature under experiment.
/// - Parameters:
/// - featureId The string feature id that identifies to the feature under experiment.
/// - recordExposureEvent Passing `true` to this parameter will record the exposure
/// event automatically if the client is enrolled in an experiment for the given `featureId`.
/// Passing `false` here indicates that the application will manually record the exposure
/// event by calling `recordExposureEvent`.
///
/// See `recordExposureEvent` for more information on manually recording the event.
///
/// - Returns a `Variables` object used to configure the feature.
func getVariables(featureId: String) -> Variables
func getVariables(featureId: String, recordExposureEvent: Bool) -> Variables
}
public protocol NimbusTelemetryConfiguration {
/// Records the `exposure` event in telemetry.
///
/// This is a manual function to accomplish the same purpose as passing `true` as the
/// `recordExposureEvent` property of the `getVariables` function. It is intended to be used
/// when requesting feature variables must occur at a different time than the actual user's
/// exposure to the feature within the app.
///
/// - Examples:
/// - If the `Variables` are needed at a different time than when the exposure to the feature
/// actually happens, such as constructing a menu happening at a different time than the
/// user seeing the menu.
/// - If `getVariables` is required to be called multiple times for the same feature and it is
/// desired to only record the exposure once, such as if `getVariables` were called
/// with every keystroke.
///
/// In the case where the use of this function is required, then the `getVariables` function
/// should be called with `false` so that the exposure event is not recorded when the variables
/// are fetched.
///
/// This function is safe to call even when there is no active experiment for the feature. The SDK
/// will ensure that an event is only recorded for active experiments.
///
/// - Parameter featureId string representing the id of the feature for which to record the exposure
/// event.
///
func recordExposureEvent(featureId: String)
}
public protocol NimbusStartup {

Просмотреть файл

@ -267,9 +267,15 @@ class NimbusTests: XCTestCase {
"branches": [
{
"slug": "test-branch",
"ratio": 1
"ratio": 1,
"feature": {
"featureId": "test-feature",
"enabled": true,
"value": {}
}
}
],
"featureIds": ["test-feature"],
"probeSets": [],
"startDate": null,
"appName": "NimbusUnitTest",
@ -293,9 +299,13 @@ class NimbusTests: XCTestCase {
""")
try nimbus.applyPendingExperimentsOnThisThread()
// Record the exposure event in Glean
nimbus.recordExposure(experimentId: "test-experiment")
// Assert that there are no events to start with
XCTAssertFalse(GleanMetrics.NimbusEvents.exposure.testHasValue(), "Event must have a value")
// Record a valid exposure event in Glean that matches the featureId from the test experiment
nimbus.recordExposureEvent(featureId: "test-feature")
// Use the Glean test API to check that the valid event is present
XCTAssertTrue(GleanMetrics.NimbusEvents.exposure.testHasValue(), "Event must have a value")
let enrollmentEvents = try GleanMetrics.NimbusEvents.exposure.testGetValue()
XCTAssertEqual(1, enrollmentEvents.count, "Event count must match")
@ -303,6 +313,19 @@ class NimbusTests: XCTestCase {
XCTAssertEqual("test-experiment", enrollmentEventExtras!["experiment"], "Experiment slug must match")
XCTAssertEqual("test-branch", enrollmentEventExtras!["branch"], "Experiment branch must match")
XCTAssertNotNil(enrollmentEventExtras!["enrollment_id"], "Experiment enrollment id must not be nil")
// Attempt to record an event for a non-existent or feature we are not enrolled in an
// experiment in to ensure nothing is recorded.
nimbus.recordExposureEvent(featureId: "not-a-feature")
// Verify the invalid event was ignored by checking again that the valid event is still the only
// event, and that it hasn't changed any of its extra properties.
let enrollmentEventsTryTwo = try GleanMetrics.NimbusEvents.exposure.testGetValue()
XCTAssertEqual(1, enrollmentEventsTryTwo.count, "Event count must match")
let enrollmentEventExtrasTryTwo = enrollmentEventsTryTwo.first!.extra
XCTAssertEqual("test-experiment", enrollmentEventExtrasTryTwo!["experiment"], "Experiment slug must match")
XCTAssertEqual("test-branch", enrollmentEventExtrasTryTwo!["branch"], "Experiment branch must match")
XCTAssertNotNil(enrollmentEventExtrasTryTwo!["enrollment_id"], "Experiment enrollment id must not be nil")
}
}