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.

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

[EBT] Add Shipper "FullStory" #129927

Merged
merged 9 commits into from
Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions packages/elastic-analytics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ NPM_MODULE_EXTRA_FILES = [
# "@npm//name-of-package"
# eg. "@npm//lodash"
RUNTIME_DEPS = [
"@npm//moment",
Copy link
Member

Choose a reason for hiding this comment

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

+1 for removing moment from the package and using plain old unix timestamps or Date objects

Copy link
Member Author

Choose a reason for hiding this comment

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

we need moment to validate the ISO string. We could replace it with a regex, but I worry we might leave out some formats depending on the TZ.

"@npm//rxjs",
]

Expand All @@ -51,6 +52,7 @@ RUNTIME_DEPS = [
TYPES_DEPS = [
"@npm//@types/node",
"@npm//@types/jest",
"@npm//moment",
"@npm//rxjs",
"//packages/kbn-logging:npm_module_types",
"//packages/kbn-logging-mocks:npm_module_types",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

// eslint-disable-next-line max-classes-per-file
import type { Observable } from 'rxjs';
import { BehaviorSubject, lastValueFrom, Subject } from 'rxjs';
import { BehaviorSubject, firstValueFrom, lastValueFrom, Subject } from 'rxjs';
import type { MockedLogger } from '@kbn/logging-mocks';
import { loggerMock } from '@kbn/logging-mocks';
import { AnalyticsClient } from './analytics_client';
Expand Down Expand Up @@ -528,6 +528,44 @@ describe('AnalyticsClient', () => {
]);
});

test('The undefined values are not forwarded to the global context', async () => {
afharo marked this conversation as resolved.
Show resolved Hide resolved
const context$ = new Subject<{ a_field?: boolean; b_field: number }>();
analyticsClient.registerContextProvider({
name: 'contextProviderA',
schema: {
a_field: {
type: 'boolean',
_meta: {
description: 'a_field description',
optional: true,
},
},
b_field: {
type: 'long',
_meta: {
description: 'b_field description',
},
},
},
context$,
});

const globalContextPromise = firstValueFrom(globalContext$.pipe(take(6), toArray()));
context$.next({ b_field: 1 });
context$.next({ a_field: false, b_field: 1 });
context$.next({ a_field: true, b_field: 1 });
context$.next({ b_field: 1 });
context$.next({ a_field: undefined, b_field: 2 });
await expect(globalContextPromise).resolves.toEqual([
{}, // Original empty state
{ b_field: 1 },
{ a_field: false, b_field: 1 },
{ a_field: true, b_field: 1 },
{ b_field: 1 }, // a_field is removed because the context provider removed it.
{ b_field: 2 }, // a_field is not forwarded because it is `undefined`
]);
});

test('Fails to register 2 context providers with the same name', () => {
analyticsClient.registerContextProvider({
name: 'contextProviderA',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,21 @@ export class ContextService {
[...this.contextProvidersRegistry.values()].reduce((acc, context) => {
return {
...acc,
...context,
...this.removeEmptyValues(context),
};
}, {} as Partial<EventContext>)
);
}

private removeEmptyValues(context?: Partial<EventContext>) {
pgayvallet marked this conversation as resolved.
Show resolved Hide resolved
if (!context) {
return {};
}
return Object.keys(context).reduce((acc, key) => {
if (context[key] !== undefined) {
acc[key] = context[key];
}
return acc;
}, {} as Partial<EventContext>);
}
}
27 changes: 27 additions & 0 deletions packages/elastic-analytics/src/events/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,34 @@

import type { ShipperName } from '../analytics_client';

/**
* Definition of the context that can be appended to the events through the {@link IAnalyticsClient.registerContextProvider}.
*/
export interface EventContext {
/**
* The unique user ID.
*/
userId?: string;
/**
* The user's organization ID.
*/
esOrgId?: string;
/**
* The product's version.
*/
version?: string;
/**
* The name of the current page.
*/
pageName?: string;
/**
* The current application ID.
*/
applicationId?: string;
/**
* The current entity ID (dashboard ID, visualization ID, etc.).
*/
entityId?: string;
// TODO: Extend with known keys
[key: string]: unknown;
}
Expand Down
6 changes: 5 additions & 1 deletion packages/elastic-analytics/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ export type {
// Types for the registerEventType API
EventTypeOpts,
} from './analytics_client';

export type { Event, EventContext, EventType, TelemetryCounter } from './events';
export { TelemetryCounterType } from './events';

export type {
RootSchema,
SchemaObject,
Expand All @@ -52,4 +54,6 @@ export type {
AllowedSchemaStringTypes,
AllowedSchemaTypes,
} from './schema';
export type { IShipper } from './shippers';

export type { IShipper, FullStorySnippetConfig, FullStoryShipperConfig } from './shippers';
export { FullStoryShipper } from './shippers';
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { formatPayload } from './format_payload';

describe('formatPayload', () => {
test('appends `_str` to string values', () => {
const payload = {
foo: 'bar',
baz: ['qux'],
};

expect(formatPayload(payload)).toEqual({
foo_str: payload.foo,
baz_strs: payload.baz,
});
});

test('appends `_int` to integer values', () => {
const payload = {
foo: 1,
baz: [100000],
};

expect(formatPayload(payload)).toEqual({
foo_int: payload.foo,
baz_ints: payload.baz,
});
});

test('appends `_real` to integer values', () => {
const payload = {
foo: 1.5,
baz: [100000.5],
};

expect(formatPayload(payload)).toEqual({
foo_real: payload.foo,
baz_reals: payload.baz,
});
});

test('appends `_bool` to booleans values', () => {
const payload = {
foo: true,
baz: [false],
};

expect(formatPayload(payload)).toEqual({
foo_bool: payload.foo,
baz_bools: payload.baz,
});
});

test('appends `_date` to Date values', () => {
const payload = {
foo: new Date(),
baz: [new Date()],
};

expect(formatPayload(payload)).toEqual({
foo_date: payload.foo,
baz_dates: payload.baz,
});
});

test('supports nested values', () => {
const payload = {
nested: {
foo: 'bar',
baz: ['qux'],
},
};

expect(formatPayload(payload)).toEqual({
nested: {
foo_str: payload.nested.foo,
baz_strs: payload.nested.baz,
},
});
});

test('does not mutate reserved keys', () => {
const payload = {
uid: 'uid',
displayName: 'displayName',
email: 'email',
acctId: 'acctId',
website: 'website',
pageName: 'pageName',
};

expect(formatPayload(payload)).toEqual(payload);
});

test('removes undefined values', () => {
const payload = {
foo: undefined,
baz: [undefined],
};

expect(formatPayload(payload)).toEqual({});
});

test('throws if null is provided', () => {
const payload = {
foo: null,
baz: [null],
};

expect(() => formatPayload(payload)).toThrowErrorMatchingInlineSnapshot(
`"Unsupported type: object"`
);
});

describe('String to Date identification', () => {
test('appends `_date` to ISO string values', () => {
const payload = {
foo: new Date().toISOString(),
baz: [new Date().toISOString()],
};

expect(formatPayload(payload)).toEqual({
foo_date: payload.foo,
baz_dates: payload.baz,
});
});

test('appends `_str` to random string values', () => {
const payload = {
foo: 'test-1',
baz: ['test-1'],
};

expect(formatPayload(payload)).toEqual({
foo_str: payload.foo,
baz_strs: payload.baz,
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import moment from 'moment';

// https://help.fullstory.com/hc/en-us/articles/360020623234#reserved-properties
const FULLSTORY_RESERVED_PROPERTIES = [
'uid',
'displayName',
'email',
'acctId',
'website',
// https://developer.fullstory.com/page-variables
'pageName',
];

export function formatPayload(context: Record<string, unknown>): Record<string, unknown> {
// format context keys as required for env vars, see docs: https://help.fullstory.com/hc/en-us/articles/360020623234
return Object.fromEntries(
Object.entries(context)
// Discard any undefined values
.map<[string, unknown]>(([key, value]) => {
return Array.isArray(value)
? [key, value.filter((v) => typeof v !== 'undefined')]
: [key, value];
})
.filter(
([, value]) => typeof value !== 'undefined' && (!Array.isArray(value) || value.length > 0)
)
// Transform key names according to the FullStory needs
.map(([key, value]) => {
if (FULLSTORY_RESERVED_PROPERTIES.includes(key)) {
return [key, value];
}
if (isRecord(value)) {
return [key, formatPayload(value)];
}
const valueType = getFullStoryType(value);
const formattedKey = valueType ? `${key}_${valueType}` : key;
return [formattedKey, value];
})
);
}

function getFullStoryType(value: unknown) {
// For arrays, make the decision based on the first element
const isArray = Array.isArray(value);
const v = isArray ? value[0] : value;
let type: string;
switch (typeof v) {
case 'string':
type = moment(v, moment.ISO_8601, true).isValid() ? 'date' : 'str';
break;
case 'number':
type = Number.isInteger(v) ? 'int' : 'real';
break;
case 'boolean':
type = 'bool';
break;
case 'object':
if (isDate(v)) {
type = 'date';
break;
}
default:
throw new Error(`Unsupported type: ${typeof v}`);
}

// convert to plural form for arrays
return isArray ? `${type}s` : type;
}

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value) && !isDate(value);
}
Comment on lines +78 to +80
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: could use _.isPlainObject

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as #129927 (comment): I wanted to avoid having another dependency in the package. If you think it's worth adding it, I don't mind adding it. :)


function isDate(value: unknown): value is Date {
return value instanceof Date;
}
Loading