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

[SDK-416] Fix missing disqualification event after global opt-out #4606

Merged
merged 2 commits into from Oct 26, 2021
Merged

[SDK-416] Fix missing disqualification event after global opt-out #4606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cc0ae4
[SDK-416] Fix missing disqualification event after global opt-out
travis79 Oct 25, 2021
4131c55
Add tests for disqualification events to Swift
travis79 Oct 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt
View file
Open in desktop
Expand Up @@ -490,18 +490,25 @@ open class Nimbus(
internal fun setGlobalUserParticipationOnThisThread(active: Boolean) = withCatchAll {
val enrolmentChanges = nimbusClient.setGlobalUserParticipation(active)
if (enrolmentChanges.isNotEmpty()) {
recordExperimentTelemetryEvents(enrolmentChanges)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok.

postEnrolmentCalculation()
}
}

override fun optOut(experimentId: String) {
dbScope.launch {
withCatchAll {
nimbusClient.optOut(experimentId).also(::recordExperimentTelemetryEvents)
optOutOnThisThread(experimentId)
}
}
}

@WorkerThread
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
internal fun optOutOnThisThread(experimentId: String) {
nimbusClient.optOut(experimentId).also(::recordExperimentTelemetryEvents)
}

override fun resetTelemetryIdentifiers() {
// The "dummy" field here is required for obscure reasons when generating code on desktop,
// so we just automatically set it to a dummy value.
Expand Down
50 changes: 50 additions & 0 deletions components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTest.kt
View file
Open in desktop
Expand Up @@ -201,6 +201,56 @@ class NimbusTest {
assertNotNull("Experiment enrollment-id must not be null", enrollmentEventExtrasTryTwo["enrollment_id"])
}

@Test
fun `opting out generates the correct Glean event`() {
// Load the experiment in nimbus so and optIn so that it will be active. This is necessary
// because recordExposure checks for active experiments before recording.
nimbus.setUpTestExperiments(packageName, appInfo)

// Assert that there are no events to start with
assertFalse(
"There must not be any pre-existing events",
NimbusEvents.disqualification.testHasValue()
)

// Opt out of the specific experiment
nimbus.optOutOnThisThread("test-experiment")

// Use the Glean test API to check that the valid event is present
assertTrue("Event must have a value", NimbusEvents.disqualification.testHasValue())
val disqualificationEvents = NimbusEvents.disqualification.testGetValue()
assertEquals("Event count must match", disqualificationEvents.count(), 1)
val enrollmentEventExtras = disqualificationEvents.first().extra!!
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"])
}

@Test
fun `toggling the global opt out generates the correct Glean event`() {
// Load the experiment in nimbus so and optIn so that it will be active. This is necessary
// because recordExposure checks for active experiments before recording.
nimbus.setUpTestExperiments(packageName, appInfo)

// Assert that there are no events to start with
assertFalse(
"There must not be any pre-existing events",
NimbusEvents.disqualification.testHasValue()
)

// Opt out of all experiments
nimbus.setGlobalUserParticipationOnThisThread(false)

// Use the Glean test API to check that the valid event is present
assertTrue("Event must have a value", NimbusEvents.disqualification.testHasValue())
val disqualificationEvents = NimbusEvents.disqualification.testGetValue()
assertEquals("Event count must match", disqualificationEvents.count(), 1)
val enrollmentEventExtras = disqualificationEvents.first().extra!!
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"])
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

private fun Nimbus.setUpTestExperiments(appId: String, appInfo: NimbusAppInfo) {
this.setExperimentsLocallyOnThisThread("""
{"data": [{
Expand Down
114 changes: 71 additions & 43 deletions megazords/ios/MozillaAppServicesTests/NimbusTests.swift
View file
Open in desktop
Expand Up @@ -259,59 +259,25 @@ class NimbusTests: XCTestCase {
// Load an experiment in nimbus that we will record an event in. The experiment bucket configuration
// is set so that it will be guaranteed to be active. This is necessary because the SDK checks for
// active experiments before recording.
try nimbus.setExperimentsLocallyOnThisThread("""
{"data": [{
"schemaVersion": "1.0.0",
"slug": "test-experiment",
"endDate": null,
"branches": [
{
"slug": "test-branch",
"ratio": 1,
"feature": {
"featureId": "test-feature",
"enabled": true,
"value": {}
}
}
],
"featureIds": ["test-feature"],
"probeSets": [],
"startDate": null,
"appName": "NimbusUnitTest",
"appId": "\(xcTestAppId())",
"channel": "test",
"bucketConfig": {
"count": 10000,
"start": 0,
"total": 10000,
"namespace": "test-experiment",
"randomizationUnit": "nimbus_id"
},
"userFacingName": "Diagnostic test experiment",
"referenceBranch": "test-branch",
"isEnrollmentPaused": false,
"proposedEnrollment": 7,
"userFacingDescription": "This is a test experiment for diagnostic purposes.",
"id": "test-experiment",
"last_modified": 1602197324372
}]}
""")
try nimbus.setExperimentsLocallyOnThisThread(minimalExperimentJSON())
try nimbus.applyPendingExperimentsOnThisThread()

// 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")
nimbus.recordExposureEvent(featureId: "aboutwelcome")

// 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")
let enrollmentEventExtras = enrollmentEvents.first!.extra
XCTAssertEqual("test-experiment", enrollmentEventExtras!["experiment"], "Experiment slug must match")
XCTAssertEqual("test-branch", enrollmentEventExtras!["branch"], "Experiment branch must match")
XCTAssertEqual("secure-gold", enrollmentEventExtras!["experiment"], "Experiment slug must match")
XCTAssertTrue(
enrollmentEventExtras!["branch"] == "control" || enrollmentEventExtras!["branch"] == "treatment",
"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
Expand All @@ -323,10 +289,72 @@ class NimbusTests: XCTestCase {
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")
XCTAssertEqual("secure-gold", enrollmentEventExtrasTryTwo!["experiment"], "Experiment slug must match")
XCTAssertTrue(
enrollmentEventExtrasTryTwo!["branch"] == "control" || enrollmentEventExtrasTryTwo!["branch"] == "treatment",
"Experiment branch must match"
)
XCTAssertNotNil(enrollmentEventExtrasTryTwo!["enrollment_id"], "Experiment enrollment id must not be nil")
}

func testRecordDisqualificationOnOptOut() throws {
let appSettings = NimbusAppSettings(appName: "NimbusUnitTest", channel: "test")
let nimbus = try Nimbus.create(nil, appSettings: appSettings, dbPath: createDatabasePath()) as! Nimbus

// Load an experiment in nimbus that we will record an event in. The experiment bucket configuration
// is set so that it will be guaranteed to be active. This is necessary because the SDK checks for
// active experiments before recording.
try nimbus.setExperimentsLocallyOnThisThread(minimalExperimentJSON())
try nimbus.applyPendingExperimentsOnThisThread()

// Assert that there are no events to start with
XCTAssertFalse(GleanMetrics.NimbusEvents.exposure.testHasValue(), "Event must have a value")

// Opt out of the experiment, which should generate a "disqualification" event
try nimbus.optOutOnThisThread("secure-gold")

// Use the Glean test API to check that the valid event is present
XCTAssertTrue(GleanMetrics.NimbusEvents.disqualification.testHasValue(), "Event must have a value")
let disqualificationEvents = try GleanMetrics.NimbusEvents.disqualification.testGetValue()
XCTAssertEqual(1, disqualificationEvents.count, "Event count must match")
let disqualificationEventExtras = disqualificationEvents.first!.extra
XCTAssertEqual("secure-gold", disqualificationEventExtras!["experiment"], "Experiment slug must match")
XCTAssertTrue(
disqualificationEventExtras!["branch"] == "control" || disqualificationEventExtras!["branch"] == "treatment",
"Experiment branch must match"
)
XCTAssertNotNil(disqualificationEventExtras!["enrollment_id"], "Experiment enrollment id must not be nil")
}

func testRecordDisqualificationOnGlobalOptOut() throws {
let appSettings = NimbusAppSettings(appName: "NimbusUnitTest", channel: "test")
let nimbus = try Nimbus.create(nil, appSettings: appSettings, dbPath: createDatabasePath()) as! Nimbus

// Load an experiment in nimbus that we will record an event in. The experiment bucket configuration
// is set so that it will be guaranteed to be active. This is necessary because the SDK checks for
// active experiments before recording.
try nimbus.setExperimentsLocallyOnThisThread(minimalExperimentJSON())
try nimbus.applyPendingExperimentsOnThisThread()

// Assert that there are no events to start with
XCTAssertFalse(GleanMetrics.NimbusEvents.exposure.testHasValue(), "Event must have a value")

// Opt out of all experiments, which should generate a "disqualification" event for the enrolled
// experiment
try nimbus.setGlobalUserParticipationOnThisThread(false)

// Use the Glean test API to check that the valid event is present
XCTAssertTrue(GleanMetrics.NimbusEvents.disqualification.testHasValue(), "Event must have a value")
let disqualificationEvents = try GleanMetrics.NimbusEvents.disqualification.testGetValue()
XCTAssertEqual(1, disqualificationEvents.count, "Event count must match")
let disqualificationEventExtras = disqualificationEvents.first!.extra
XCTAssertEqual("secure-gold", disqualificationEventExtras!["experiment"], "Experiment slug must match")
XCTAssertTrue(
disqualificationEventExtras!["branch"] == "control" || disqualificationEventExtras!["branch"] == "treatment",
"Experiment branch must match"
)
XCTAssertNotNil(disqualificationEventExtras!["enrollment_id"], "Experiment enrollment id must not be nil")
}
}

extension Device {
Expand Down