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

feat(internal-plugin-metrics): allow caller to specify metadata of business metric events #4000

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
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
138 changes: 64 additions & 74 deletions packages/@webex/internal-plugin-metrics/src/business-metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,82 +8,22 @@ import {EventPayload, Table} from './metrics.types';
*/
export default class BusinessMetrics extends GenericMetrics {
/**
* unfortunately, the pinot team does not allow changes to the schema of wbxapp_callend_metrics
* so we have to shim this layer specifically for this
* https://confluence-eng-gpk2.cisco.com/conf/display/WAP/Table+wbxapp_callend_metrics
* @param {EventPayload} payload payload of the metric
* @returns {Promise<any>}
*/
private submitCallEndEvent({payload}: {payload: EventPayload}) {
const event = {
type: ['business'],
eventPayload: {
key: 'callEnd',
client_timestamp: new Date().toISOString(),
appType: 'Web Client',
value: {
...payload,
},
},
};

return this.submitEvent({
kind: 'buisness-events:wbxapp_callend_metrics -> ',
name: 'wbxapp_callend_metrics',
event,
});
}

/**
* Submit a buisness metric to our metrics endpoint, going to the default business_ucf table
* all event payload keys are converted into a hex string value
* unfortunately, the pinot team does not allow changes to the schema of business_metrics
* so we have to shim this layer specifically for this
* https://confluence-eng-gpk2.cisco.com/conf/display/WAP/Table%3A+business_metrics
* Build the metric event to submit.
* @param {string} name of the metric
* @param {EventPayload} payload payload of the metric
* @returns {Promise<any>}
*/
private submitBusinessMetricsEvent({name, payload}: {name: string; payload: EventPayload}) {
const event = {
type: ['business'],
eventPayload: {
key: name,
client_timestamp: new Date().toISOString(),
appType: 'Web Client',
value: {
...this.getContext(),
...this.getBrowserDetails(),
...payload,
},
},
};

return this.submitEvent({kind: 'buisness-events:business_metrics -> ', name, event});
}

/**
* Submit a buisness metric to our metrics endpoint, going to the default business_ucf table
* all event payload keys are converted into a hex string value
* https://confluence-eng-gpk2.cisco.com/conf/display/WAP/Business+metrics++-%3E+ROMA
* @param {string} name of the metric
* @param {EventPayload} user payload of the metric
* @returns {Promise<any>}
* @param {EventPayload} payload user payload of the metric
* @param {EventPayload} metadata to include outside of eventPayload.value
* @returns {object}
*/
private submitDefaultEvent({name, payload}: {name: string; payload: EventPayload}) {
const event = {
private buildEvent({name, payload, metadata}: {name: string; payload: object; metadata: object}) {
return {
type: ['business'],
eventPayload: {
key: name,
appType: 'Web Client',
client_timestamp: new Date().toISOString(),
context: this.getContext(),
browserDetails: this.getBrowserDetails(),
...metadata,
value: payload,
},
};

return this.submitEvent({kind: 'buisness-events:default -> ', name, event});
}

/**
Expand All @@ -93,30 +33,80 @@ export default class BusinessMetrics extends GenericMetrics {
* @param {string} name of the metric, ignored if going to wbxapp_callend_metrics
* @param {EventPayload} payload user payload of the metric
* @param {Table} table optional - to submit the metric to and adapt the sent schema
* @param {EventPayload} metadata optional - to include outside of eventPayload.value
* @returns {Promise<any>}
*/
public submitBusinessEvent({
name,
payload,
table,
metadata,
}: {
name: string;
payload: EventPayload;
table?: Table;
metadata?: EventPayload;
}): Promise<void> {
if (!table) {
table = 'default';
}
if (!metadata) {
metadata = {};
}
if (!metadata.appType) {
metadata.appType = 'Web Client';
}
switch (table) {
case 'wbxapp_callend_metrics':
return this.submitCallEndEvent({payload});
case 'business_metrics':
return this.submitBusinessMetricsEvent({name, payload});
case 'wbxapp_callend_metrics': {
// https://confluence-eng-gpk2.cisco.com/conf/display/WAP/Table+wbxapp_callend_metrics
const callEndEvent = this.buildEvent({name: 'callEnd', payload, metadata});

return this.submitEvent({
kind: 'buisness-events:wbxapp_callend_metrics -> ',
name: 'wbxapp_callend_metrics',
event: callEndEvent,
});
}

case 'business_metrics': {
// all event payload keys are converted into a hex string value
// unfortunately, the pinot team does not allow changes to the schema of business_metrics
// so we have to shim this layer specifically for this
// https://confluence-eng-gpk2.cisco.com/conf/display/WAP/Table%3A+business_metrics
const businessEvent = this.buildEvent({
name,
payload: {
...this.getContext(),
...this.getBrowserDetails(),
...payload,
},
metadata,
});

return this.submitEvent({
kind: 'buisness-events:business_metrics -> ',
name,
event: businessEvent,
});
}

case 'business_ucf':
return this.submitDefaultEvent({name, payload});
case 'default':
default:
return this.submitDefaultEvent({name, payload});
default: {
// all event payload keys are converted into a hex string value
// https://confluence-eng-gpk2.cisco.com/conf/display/WAP/Business+metrics++-%3E+ROMA
const defaultEvent = this.buildEvent({
name,
payload,
metadata: {
context: this.getContext(),
browserDetails: this.getBrowserDetails(),
...metadata,
},
});

return this.submitEvent({kind: 'buisness-events:default -> ', name, event: defaultEvent});
}
lialang-cisco marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,12 @@ class Metrics extends WebexPlugin {
name,
payload,
table,
metadata,
}: {
name: string;
payload: EventPayload;
table?: Table;
metadata?: EventPayload;
}) {
if (!this.isReady) {
// @ts-ignore
Expand All @@ -223,7 +225,7 @@ class Metrics extends WebexPlugin {

this.lazyBuildBusinessMetrics();

return this.businessMetrics.submitBusinessEvent({name, payload, table});
return this.businessMetrics.submitBusinessEvent({name, payload, table, metadata});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,12 @@ describe('internal-plugin-metrics', () => {
businessMetrics.clientMetricsBatcher.request = request;

assert.equal(requestCalls.length, 0)
businessMetrics.submitBusinessEvent({ name: "foobar", payload: {bar:"gee"}, table: 'business_metrics' })
businessMetrics.submitBusinessEvent({ name: "foobar", payload: {bar: "gee"}, table: 'business_metrics', metadata: {asdf: 'hjkl'} })
assert.equal(requestCalls.length, 1)
assert.deepEqual(requestCalls[0], {
eventPayload: {
key: 'foobar',
asdf: 'hjkl',

Choose a reason for hiding this comment

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

So, if the given metadata field already exists in the request, then this input will override it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's right

appType: 'Web Client',
client_timestamp: requestCalls[0].eventPayload.client_timestamp, // This is to bypass time check, which is checked below.
value: {
Expand All @@ -173,7 +174,6 @@ describe('internal-plugin-metrics', () => {
os: getOSNameInternal(),
app: {version: 'webex-version'},
device: {id: 'deviceId'},
locale: 'language',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicate, locale is already included a few lines above

}
},
type: ['business'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,14 @@ describe('internal-plugin-metrics', () => {
name: 'foobar',
payload: {},
table: 'test',
metadata: { foo: 'bar' },
});

assert.calledWith(webex.internal.newMetrics.businessMetrics.submitBusinessEvent, {
name: 'foobar',
payload: {},
table: 'test',
metadata: { foo: 'bar' },
});
});

Expand Down
Loading