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

Fix tests, add docs, apply nits, fix types #10

Merged
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
16 changes: 10 additions & 6 deletions packages/analytics/client/src/analytics_client/analytics_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import type { Type } from 'io-ts';
import type { Mixed } from 'io-ts';
import type { Observable } from 'rxjs';
import { BehaviorSubject, Subject, combineLatest, from, merge } from 'rxjs';
import {
Expand Down Expand Up @@ -43,7 +43,7 @@ import { ContextService } from './context_service';
import { schemaToIoTs, validateSchema } from '../schema/validation';

interface EventDebugLogMeta extends LogMeta {
ebt_event: Event;
ebt_event: Event<unknown>;
Copy link
Author

Choose a reason for hiding this comment

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

Fixed the issue with the properties being forced to extend Record<string, unknown>

}

export class AnalyticsClient implements IAnalyticsClient {
Expand All @@ -65,7 +65,7 @@ export class AnalyticsClient implements IAnalyticsClient {
private readonly shipperRegistered$ = new Subject<void>();
private readonly eventTypeRegistry = new Map<
EventType,
EventTypeOpts<unknown> & { validator?: Type<Record<string, unknown>> }
EventTypeOpts<unknown> & { validator?: Mixed }
>();
private readonly contextService: ContextService;
private readonly context$ = new BehaviorSubject<Partial<EventContext>>({});
Expand All @@ -88,7 +88,7 @@ export class AnalyticsClient implements IAnalyticsClient {
this.reportEnqueuedEventsWhenClientIsReady();
}

public reportEvent = <EventTypeData extends Record<string, unknown>>(
public reportEvent = <EventTypeData extends object>(
eventType: EventType,
eventData: EventTypeData
) => {
Expand Down Expand Up @@ -119,14 +119,18 @@ export class AnalyticsClient implements IAnalyticsClient {

// If the validator is registered (dev-mode only), perform the validation.
if (eventTypeOpts.validator) {
validateSchema(`Event Type '${eventType}'`, eventTypeOpts.validator, eventData);
validateSchema<EventTypeData>(
`Event Type '${eventType}'`,
eventTypeOpts.validator,
eventData
);
}

const event: Event = {
timestamp,
event_type: eventType,
context: this.context$.value,
properties: eventData,
properties: eventData as unknown as Record<string, unknown>,
};

// debug-logging before checking the opt-in status to help during development
Expand Down
2 changes: 1 addition & 1 deletion packages/analytics/client/src/analytics_client/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export interface IAnalyticsClient {
* @param eventType The event type registered via the `registerEventType` API.
* @param eventData The properties matching the schema declared in the `registerEventType` API.
*/
reportEvent: <EventTypeData extends Record<string, unknown>>(
reportEvent: <EventTypeData extends object>(
eventType: EventType,
eventData: EventTypeData
) => void;
Expand Down
4 changes: 2 additions & 2 deletions packages/analytics/client/src/events/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export interface TelemetryCounter {
/**
* Definition of the full event structure
*/
export interface Event {
export interface Event<Properties = Record<string, unknown>> {
/**
* The time the event was generated in ISO format.
*/
Expand All @@ -120,7 +120,7 @@ export interface Event {
/**
* The specific properties of the event type.
*/
properties: Record<string, unknown>;
properties: Properties;
/**
* The {@link EventContext} enriched during the processing pipeline.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const FULLSTORY_RESERVED_PROPERTIES = [
'pageName',
];

export function formatPayload(context: Record<string, unknown>): Record<string, unknown> {
export function formatPayload(context: object): 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ RUNTIME_DEPS = [
"@npm//rxjs",
"@npm//uuid",
"//packages/analytics/client",
"//packages/kbn-ebt-tools",
"//packages/core/base/core-base-browser-mocks",
"//packages/core/injected-metadata/core-injected-metadata-browser-mocks",
]
Expand All @@ -41,6 +42,7 @@ TYPES_DEPS = [
"@npm//rxjs",
"//packages/kbn-logging:npm_module_types",
"//packages/analytics/client:npm_module_types",
"//packages/kbn-ebt-tools:npm_module_types",
"//packages/core/base/core-base-browser-internal:npm_module_types",
"//packages/core/injected-metadata/core-injected-metadata-browser-internal:npm_module_types",
"//packages/core/analytics/core-analytics-browser:npm_module_types",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ describe('AnalyticsService', () => {
await expect(
firstValueFrom(analyticsClientMock.registerContextProvider.mock.calls[0][0].context$)
).resolves.toMatchInlineSnapshot(`
Object {
"branch": "branch",
"buildNum": 100,
"buildSha": "buildSha",
"isDev": true,
"isDistributable": false,
"version": "version",
}
`);
Object {
"branch": "branch",
"buildNum": 100,
"buildSha": "buildSha",
"isDev": true,
"isDistributable": false,
"version": "version",
}
`);
await expect(
firstValueFrom(analyticsClientMock.registerContextProvider.mock.calls[1][0].context$)
).resolves.toEqual({ session_id: expect.any(String) });
Expand All @@ -44,6 +44,155 @@ describe('AnalyticsService', () => {
});
});

test('should register the `metrics` and `clicks` event types on creation', () => {
expect(analyticsClientMock.registerEventType).toHaveBeenCalledTimes(2);
expect(analyticsClientMock.registerEventType.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"eventType": "metric",
"schema": Object {
"duration": Object {
"_meta": Object {
"description": "The main event duration in ms",
"optional": true,
},
"type": "integer",
},
"eventName": Object {
"_meta": Object {
"description": "Type of the event",
},
"type": "keyword",
},
"jsHeapSizeLimit": Object {
"_meta": Object {
"description": "performance.memory.jsHeapSizeLimit",
"optional": true,
},
"type": "long",
},
"key1": Object {
"_meta": Object {
"description": "Performance metric label 1",
"optional": true,
},
"type": "keyword",
},
"key2": Object {
"_meta": Object {
"description": "Performance metric label 2",
"optional": true,
},
"type": "keyword",
},
"key3": Object {
"_meta": Object {
"description": "Performance metric label 3",
"optional": true,
},
"type": "keyword",
},
"key4": Object {
"_meta": Object {
"description": "Performance metric label 4",
"optional": true,
},
"type": "keyword",
},
"key5": Object {
"_meta": Object {
"description": "Performance metric label 5",
"optional": true,
},
"type": "keyword",
},
"meta": Object {
"_meta": Object {
"description": "Meta data that is searchable but not aggregatable",
"optional": true,
},
"type": "pass_through",
},
"status": Object {
"_meta": Object {
"description": "A status",
"optional": true,
},
"type": "keyword",
},
"totalJSHeapSize": Object {
"_meta": Object {
"description": "performance.memory.totalJSHeapSize",
"optional": true,
},
"type": "long",
},
"usedJSHeapSize": Object {
"_meta": Object {
"description": "performance.memory.usedJSHeapSize",
"optional": true,
},
"type": "long",
},
"value1": Object {
"_meta": Object {
"description": "Performance metric value 1",
"optional": true,
},
"type": "long",
},
"value2": Object {
"_meta": Object {
"description": "Performance metric value 2",
"optional": true,
},
"type": "long",
},
"value3": Object {
"_meta": Object {
"description": "Performance metric value 3",
"optional": true,
},
"type": "long",
},
"value4": Object {
"_meta": Object {
"description": "Performance metric value 4",
"optional": true,
},
"type": "long",
},
"value5": Object {
"_meta": Object {
"description": "Performance metric value 5",
"optional": true,
},
"type": "long",
},
},
},
]
`);
expect(analyticsClientMock.registerEventType.mock.calls[1]).toMatchInlineSnapshot(`
Array [
Object {
"eventType": "click",
"schema": Object {
"target": Object {
"items": Object {
"_meta": Object {
"description": "The attributes of the clicked element and all its parents in the form \`{attr.name}={attr.value}\`. It allows finding the clicked elements by looking up its attributes like \\"data-test-subj=my-button\\".",
},
"type": "keyword",
},
"type": "array",
},
},
},
]
`);
});

test('setup should expose all the register APIs, reportEvent and opt-in', () => {
const injectedMetadata = injectedMetadataServiceMock.createSetupContract();
expect(analyticsService.setup({ injectedMetadata })).toStrictEqual({
Expand Down Expand Up @@ -76,12 +225,12 @@ describe('AnalyticsService', () => {
await expect(
firstValueFrom(analyticsClientMock.registerContextProvider.mock.calls[3][0].context$)
).resolves.toMatchInlineSnapshot(`
Object {
"cluster_name": "cluster_name",
"cluster_uuid": "cluster_uuid",
"cluster_version": "version",
}
`);
Object {
"cluster_name": "cluster_name",
"cluster_uuid": "cluster_uuid",
"cluster_version": "version",
}
`);
});

test('setup should expose only the APIs report and opt-in', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { of } from 'rxjs';
import type { AnalyticsClient } from '@kbn/analytics-client';
import { createAnalytics } from '@kbn/analytics-client';
import { registerMetricEventType } from '@kbn/ebt-tools';
import type { CoreContext } from '@kbn/core-base-browser-internal';
import type { InternalInjectedMetadataSetup } from '@kbn/core-injected-metadata-browser-internal';
import type { AnalyticsServiceSetup, AnalyticsServiceStart } from '@kbn/core-analytics-browser';
Expand All @@ -34,6 +35,8 @@ export class AnalyticsService {
});

this.registerBuildInfoAnalyticsContext(core);
// Register special `metrics` type
registerMetricEventType(this.analyticsClient);
Comment on lines +38 to +39
Copy link
Author

Choose a reason for hiding this comment

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

The analytics service in core registers the metric type 😇

We can work on a follow up PR to move the reportMetricEvent API to an actual API exposed by the AnalyticServices in core. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

Don't think it's critical, we already have a package.
As you fish 🐟


// We may eventually move the following to the client's package since they are not Kibana-specific
// and can benefit other consumers of the client.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ NPM_MODULE_EXTRA_FILES = [
RUNTIME_DEPS = [
"@npm//rxjs",
"//packages/analytics/client",
"//packages/kbn-ebt-tools",
]

TYPES_DEPS = [
"@npm//@types/node",
"@npm//@types/jest",
"@npm//rxjs",
"//packages/analytics/client:npm_module_types",
"//packages/kbn-ebt-tools:npm_module_types",
"//packages/core/base/core-base-server-internal:npm_module_types",
"//packages/core/analytics/core-analytics-server:npm_module_types",
]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* 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 { AnalyticsClient } from '@kbn/analytics-client';
import { Subject } from 'rxjs';

export const analyticsClientMock: jest.Mocked<AnalyticsClient> = {
optIn: jest.fn(),
reportEvent: jest.fn(),
registerEventType: jest.fn(),
registerContextProvider: jest.fn(),
removeContextProvider: jest.fn(),
registerShipper: jest.fn(),
telemetryCounter$: new Subject(),
shutdown: jest.fn(),
};

jest.doMock('@kbn/analytics-client', () => ({
createAnalytics: () => analyticsClientMock,
}));
Loading