Skip to content

Don't redact ABExp Contact#308789

Merged
lramos15 merged 1 commit intomainfrom
lramos15/alright-tuna
Apr 9, 2026
Merged

Don't redact ABExp Contact#308789
lramos15 merged 1 commit intomainfrom
lramos15/alright-tuna

Conversation

@lramos15
Copy link
Copy Markdown
Member

@lramos15 lramos15 commented Apr 9, 2026

Fix #308788

Copilot AI review requested due to automatic review settings April 9, 2026 14:07
@lramos15 lramos15 enabled auto-merge (squash) April 9, 2026 14:07
@lramos15 lramos15 self-assigned this Apr 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes telemetry PII-cleaning unintentionally redacting experimentation assignment context (e.g. when assignment strings contain substrings like key/token), which can break experimentation analysis and targeting.

Changes:

  • Mark experiment/ABExp assignment context as “trusted” so it bypasses the generic secret redaction logic.
  • Update Copilot telemetry plumbing to propagate trusted values through shared properties and sender APIs.
  • Tighten internal-event sending behavior for Copilot’s MSFT telemetry sender.
Show a summary per file
File Description
src/vs/platform/telemetry/common/telemetryService.ts Wraps experiment properties in TelemetryTrustedValue so PII cleaning won’t redact ABExp context.
extensions/copilot/src/platform/telemetry/common/msftTelemetrySender.ts Removes a redundant postEvent helper and ensures internal telemetry is gated on internal status.
extensions/copilot/src/platform/telemetry/common/baseTelemetryService.ts Stores abexp.assignmentcontext as a trusted value and adjusts how experimentation events are forwarded to senders.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/platform/telemetry/common/telemetryService.ts:136

  • setExperimentProperty now wraps all experiment properties in TelemetryTrustedValue to bypass cleanData redaction. There are existing unit tests for TelemetryService but none that assert experiment properties containing substrings like "token"/"key" are preserved (the motivating regression in #308788). Please add a test (e.g. in src/vs/platform/telemetry/test/browser/telemetryService.test.ts) that sets an experiment property with such a value, logs an event, and verifies the appender receives the unredacted assignment context.
	setExperimentProperty(name: string, value: string): void {
		this._experimentProperties[name] = new TelemetryTrustedValue(value);

		// On first call, flush all pending events that were buffered waiting for experiment properties
		if (!this._isExperimentPropertySet) {
			this._flushPendingEvents();
		}
	}
  • Files reviewed: 3/3 changed files
  • Comments generated: 1

@lramos15 lramos15 merged commit 558b5e5 into main Apr 9, 2026
27 checks passed
@lramos15 lramos15 deleted the lramos15/alright-tuna branch April 9, 2026 14:34
@vs-code-engineering vs-code-engineering bot added this to the 1.116.0 milestone Apr 9, 2026
joshspicer pushed a commit that referenced this pull request Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Abexp context should be exempt from PII cleaning

3 participants