Skip to content

fix(internal-plugin-metrics): log call diagnostic milestones #5045

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
maxinteger merged 2 commits into from
Jul 9, 2026
+32 −0
Merged

fix(internal-plugin-metrics): log call diagnostic milestones #5045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dec15d1
fix(internal-plugin-metrics): log call diagsnotc milestons
maxinteger Jun 24, 2026
4983b4f
fix(internal-plugin-metrics): log call diagsnotc milestons
maxinteger Jul 1, 2026
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
5 changes: 5 additions & 0 deletions packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,11 @@ export const prepareDiagnosticMetricItem = (webex: any, item: any) => {

item.eventPayload.origin = Object.assign(origin, item.eventPayload.origin);

// Mark call milestones in logs for easier filtering and analysis
if (eventName) {
webex.logger.log('Milestone,CallDiagnostic', eventName);
Comment on lines +394 to +395

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Log milestones once per metric

When a call-diagnostic metrics POST hits a transient NetworkOrCORSError, the default metrics batcher retries by calling rerequest(), which invokes this batcher's prepareItem() again before re-enqueueing the same item. Because the milestone log is emitted inside prepareDiagnosticMetricItem(), a single client event can produce repeated Milestone,CallDiagnostic entries on each retry, seconds apart, making the client-log milestone timeline misleading; guard this per item or move the mark to the point where the event is originally created.

Useful? React with 👍 / 👎.

}

webex.logger.log(
`CallDiagnosticLatencies,prepareDiagnosticMetricItem: ${JSON.stringify({
latencies: Object.fromEntries(cdl.latencyTimestamps),
Expand Down
27 changes: 27 additions & 0 deletions ...ex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,33 @@ describe('internal-plugin-metrics', () => {
});
});

it('logs a Milestone entry when event name is present', () => {
const logSpy = sinon.spy(webex.logger, 'log');
const eventName = 'client.call.initiated';
const eventPayload = {event: {name: eventName}};

prepareDiagnosticMetricItem(webex, {eventPayload, type: ['diagnostic-event']});

assert.isTrue(
logSpy.calledWith('Milestone,CallDiagnostic', eventName),
Comment thread
dominiccarrington marked this conversation as resolved.
'expected logger.log to be called with Milestone,CallDiagnostic and the event name'
Comment on lines +470 to +472

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fix mismatched milestone log assertion

The implementation added above calls logger.log() with one formatted string (Milestone,CallDiagnostic: ${eventName}), but this assertion expects two separate arguments. In the internal-plugin-metrics unit suite this test is therefore always false even when the milestone log is emitted, so CI will fail until the assertion matches the actual call shape or the production log call is changed to pass two arguments.

Useful? React with 👍 / 👎.

);
sinon.restore();
});

it('does not log a Milestone entry when event name is absent', () => {
const logSpy = sinon.spy(webex.logger, 'log');
const eventPayload = {event: {}};

prepareDiagnosticMetricItem(webex, {eventPayload, type: ['diagnostic-event']});

assert.isFalse(
logSpy.calledWith('Milestone,CallDiagnostic', sinon.match.any),
'expected logger.log not to be called with Milestone,CallDiagnostic when event name is absent'
);
sinon.restore();
});

it('sets buildType and upgradeChannel correctly', () => {
const item: any = {
eventPayload: {
Expand Down