-
Notifications
You must be signed in to change notification settings - Fork 255
[main] enhance exception telemetry with optional script Information #2388
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
Conversation
| * [Optional] If set to true, when exception is sent out, the SDK will also send out all scripts basic info that are loaded on the page. | ||
| * Notice: This would increase the size of the exception telemetry. | ||
| */ | ||
| getExceptionScriptsInfo?: boolean; |
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.
Rather than "get" (which to me sounds like a function) we want to call out that this is an option, which is where we have in the past called these "enableXXXX" or "disableXXXX" in this case maybe something like "includeXXX" and as we are also adding the debug logs and maybe some other stuff for the internal project, perhaps we call these out into an exception config (like the Cookie manager config)
So something like
expCfg: {
inclScripts: true/false,
getDbgLogs: () => string[]
}We can call the interface name for this something meaningful like IExceptionConfig as I suspect that we are going to need additional optional "values" that partners will need to include.
| export function findAllScripts(doc: any) { | ||
| let scripts = doc.getElementsByTagName("script"); | ||
| let result: scriptsInfo[] = []; | ||
| for (let i = 0; i < scripts.length; i++) { |
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.
Lets use arrForEach to help with minification
| crossOrigin?: string; | ||
| async?: boolean; | ||
| defer?: boolean; | ||
| referrePolicy?: string; |
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.
typo, missing an 'r' 😄
No description provided.