#CX-23917 : Use pre-login API on JS sdk for pre-login metrics#4795
#CX-23917 : Use pre-login API on JS sdk for pre-login metrics#4795Hu4k3n wants to merge 3 commits intowebex:nextfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04a3ab1500
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| key: name, | ||
| client_timestamp: new Date().toISOString(), | ||
| context: this.getContext(), | ||
| timestamp: new Date().getTime(), | ||
| browserDetails: this.getBrowserDetails(), | ||
| ...metadata, |
There was a problem hiding this comment.
submitPreLoginEvent() treats metadata as caller-controlled input, but buildEvent() spreads it after key, client_timestamp, context, and browserDetails. In any JS caller (or TS caller using as any), values like metadata.key or metadata.client_timestamp will silently replace the SDK-generated fields, causing the event to be attributed to the wrong metric or to carry an invalid timestamp. This regression is new to the pre-login path because the old code only merged metadata into eventPayload.value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0822ab70a8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| eventPayload: { | ||
| metricName, | ||
| browserDetails: this.getBrowserDetails(), | ||
| ...metadata, |
There was a problem hiding this comment.
originTime before spreading pre-login metadata
Because metadata is now copied directly onto eventPayload, a caller can pass metadata.originTime with any truthy scalar (this is valid under EventPayload). PreLoginMetricsBatcher.prepareRequest() later does item.eventPayload.originTime = item.eventPayload.originTime || {}; item.eventPayload.originTime.sent = ... in packages/@webex/internal-plugin-metrics/src/prelogin-metrics-batcher.ts, which will throw in module/strict mode when originTime is a string or boolean. Before this change, metadata stayed under eventPayload.value, so arbitrary metadata keys could not break batching.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Should the extra meta data not be inside 'value' anyway?
Maybe we should look at an existing prelogin metric from webex to check?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efbe281bfd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| eventPayload: { | ||
| metricName, | ||
| browserDetails: this.getBrowserDetails(), | ||
| ...metadata, |
There was a problem hiding this comment.
Spreading caller metadata directly into eventPayload lets a valid EventPayload like metadata.event = true escape into the batch item shape. In paired-device environments, prepareDiagnosticMetricItem() later writes item.eventPayload.event.pairingState/pairedDevice, which throws when event is a scalar, causing pre-login metric submission to fail for that request. Before this change, metadata stayed under eventPayload.value, so user metadata could not collide with the batcher’s internal event object.
Useful? React with 👍 / 👎.
3 participants
COMPLETES
https://jira-eng-sjc12.cisco.com/jira/browse/CX-23917
This pull request addresses
< DESCRIBE THE CONTEXT OF THE ISSUE >
The keys for the business event schema were written wrong. this is fixing it as part of the pre login metrics story
by making the following changes
updating the buisiness event parameters
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.