adds support for sending PDP Events via o11y @W-21179952#469
Conversation
There was a problem hiding this comment.
LGTM. I initially expected sendPdpEvent to live in plugin-telemetry, but given that PdpEvent is feature-agnostic, keeping it in the telemetry library also makes sense. I’d also suggest incorporating the change to sendTelemetryEventWithSchema, as this would improve schema handling more generally and support other events that rely on a defined o11ySchema. I’ve added a code snippet suggestions for sendTelemetryEventWithSchema that you can consider applying.
Separately, we need to update @salesforce/o11y-reporter to the latest version that includes the fix for logEventWithSchema, which hasn’t yet been published due to the npm token issue. @shetzel , I’ve followed up with you separately on this. Before merging this PR, we should bump to the updated @salesforce/o11y-reporter version that contains the fix.
| const merged = { ...this.commonProperties, ...attributes }; | ||
|
|
||
| const eventData: { [key: string]: unknown } = { | ||
| eventName: `${this.extensionName}/${eventName}`, | ||
| ...merged, | ||
| }; |
There was a problem hiding this comment.
| const eventData: { [key: string]: unknown } = { ...attributes }; |
There was a problem hiding this comment.
Just event specific attributes confined to the schema.
There was a problem hiding this comment.
We could also bring in zod, build a zod schema from the o11y schema passed in, then parse the event data with the zod schema to ensure it conforms. This seems like a separate effort though.
There was a problem hiding this comment.
The schema we pass in is the O11y encoding schema (from o11y_schema), not a JSON Schema, so we can’t reliably derive a zod schema from it here. Callers should validate attributes (e.g. with zod) before calling, using the same schema context they use when importing the schema.
| const merged = { ...this.commonProperties, ...attributes }; | ||
|
|
||
| const eventData: { [key: string]: unknown } = { | ||
| eventName: `${this.extensionName}/${eventName}`, | ||
| ...merged, | ||
| }; |
There was a problem hiding this comment.
Just event specific attributes confined to the schema.
| * Populate this if there is a unique component with your Event for which a distinct count would be a relevant metric | ||
| * E.g., campaignID → count the number of campaigns that were interacted with. | ||
| */ | ||
| componentId?: string; |
There was a problem hiding this comment.
do we have any opinions on how dx should use these props, for consistency?
There was a problem hiding this comment.
Only this recommendation from the canvas. I'll update the doc with it.
CLI plugin command name (<pluginName.commandName>) or ext command name
src/types.ts
Outdated
| * action = Action the user takes in past tense. This should only be ONE word, in lower case | ||
| * Examples: processed, selected, sent, saved | ||
| */ | ||
| eventName: string; |
There was a problem hiding this comment.
would you want it typed as ${string}.{string} or something tighter? Same idea with productFeatureId being aJC{$string}
|
@shetzel Please update the @salesforce/o11y-reporter to the latest version : v1.7.3 |
3 participants
What does this PR do?
adds support for sending PDP Events via o11y.
adds dependency on o11y_schema package to use the
pdpEventSchema.What issues does this PR fix or reference?
@W-21179952@