Repository Quality: Telemetry Opt-In Correctness and Data Hygiene

[github-actions]

🎯 Repository Quality Improvement Report — Telemetry Opt-In Correctness and Data Hygiene

Analysis Date: 2026-08-03
Focus Area: Telemetry Opt-In Correctness and Data Hygiene
Strategy Type: Custom

Executive Summary

VSTest's telemetry architecture has several correctness and hygiene issues. The most impactful is a dual opt-in path: callers using the translation layer (VS IDE, Azure DevOps) can activate metrics collection by setting CollectMetrics = true in TestPlatformOptions, independently of the VSTEST_TELEMETRY_OPTEDIN environment variable (TestRequestManager.cs:1423). This means an IDE integration can collect telemetry even when users never set the env var.

Additionally, two telemetry fields (DataCollectorsEnabled and LoggerUsed) emit user-defined identifiers without sanitization. DataCollectorsEnabled uses an anonymous-type .ToString() call that produces strings like { FriendlyName = AcmeCodeCoverage, Uri = (acme/redacted) }. LoggerUsed calls string.Join<Type>() which resolves to Type.FullName — exposing custom logger class names. A third issue is that IsTelemetryOptedIn() is copy-pasted into ArtifactProcessingManager, and TextFileTelemetryPublisher has an inverted doc comment.

Full Analysis Report

Current State

Metric	Value	Status
Distinct telemetry events	3 (discovery, execution, attachments)	✅
IsTelemetryOptedIn() implementations	2 (duplicated)	⚠️
Opt-in paths	2 (env var + CollectMetrics)	⚠️
User-defined strings in telemetry	2 fields	⚠️
Misleading XML doc comments	1	⚠️

Strengths

• NoOpMetricsCollection null-object pattern ensures zero CPU cost for opted-out callers.
• CommandLineSwitches only records the presence of known switches, not their values.
• TargetDevice / TargetOS are sanitized to enum-like strings.
• TelemetryReporter correctly gates on IsTelemetryOptedIn before IPC transmission.

Issues Found

1. Dual opt-in path — GetRequestData() enables collection when _telemetryOptedIn || IsTelemetryOptedIn(). _telemetryOptedIn is set from TestPlatformOptions.CollectMetrics by the caller (IDE), not the user.

2. DataCollectorsEnabled leaks user-defined names — ProxyDataCollectionManager.cs:402 uses new { x.FriendlyName, x.Uri }.ToString(). Anonymous-type .ToString() produces { FriendlyName = UserDefined, Uri = ... }.

3. LoggerUsed sends custom Type.FullName — TestLoggerManager.cs:189 calls string.Join<Type>(...) which invokes Type.ToString() = FullName. Custom loggers expose company-internal type names.

4. Duplicated IsTelemetryOptedIn() — Identical private static method exists in both TestRequestManager and ArtifactProcessingManager.

5. Inverted doc comment on TextFileTelemetryPublisher — Summary says "initialized if Telemetry is opted out" but it is active when opted IN with VSTEST_LOGTELEMETRY=1.

---

🤖 Suggested Improvement Tasks

Task 1: Document and test the dual telemetry opt-in contract

Priority: Medium | Effort: Small

Add XML docs to GetRequestData() explaining that _telemetryOptedIn reflects caller/IDE consent and IsTelemetryOptedIn() reflects user env-var consent. Add a unit test asserting that CollectMetrics = true activates metrics collection even without the env var set.

File: src/vstest.console/TestPlatformHelpers/TestRequestManager.cs

---

Task 2: Sanitize DataCollectorsEnabled to avoid user-defined friendly names

Priority: Medium | Effort: Small

Replace the anonymous-type .ToString() with only the URI (a stable registered identifier):

// Before (ProxyDataCollectionManager.cs ~402)
var dataCollectors = enabledDataCollectors.Select(x => new { x.FriendlyName, x.Uri }.ToString());

// After
var dataCollectors = enabledDataCollectors.Select(x => x.Uri?.ToString() ?? string.Empty);

File: src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/ProxyDataCollectionManager.cs

---

Task 3: Sanitize LoggerUsed to avoid emitting custom Type.FullName

Priority: Medium | Effort: Small

TestLoggerManager.cs:189 emits custom logger type names. Replace with an assembly-boundary check that anonymizes unknown loggers:

// After
var loggerNames = _initializedLoggers.Select(t =>
    t.Assembly == typeof(TestLoggerManager).Assembly ? t.FullName! : "<custom>");
_requestData.MetricsCollection.Add(TelemetryDataConstants.LoggerUsed, string.Join(",", loggerNames));

File: src/Microsoft.TestPlatform.CrossPlatEngine/Client/TestLoggerManager.cs

---

Task 4: Deduplicate IsTelemetryOptedIn() into a shared helper

Priority: Low | Effort: Small

Extract the duplicated method into TelemetryDataConstants (or a new TelemetryHelper) in Microsoft.TestPlatform.Common:

internal static bool IsTelemetryOptedIn()
    => Environment.GetEnvironmentVariable("VSTEST_TELEMETRY_OPTEDIN")?.Equals("1", StringComparison.Ordinal) == true;

Update call sites in TestRequestManager.cs:1338 and ArtifactProcessingManager.cs:252.

---

Task 5: Fix TextFileTelemetryPublisher doc comment and NoOpMetricsCollection visibility

Priority: Low | Effort: Small

5a. Fix the inverted summary in TextFileTelemetryPublisher:

"This class will be initialized if Telemetry is opted out." → "Writes telemetry events to a local text file. Active only when telemetry is opted in AND VSTEST_LOGTELEMETRY=1."

5b. NoOpMetricsCollection is public but does not appear in any PublicAPI.Shipped.txt. Change to internal to reduce unnecessary API surface.

Files: src/vstest.console/Publisher/TextFileTelemetryPublisher.cs, src/Microsoft.TestPlatform.Common/Telemetry/NoOpMetricsCollection.cs

---

📊 Historical Context

Last 5 focus areas

Date	Focus Area	Type
2026-07-29	ipc-connection-resilience-timeout-arithmetic-correctness	Custom
2026-07-30	logger-concurrency-correctness-and-output-reliability	Custom
2026-07-31	testproperty-propertybag-type-safety-and-serialization-correctness	Custom
2026-08-03	telemetry-opt-in-correctness-and-data-hygiene	Custom

Next analysis: 2026-08-04 — Focus area selected based on diversity algorithm

Generated by Repository Quality Improver · 67.1 AIC · ⌖ 7.72 AIC · ⊞ 8.9K · ◷

• expires on Aug 5, 2026, 4:58 AM UTC

[github-actions]

This issue was automatically closed because it expired on 2026-08-05T04:58:03.928Z.

Closed by Workflow