Skip to content
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

Normalize stack traces in telemetry #6460

Closed
luabud opened this issue Jul 4, 2019 · 2 comments
Closed

Normalize stack traces in telemetry #6460

luabud opened this issue Jul 4, 2019 · 2 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug good first issue important Issue identified as high-priority

Comments

@luabud
Copy link
Member

luabud commented Jul 4, 2019

When we normalize stack traces to unify them and remove PII, we should also normalize file separators. i.e.. we're getting stack traces with \ and /:

at ChildProcess.module.exports.exec.c.once <pvsc>\out\client\extension.js:9:37440 at Object.onceWrapper <hidden>\events.js:273:13 at ChildProcess.emit <hidden>\events.js:182:13 at maybeClose <hidden>\child_process.js:961:16 at Process.ChildProcess._handle.onexit <hidden>\child_process.js:248:5

and

at <anonymous> at ChildProcess.module.exports.exec.c.once <pvsc>/out/client/extension.js:9:37440 at Object.onceWrapper <hidden>/events.js:273:13 at ChildProcess.emit <hidden>/events.js:182:13 at maybeClose <hidden>/child_process.js:961:16 at Process.ChildProcess._handle.onexit <hidden>/child_process.js:248:5

@luabud luabud added bug Issue identified by VS Code Team member as probable bug triage-needed Needs assignment to the proper sub-team labels Jul 4, 2019
@DonJayamanne
Copy link

Curious why is this a bug and what's the benefit of the change?

@luabud luabud added debt Covers everything internal: CI, testing, refactoring of the codebase, etc. and removed bug Issue identified by VS Code Team member as probable bug labels Jul 5, 2019
@DonJayamanne DonJayamanne added bug Issue identified by VS Code Team member as probable bug needs PR and removed debt Covers everything internal: CI, testing, refactoring of the codebase, etc. labels Jul 8, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Jul 8, 2019
@DonJayamanne DonJayamanne added triage-needed Needs assignment to the proper sub-team feature-* good first issue and removed triage-needed Needs assignment to the proper sub-team labels Jul 8, 2019
@brettcannon brettcannon added reason-preexisting important Issue identified as high-priority labels Jul 8, 2019
@DonJayamanne
Copy link

Prescribed Solution

  • Modify getStactTrace in src/client/telemetry/index.ts to ensure \ is replaced with / on windows.

@DonJayamanne DonJayamanne added this to the 2019 - July Sprint 14 milestone Jul 17, 2019
@DonJayamanne DonJayamanne self-assigned this Jul 31, 2019
@ghost ghost removed the needs PR label Aug 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug good first issue important Issue identified as high-priority
Projects
None yet
Development

No branches or pull requests

3 participants