-
Notifications
You must be signed in to change notification settings - Fork 360
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(cc-metrics): add MetricsManager #4127
base: wxcc
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis change replaces the dependency on Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
yarn install v1.22.22 (For a CapTP with native promises, see @endo/eventual-send and @endo/captp) ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (8)
packages/@webex/plugin-cc/test/unit/spec/MetricsManager.ts (2)
74-74
: Use a more explicit method to reset the singleton between tests.Directly accessing and modifying a private static property of a class is not a best practice. It would be better to expose a testing-specific method to reset the singleton state or to use proper dependency injection.
- MetricsManager['instance'] = undefined; + // Add a static resetInstance method to MetricsManager class + MetricsManager.resetInstance();Then add this method to the MetricsManager class:
// Add to MetricsManager.ts public static resetInstance(): void { MetricsManager.instance = undefined; }
3-3
:❓ Verification inconclusive
Avoid importing from implementation files.
Importing types directly from implementation files (like
metrics.types.ts
) rather than from the package's public API can lead to tight coupling and make your code more brittle to internal changes in the dependency.-import { EventPayload } from '@webex/internal-plugin-metrics/src/metrics.types'; +import { EventPayload } from '@webex/internal-plugin-metrics';Let's check if the type is exported from the main package entry point:
🏁 Script executed:
#!/bin/bash # Check if EventPayload is exported from the main package entry point rg -A 3 "export.*EventPayload" --type-add "ts:*.{ts,tsx}" --type ts packages/@webex/internal-plugin-metrics/Length of output: 1534
Update Import to Use the Public API
It looks like the type is originally declared and exported in the internal file (
src/metrics.types.ts
), but to avoid tight coupling you should import it from the package’s public API. Please ensure that the package’s main entry point (e.g., its index file) properly re‑exportsEventPayload
so that changing the import won’t break consumers.
File:
packages/@webex/plugin-cc/test/unit/spec/MetricsManager.ts
(Line 3)Action: Update the import statement as follows:
-import { EventPayload } from '@webex/internal-plugin-metrics/src/metrics.types'; +import { EventPayload } from '@webex/internal-plugin-metrics';Verification: Confirm that
EventPayload
is available via the public API. If not, update the main entry point of@webex/internal-plugin-metrics
to re-export it.packages/@webex/plugin-cc/src/MetricsManager.ts (5)
1-1
: Avoid importing from implementation files.Similar to the test file, importing types directly from implementation files rather than from the package's public API can lead to tight coupling.
-import {EventPayload} from '@webex/internal-plugin-metrics/src/metrics.types'; +import {EventPayload} from '@webex/internal-plugin-metrics';
5-5
: Make theeventNames
type more extensible.The current implementation limits event names to just three predefined values. This could become a maintenance burden as new events are added. Consider using a more extensible approach.
-type eventNames = 'login' | 'agent-login' | 'agent-logout'; +type CoreEventNames = 'login' | 'agent-login' | 'agent-logout'; +type eventNames = CoreEventNames | string;Alternatively, if you want to keep the strict typing for event names but make it more maintainable:
enum EventNames { Login = 'login', AgentLogin = 'agent-login', AgentLogout = 'agent-logout' } type eventNames = `${EventNames}`;
7-14
: Consider using dependency injection instead of a singleton pattern.The singleton pattern can make testing difficult and creates tight coupling. A more flexible approach would be to use dependency injection, where the MetricsManager is created and passed to components that need it.
While the singleton pattern works, it might be worth considering dependency injection for better testability and flexibility. This would involve:
- Removing the static instance property
- Making the constructor public
- Creating the instance in a higher-level component (e.g., a service locator or composition root)
- Passing the instance to components that need it
This approach would also eliminate the need for the getInstance method.
16-26
: Add error handling to the metric tracking.The current implementation silently fails if there's an issue with submitting the metric. Consider adding error logging to capture potential problems.
public trackBehavioralMetric(eventName: eventNames, data?: EventPayload) { if (this.webex?.internal?.newMetrics) { + try { this.webex.internal.newMetrics.submitBehavioralEvent({ product: 'wxcc_desktop', agent: 'sdk', target: eventName, verb: 'load', payload: data, }); + } catch (error) { + console.error('Failed to submit behavioral metric:', error); + } } }
28-35
: Add error handling to the operational metric tracking.Similar to the behavioral metrics, add error handling for operational metrics to prevent silent failures.
public trackOperationalMetric(eventName: string, payload?: EventPayload) { if (this.webex?.internal?.newMetrics) { + try { this.webex.internal.newMetrics.submitOperationalEvent({ name: eventName, payload, }); + } catch (error) { + console.error('Failed to submit operational metric:', error); + } } }packages/@webex/plugin-cc/src/services/agent/index.ts (1)
62-74
: DRY opportunity: Extract common metrics tracking logic.The metrics tracking code in this error handler is nearly identical to the one in the notifFail section (lines 89-94). Consider extracting this into a helper function to avoid duplication.
+ // Helper function to track login metrics + const trackLoginMetrics = (isSuccess: boolean, error?: any) => { + metricsManager.trackBehavioralMetric('agent-login', { + ...p.data, + isSuccess, + roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '', + ...(error && { error: JSON.stringify(error) }), + }); + }; + err: /* istanbul ignore next */ (e: any) => { - metricsManager.trackBehavioralMetric('agent-login', { - ...p.data, - isSuccess: false, - roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '', - }); + trackLoginMetrics(false); return new Err.Details('Service.aqm.agent.stationLogin', { status: e.response?.status ?? 0, type: e.response?.data?.errorType, trackingId: e.response?.headers?.trackingid?.split('_')[1], }); },And then at line 88:
err(e) { - metricsManager.trackBehavioralMetric('agent-login', { - ...p.data, - isSuccess: false, - roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '', - error: JSON.stringify(e), - }); + trackLoginMetrics(false, e); },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (8)
packages/@webex/plugin-cc/package.json
(1 hunks)packages/@webex/plugin-cc/src/MetricsManager.ts
(1 hunks)packages/@webex/plugin-cc/src/cc.ts
(4 hunks)packages/@webex/plugin-cc/src/services/agent/index.ts
(3 hunks)packages/@webex/plugin-cc/src/services/index.ts
(2 hunks)packages/@webex/plugin-cc/src/types.ts
(2 hunks)packages/@webex/plugin-cc/test/unit/spec/MetricsManager.ts
(1 hunks)packages/@webex/plugin-cc/test/unit/spec/services/agent/index.ts
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Initialize Project
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (13)
packages/@webex/plugin-cc/src/services/index.ts (2)
1-1
: Dependency change aligns with metrics integrationGood addition of the Metrics import from the internal plugin. This aligns with the PR objective of introducing metrics tracking capabilities.
25-25
: Clean implementation of metrics dependency injectionGood approach using dependency injection to provide the metrics capability to the routing agent. The singleton pattern with
getInstance
ensures consistent usage of the same metrics instance throughout the application.packages/@webex/plugin-cc/src/cc.ts (4)
30-30
: Module import properly structuredThe import of the MetricsManager is correctly positioned with the other imports.
42-42
: Private member correctly definedGood practice using a private member variable for the metrics manager.
69-69
: Proper initialization timingThe initialization of the metrics manager is correctly placed after the webex instance is ready and follows the same singleton pattern used elsewhere in the code.
207-210
: Well-structured metrics event implementationGood implementation of the metrics tracking for agent login:
- The timing is correct - after the login response is successfully processed
- The event name is clear and descriptive
- The data payload contains relevant information without exposing sensitive details
This aligns well with the PR objective of adding behavioral metrics.
packages/@webex/plugin-cc/test/unit/spec/services/agent/index.ts (3)
1-1
: Import path correctly structuredThe import path for MetricsManager is appropriate for the test file.
14-14
: Mock variable declaration looks goodThe declaration of the mock MetricsManager follows the same pattern as other mocks in this file.
26-26
: Correctly updated function call with new parameterThe call to routingAgent has been correctly updated to include the mock metrics manager, following the implementation change.
packages/@webex/plugin-cc/package.json (1)
28-28
:❓ Verification inconclusive
Dependency updated for metrics functionality
The dependency has been correctly updated from
@webex/internal-plugin-mercury
to@webex/internal-plugin-metrics
to support the new metrics tracking capability.
🏁 Script executed:
#!/bin/bash # Description: Check if the mercury plugin was used elsewhere in the codebase # This helps ensure we haven't removed a dependency that's still needed # Search for imports of the mercury plugin echo "Checking for imports of @webex/internal-plugin-mercury:" rg --type-add 'ts:*.{ts,js}' --type ts '@webex/internal-plugin-mercury' --stats # Check for any potential references to mercury in the code echo -e "\nChecking for potential references to Mercury functionality:" rg --type-add 'ts:*.{ts,js}' --type ts -i 'mercury' 'packages/@webex/plugin-cc/' --statsLength of output: 5065
Metrics dependency updated – please verify lingering Mercury references
The dependency change in packages/@webex/plugin-cc/package.json has been applied correctly by replacing the Mercury dependency with Metrics. However, our search indicates that there are still references to
@webex/internal-plugin-mercury
and related Mercury terminology in plugin‑cc (e.g., in comments and type definitions insrc/cc.ts
andsrc/types.ts
). Please review these occurrences and confirm whether they are intentional (for backward compatibility or legacy documentation) or if they should also be updated to align with the new Metrics functionality.packages/@webex/plugin-cc/src/types.ts (1)
2-2
: Good decision to directly import the metric event types.Using the specific types from
@webex/internal-plugin-metrics
provides proper type safety and makes the interface definition clearer.packages/@webex/plugin-cc/src/services/agent/index.ts (2)
8-8
: Import looks good.The import of the MetricsManager is correctly placed and follows the project's import style.
16-16
: Updated function signature is necessary for the metrics integration.The function signature has been appropriately updated to include the MetricsManager parameter, which is required for tracking metrics within the routing agent functionality.
fakeMetricsManager.trackBehavioralMetric = jest.fn(); | ||
fakeMetricsManager.trackOperationalMetric = jest.fn(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing initialization for fakeMetricsManager
The mock functions are being defined on fakeMetricsManager
before it's initialized, which would cause runtime errors when the tests are executed.
fakeAqm = new AqmReqs() as jest.Mocked<AqmReqs>;
fakeAqm.reqEmpty = jest.fn().mockImplementation((fn) => fn);
fakeAqm.req = jest.fn().mockImplementation((fn) => fn);
+ fakeMetricsManager = {
+ trackBehavioralMetric: jest.fn(),
+ trackOperationalMetric: jest.fn(),
+ } as unknown as jest.Mocked<MetricsManager>;
- fakeMetricsManager.trackBehavioralMetric = jest.fn();
- fakeMetricsManager.trackOperationalMetric = jest.fn();
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
fakeMetricsManager.trackBehavioralMetric = jest.fn(); | |
fakeMetricsManager.trackOperationalMetric = jest.fn(); | |
fakeAqm = new AqmReqs() as jest.Mocked<AqmReqs>; | |
fakeAqm.reqEmpty = jest.fn().mockImplementation((fn) => fn); | |
fakeAqm.req = jest.fn().mockImplementation((fn) => fn); | |
fakeMetricsManager = { | |
trackBehavioralMetric: jest.fn(), | |
trackOperationalMetric: jest.fn(), | |
} as unknown as jest.Mocked<MetricsManager>; |
newMetrics: { | ||
submitBehavioralEvent: SubmitBehavioralEvent; | ||
submitOperationalEvent: SubmitOperationalEvent; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider using a more permanent property name instead of "newMetrics".
The property name "newMetrics" suggests a transitional approach. Since this appears to be a permanent refactoring of how metrics are handled, consider using a more semantic name like "metrics" or "metricsService" for long-term maintainability.
- newMetrics: {
+ metrics: {
submitBehavioralEvent: SubmitBehavioralEvent;
submitOperationalEvent: SubmitOperationalEvent;
};
Let's check if there are other occurrences of "newMetrics" in the codebase to understand the scope of this change:
🏁 Script executed:
#!/bin/bash
# Find all occurrences of "newMetrics" to evaluate the impact of renaming
rg "newMetrics" --type-add "ts:*.{ts,tsx}" --type ts
Length of output: 32253
Action: Rename "newMetrics" to a permanent, semantic property name across the codebase.
The identifier "newMetrics"
is used extensively—not only in the type definitions in packages/@webex/plugin-cc/src/types.ts
but also across modules in MetricsManager, various tests, and even in plugins like internal-plugin-metrics and plugin-meetings. Since this represents the permanent approach for handling metrics, it would improve long-term maintainability to adopt a more descriptive name such as "metrics"
or "metricsService"
.
Recommendations:
- Type Definition: Update the property name in
packages/@webex/plugin-cc/src/types.ts
. - Implementation Files: Change all references in files like
packages/@webex/plugin-cc/src/MetricsManager.ts
accordingly. - Tests and Other Modules: Ensure that any unit tests or other modules (e.g., files under internal-plugin-metrics and plugin-meetings) that refer to
"newMetrics"
are also updated to maintain consistency.
Diff Suggestion:
- newMetrics: {
+ metrics: {
submitBehavioralEvent: SubmitBehavioralEvent;
submitOperationalEvent: SubmitOperationalEvent;
};
Please make sure that if you decide to rename "newMetrics"
, all affected files are updated to prevent any potential reference errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
newMetrics: { | |
submitBehavioralEvent: SubmitBehavioralEvent; | |
submitOperationalEvent: SubmitOperationalEvent; | |
}; | |
metrics: { | |
submitBehavioralEvent: SubmitBehavioralEvent; | |
submitOperationalEvent: SubmitOperationalEvent; | |
}; |
// describe('trackOperationalMetric', () => { | ||
// it('should submit an operational event when newMetrics is available', () => { | ||
// metricsManager.trackOperationalMetric(); | ||
|
||
// expect(webex.internal.newMetrics.submitOperationalEvent).toHaveBeenCalledWith({ | ||
// name: 'ccsdk.event', | ||
// payload: { | ||
// action: 'load', | ||
// }, | ||
// }); | ||
// }); | ||
|
||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Uncomment and implement the operational metrics test.
The operational metrics testing is commented out, which means this functionality isn't being verified. As operational metrics are a key part of the new feature, it's important to test them properly.
- // describe('trackOperationalMetric', () => {
- // it('should submit an operational event when newMetrics is available', () => {
- // metricsManager.trackOperationalMetric();
-
- // expect(webex.internal.newMetrics.submitOperationalEvent).toHaveBeenCalledWith({
- // name: 'ccsdk.event',
- // payload: {
- // action: 'load',
- // },
- // });
- // });
-
- // });
+ describe('trackOperationalMetric', () => {
+ it('should submit an operational event when newMetrics is available', () => {
+ const eventName = 'ccsdk.event';
+ const payload = { action: 'load' };
+
+ metricsManager.trackOperationalMetric(eventName, payload);
+
+ expect(webex.internal.newMetrics.submitOperationalEvent).toHaveBeenCalledWith({
+ name: eventName,
+ payload,
+ });
+ });
+
+ it('should not submit an operational event when newMetrics is not available', () => {
+ webex.internal.newMetrics = undefined as any;
+
+ metricsManager.trackOperationalMetric('ccsdk.event');
+
+ // Test passes if there's no error
+ expect(true).toBe(true);
+ });
+ });
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// describe('trackOperationalMetric', () => { | |
// it('should submit an operational event when newMetrics is available', () => { | |
// metricsManager.trackOperationalMetric(); | |
// expect(webex.internal.newMetrics.submitOperationalEvent).toHaveBeenCalledWith({ | |
// name: 'ccsdk.event', | |
// payload: { | |
// action: 'load', | |
// }, | |
// }); | |
// }); | |
// }); | |
describe('trackOperationalMetric', () => { | |
it('should submit an operational event when newMetrics is available', () => { | |
const eventName = 'ccsdk.event'; | |
const payload = { action: 'load' }; | |
metricsManager.trackOperationalMetric(eventName, payload); | |
expect(webex.internal.newMetrics.submitOperationalEvent).toHaveBeenCalledWith({ | |
name: eventName, | |
payload, | |
}); | |
}); | |
it('should not submit an operational event when newMetrics is not available', () => { | |
webex.internal.newMetrics = undefined as any; | |
metricsManager.trackOperationalMetric('ccsdk.event'); | |
// Test passes if there's no error | |
expect(true).toBe(true); | |
}); | |
}); |
// it('should not submit a behavioral event when newMetrics is not available', () => { | ||
// webex.internal.newMetrics = undefined; | ||
|
||
// metricsManager.trackBehavioralMetric('login'); | ||
|
||
// expect(webex.internal.newMetrics?.submitBehavioralEvent).not.toHaveBeenCalled(); | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Uncomment and implement the negative test case.
Commented-out tests reduce the coverage and can lead to gaps in functionality testing. The case for when newMetrics is not available is important, as it tests a defensive coding pattern in the implementation.
- // it('should not submit a behavioral event when newMetrics is not available', () => {
- // webex.internal.newMetrics = undefined;
-
- // metricsManager.trackBehavioralMetric('login');
-
- // expect(webex.internal.newMetrics?.submitBehavioralEvent).not.toHaveBeenCalled();
- // });
+ it('should not submit a behavioral event when newMetrics is not available', () => {
+ webex.internal.newMetrics = undefined as any;
+
+ metricsManager.trackBehavioralMetric('login');
+
+ // Test passes if there's no error
+ expect(true).toBe(true);
+ });
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// it('should not submit a behavioral event when newMetrics is not available', () => { | |
// webex.internal.newMetrics = undefined; | |
// metricsManager.trackBehavioralMetric('login'); | |
// expect(webex.internal.newMetrics?.submitBehavioralEvent).not.toHaveBeenCalled(); | |
// }); | |
it('should not submit a behavioral event when newMetrics is not available', () => { | |
webex.internal.newMetrics = undefined as any; | |
metricsManager.trackBehavioralMetric('login'); | |
// Test passes if there's no error | |
expect(true).toBe(true); | |
}); |
// Make the class a singleton | ||
public static getInstance(options: {webex: WebexSDK}): MetricsManager { | ||
if (!MetricsManager.instance) { | ||
MetricsManager.instance = new MetricsManager(options); | ||
} | ||
|
||
return MetricsManager.instance; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add safety checks in getInstance to prevent invalid initialization.
The getInstance
method doesn't validate that options.webex
is provided. This could lead to runtime errors if the method is called without the required parameters.
public static getInstance(options: {webex: WebexSDK}): MetricsManager {
+ if (!options || !options.webex) {
+ throw new Error('WebexSDK instance is required to create a MetricsManager');
+ }
+
if (!MetricsManager.instance) {
MetricsManager.instance = new MetricsManager(options);
}
return MetricsManager.instance;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Make the class a singleton | |
public static getInstance(options: {webex: WebexSDK}): MetricsManager { | |
if (!MetricsManager.instance) { | |
MetricsManager.instance = new MetricsManager(options); | |
} | |
return MetricsManager.instance; | |
} | |
// Make the class a singleton | |
public static getInstance(options: {webex: WebexSDK}): MetricsManager { | |
if (!options || !options.webex) { | |
throw new Error('WebexSDK instance is required to create a MetricsManager'); | |
} | |
if (!MetricsManager.instance) { | |
MetricsManager.instance = new MetricsManager(options); | |
} | |
return MetricsManager.instance; | |
} |
err(e) { | ||
metricsManager.trackBehavioralMetric('agent-login', { | ||
...p.data, | ||
isSuccess: false, | ||
roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '', | ||
error: JSON.stringify(e), | ||
}); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Track success metrics as well as failures.
Currently, metrics are tracked only in error scenarios. For completeness and to get a full picture of the system's performance, it would be valuable to track successful operations as well.
Add a success tracking implementation in the notifSuccess handler:
notifSuccess: {
bind: {
type: CC_EVENTS.AGENT_STATION_LOGIN,
data: {type: CC_EVENTS.AGENT_STATION_LOGIN_SUCCESS},
},
msg: {} as Agent.StationLoginSuccess,
+ success() {
+ metricsManager.trackBehavioralMetric('agent-login', {
+ ...p.data,
+ isSuccess: true,
+ roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : ''
+ });
+ },
},
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
err(e) { | |
metricsManager.trackBehavioralMetric('agent-login', { | |
...p.data, | |
isSuccess: false, | |
roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '', | |
error: JSON.stringify(e), | |
}); | |
}, | |
err(e) { | |
metricsManager.trackBehavioralMetric('agent-login', { | |
...p.data, | |
isSuccess: false, | |
roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '', | |
error: JSON.stringify(e), | |
}); | |
}, | |
notifSuccess: { | |
bind: { | |
type: CC_EVENTS.AGENT_STATION_LOGIN, | |
data: { type: CC_EVENTS.AGENT_STATION_LOGIN_SUCCESS }, | |
}, | |
msg: {} as Agent.StationLoginSuccess, | |
success() { | |
metricsManager.trackBehavioralMetric('agent-login', { | |
...p.data, | |
isSuccess: true, | |
roles: Array.isArray(p.data.roles) ? p.data.roles.join(',') : '' | |
}); | |
}, | |
}, |
This pull request is automatically being deployed by Amplify Hosting (learn more). |
@@ -25,7 +25,7 @@ | |||
"dependencies": { | |||
"@types/platform": "1.3.4", | |||
"@webex/calling": "workspace:*", | |||
"@webex/internal-plugin-mercury": "workspace:*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why removing mercury here ?
public trackBehavioralMetric(eventName: eventNames, data?: EventPayload) { | ||
if (this.webex?.internal?.newMetrics) { | ||
this.webex.internal.newMetrics.submitBehavioralEvent({ | ||
product: 'wxcc_desktop', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need a wrapper , can you check the web client if they do it based on config for adding default values ?
@@ -201,6 +204,11 @@ export default class ContactCenter extends WebexPlugin implements IContactCenter | |||
|
|||
await loginResponse; | |||
|
|||
this.metricsManager.trackBehavioralMetric('agent-login', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all constnt to config file
COMPLETES #< INSERT LINK TO ISSUE >
This pull request addresses SPARK-572789
Add behavioral and operational metrics for cc sdk
by making the following changes
Added new MetricsManager class
Change Type
The following scenarios were tested
I certified that
I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly
Make sure to have followed the contributing guidelines before submitting.