debug: filter exceptions from DAP telemetry on web #97627
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change will filter DAP keys beginning with `!` when the remote authority is "other", which SteVen recommended as a way to tell if we're on web/codespaces. Isidor mentioned previously that there was filtering for keys beginning with an underscore, but this was only applied to some data in error responses, not general telemetry events. An alternative approach to this is filtering exceptions based on the environment on the extension side, but I figured it would be better to have VS Code do that so that we don't end up with N many possibly deviant sets of environment detection logic.
isidorn
approved these changes
May 12, 2020
| @@ -848,7 +848,17 @@ export class DebugSession implements IDebugSession { | |||
| // and the user opted in telemetry | |||
| if (this.raw.customTelemetryService && this.telemetryService.isOptedIn) { | |||
| // __GDPR__TODO__ We're sending events in the name of the debug extension and we can not ensure that those are declared correctly. | |||
| this.raw.customTelemetryService.publicLog(event.body.output, event.body.data); | |||
| let data = event.body.data; | |||
There was a problem hiding this comment.
Does it make sense to move this out to a helper method which would live in debugTelemetry.ts?
There was a problem hiding this comment.
The DebugTelemetry class isn't very accessible here; that's used for the top-level of the session and sends to the global(?) telemetry service, and for this bit of telemetry we're sending to a separate instance of the service created from the aiKey inside the debug extension's package.json.
So perhaps not a method, but I did pop this to its own helper function in debugUtils.
|
Looks good, great job 👏 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change will filter DAP keys beginning with
!when the remoteauthority is "other", which SteVen recommended as a way to tell if we're
on web/codespaces.
Isidor mentioned previously that there was filtering for keys beginning
with an underscore, but this was only applied to some data in error
responses, not general telemetry events. An alternative approach to this
is filtering exceptions based on the environment on the extension side,
but I figured it would be better to have VS Code do that so that we
don't end up with N many possibly deviant sets of environment detection
logic.