Skip to content

feat(flag_decisions): update decisionSnapshot in event-processor to include metadata object #601

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

Merged
merged 3 commits into from
Oct 16, 2020
Merged

feat(flag_decisions): update decisionSnapshot in event-processor to include metadata object #601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7fc08a2
Include metadata object to makeDecisionSnapshot
yavorona Oct 16, 2020
9de8d40
Update CHANGELOG
yavorona Oct 16, 2020
f165098
Fix specs
yavorona Oct 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/event-processor/CHANGELOG.MD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]
Changes that have landed but are not yet released.
- Update `Visitor.Snapshot` to include metadata object to support sending flag decisions.

## [0.6.0] - July 28, 2020

Expand Down
119 changes: 118 additions & 1 deletion packages/event-processor/__tests__/buildEventV1.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { ImpressionEvent, ConversionEvent } from '../src/events'

describe('buildEventV1', () => {
describe('buildImpressionEventV1', () => {
it('should build an build an ImpressionEventV1', () => {
it('should build an ImpressionEventV1 when experiment and variation are defined', () => {
const impressionEvent: ImpressionEvent = {
type: 'impression',
timestamp: 69,
Expand Down Expand Up @@ -58,6 +58,10 @@ describe('buildEventV1', () => {
id: 'varId',
key: 'varKey',
},

ruleKey: 'expKey',
flagKey: 'flagKey1',
ruleType: 'experiment',
}

const result = buildImpressionEventV1(impressionEvent)
Expand All @@ -79,6 +83,12 @@ describe('buildEventV1', () => {
campaign_id: 'layerId',
experiment_id: 'expId',
variation_id: 'varId',
metadata: {
flag_key: 'flagKey1',
rule_key: 'expKey',
rule_type: 'experiment',
variation_key: 'varKey',
},
},
],
events: [
Expand Down Expand Up @@ -110,6 +120,103 @@ describe('buildEventV1', () => {
],
})
})

it('should build an ImpressionEventV1 when experiment and variation are not defined', () => {
const impressionEvent: ImpressionEvent = {
type: 'impression',
timestamp: 69,
uuid: 'uuid',

context: {
accountId: 'accountId',
projectId: 'projectId',
clientName: 'node-sdk',
clientVersion: '3.0.0',
revision: 'revision',
botFiltering: true,
anonymizeIP: true,
},

user: {
id: 'userId',
attributes: [{ entityId: 'attr1-id', key: 'attr1-key', value: 'attr1-value' }],
},

layer: {
id: null,
},

experiment: {
id: null,
key: '',
},

variation: {
id: null,
key: '',
},

ruleKey: '',
flagKey: 'flagKey1',
ruleType: 'rollout',
}

const result = buildImpressionEventV1(impressionEvent)
expect(result).toEqual({
client_name: 'node-sdk',
client_version: '3.0.0',
account_id: 'accountId',
project_id: 'projectId',
revision: 'revision',
anonymize_ip: true,
enrich_decisions: true,

visitors: [
{
snapshots: [
{
decisions: [
{
campaign_id: null,
experiment_id: null,
variation_id: null,
metadata: {
flag_key: 'flagKey1',
rule_key: '',
rule_type: 'rollout',
variation_key: '',
},
},
],
events: [
{
entity_id: null,
timestamp: 69,
key: 'campaign_activated',
uuid: 'uuid',
},
],
},
],
visitor_id: 'userId',
attributes: [
{
entity_id: 'attr1-id',
key: 'attr1-key',
type: 'custom',
value: 'attr1-value',
},
{
entity_id: '$opt_bot_filtering',
key: '$opt_bot_filtering',
type: 'custom',
value: true,
},
],
},
],
})
})
})

describe('buildConversionEventV1', () => {
Expand Down Expand Up @@ -438,6 +545,10 @@ describe('buildEventV1', () => {
id: 'varId',
key: 'varKey',
},

ruleKey: 'expKey',
flagKey: 'flagKey1',
ruleType: 'experiment',
}

const result = makeBatchedEventV1([impressionEvent, conversionEvent])
Expand All @@ -460,6 +571,12 @@ describe('buildEventV1', () => {
campaign_id: 'layerId',
experiment_id: 'expId',
variation_id: 'varId',
metadata: {
flag_key: 'flagKey1',
rule_key: 'expKey',
rule_type: 'experiment',
variation_key: 'varKey',
},
},
],
events: [
Expand Down
4 changes: 4 additions & 0 deletions packages/event-processor/__tests__/v1EventProcessor.react_native.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ function createImpressionEvent() {
id: 'varId',
key: 'varKey',
},

ruleKey: 'expKey',
flagKey: 'flagKey1',
ruleType: 'experiment',
}
}

Expand Down
4 changes: 4 additions & 0 deletions packages/event-processor/__tests__/v1EventProcessor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ function createImpressionEvent() {
id: 'varId',
key: 'varKey',
},

ruleKey: 'expKey',
flagKey: 'flagKey1',
ruleType: 'experiment',
}
}

Expand Down
10 changes: 7 additions & 3 deletions packages/event-processor/src/events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,22 @@ export interface ImpressionEvent extends BaseEvent {
}

layer: {
id: string
id: string | null
} | null

experiment: {
id: string
id: string | null
key: string
} | null

variation: {
id: string
id: string | null
Comment on lines +48 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these now allowed to be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When decision.experiment is not defined, we assign a ruleKey/experimentKey to an empty string, in which case I assign an experimentId and layerId to null. Similarly, the variationId is now null when variation is not defined. Please see the implementation here for details. Another way would be to assign these params to an empty string instead of null. LMKWYT @mjc1283

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, I think using null is ok.

key: string
} | null

ruleKey: string
flagKey: string
ruleType: string
}

export interface ConversionEvent extends BaseEvent {
Expand Down
17 changes: 16 additions & 1 deletion packages/event-processor/src/v1/buildEventV1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ namespace Visitor {
campaign_id: string | null
experiment_id: string | null
variation_id: string | null
metadata: Metadata
}

type Metadata = {
flag_key: string;
rule_key: string;
rule_type: string;
variation_key: string;
}

export type SnapshotEvent = {
Expand Down Expand Up @@ -135,17 +143,24 @@ function makeConversionSnapshot(conversion: ConversionEvent): Visitor.Snapshot {
}

function makeDecisionSnapshot(event: ImpressionEvent): Visitor.Snapshot {
const { layer, experiment, variation } = event
const { layer, experiment, variation, ruleKey, flagKey, ruleType } = event
let layerId = layer ? layer.id : null
let experimentId = experiment ? experiment.id : null
let variationId = variation ? variation.id : null
let variationKey = variation ? variation.key : ''

return {
decisions: [
{
campaign_id: layerId,
experiment_id: experimentId,
variation_id: variationId,
metadata: {
flag_key: flagKey,
rule_key: ruleKey,
rule_type: ruleType,
variation_key: variationKey,
},
},
],
events: [
Expand Down