Fix PET timeout telemetry attribution

[Karthik Nadig (karthiknadig)]

Summary

Correct PET/setup telemetry so timeout investigations retain numeric context and reliable binary attribution.

• send refresh, configure, and restart counts through telemetry measurements
• classify PET JSON-RPC timeouts by method and preserve refresh failure context
• retry transient PET info timeouts while retaining last known build metadata across restarts
• regression-test setup stage duration, timeout categories, refresh measures, and retry bounds

Validation

• npm run lint
• npm run compile-tests
• npm run unittest (1500 passing, 5 pending)
• npm run compile

Fixes microsoft/python-environment-tools#478

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR adjusts PET/setup telemetry so timeout investigations keep numeric measurement context and more accurate timeout attribution (including JSON-RPC method classification), while improving resilience of PET info attribution across restarts.

Changes:

• Move refresh/configure/restart numeric context into telemetry measurements (and add refresh measurement builder helper).
• Classify RpcTimeoutError by JSON-RPC method and update telemetry “timeout vs error” result attribution.
• Retry transient PET info timeouts with bounded attempts and preserve last-known build metadata across restarts; add unit tests for these behaviors.

Show a summary per file

File	Description
src/managers/common/nativePythonFinder.ts	Adds refresh measurement builder + RPC-timeout retry helper; updates PET telemetry payloads, timeout attribution, and info fetch retry/persistence.
src/common/telemetry/errorClassifier.ts	Updates timeout classification to be method-specific for RpcTimeoutError.
src/common/telemetry/constants.ts	Adjusts telemetry property mappings to reflect movement of numeric fields into measurements.
src/test/managers/common/nativePythonFinder.telemetry.unit.test.ts	Adds unit tests for refresh measurement building and RPC-timeout retry behavior.
src/test/common/telemetry/sender.unit.test.ts	Adds regression test asserting sender passes durations as measurements.
src/test/common/telemetry/errorClassifier.unit.test.ts	Updates tests to validate method-specific RPC timeout categorization.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (4)

src/managers/common/petTelemetry.ts:46

• The conda detection relies on the magic string 'Conda', which can drift from the actual NativePythonEnvironmentKind values (e.g., casing changes or enum string values). Consider normalizing info.kind (e.g., lowercasing) and comparing against a single canonical value, or accepting both expected representations, so condaEnvCount remains stable if upstream naming changes.

    for (const info of input.nativeInfo) {
        if (info.tool) {
            managerCount++;
        } else {
            envCount++;
            if (info.kind === 'Conda') {
                condaEnvCount++;
            }
        }
    }

src/managers/common/nativePythonFinder.ts:777

• With this.petInfo no longer reset on start(), a newly started PET process can inherit stale build attribution if the PET binary is replaced in-place between restarts (same path, different build) and the info call fails (e.g., transient failures). To avoid incorrect attribution, consider invalidating petInfo when you can detect a binary change (e.g., by storing and comparing executable mtime/size, or clearing the cache when the failure indicates an older binary such as a method-not-found response).

        connection.listen();

        // Stamp PET telemetry with version/buildId/commitSha. Fire-and-forget — must not block refresh.
        this.kickoffInfoFetch(connection);

src/managers/common/nativePythonFinder.ts:973

• On refresh failures, locatorsJson (and any available locator timing context) is not included even when refreshPerf may have partial data. Since the success path includes locatorsJson, consider also attaching locatorsJson in the error path when refreshPerf is present to improve failure investigations without changing the numeric measures payload.

                {
                    result: ex instanceof RpcTimeoutError ? 'timeout' : 'error',
                    errorType,
                    ...this.getPetInfoProperties(),
                },

src/managers/common/nativePythonFinder.ts:294

• The new input-validation behavior for retryRpcTimeout (throwing RangeError when maxAttempts < 1 or non-integer) isn’t covered by the new unit tests. Add a unit test to assert the RangeError is thrown for invalid attempt limits (e.g., 0 and 1.5) so the contract stays stable.

    if (!Number.isInteger(maxAttempts) || maxAttempts < 1) {
        throw new RangeError('maxAttempts must be a positive integer');
    }

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite