Skip to content

Commit 990a8ff

Browse files
feat: include git commit SHA in PET info response for telemetry regression tracking (#473)
## Background PET (Python Environment Tools) is a Rust-based JSONRPC server used by the VS Code Python extension to discover Python environments. PET does not send its own telemetry — instead, it returns metadata to the calling extension (vscode-python-environments), which forwards it via telemetry. To help catch regressions, we need a way to know exactly which PET build a user is running by querying telemetry. [PR #470](#470) added a JSONRPC `info` request that returns PET's version and an optional CI build ID. However, the build number alone isn't enough to pinpoint the exact source code — we also need the git commit hash. This PR extends the `info` response to include the **git commit SHA** baked into the binary at build time. On the extension side, the companion PR ([vscode-python-environments#1548](microsoft/vscode-python-environments#1548)) passes this information along to telemetry, so we can correlate user-reported issues or regressions back to the specific PET commit. ## What Changed ### 1. Build script (`crates/pet/build.rs`) - Added support for three new environment variables that CI systems set automatically: - `PET_COMMIT_SHA` — explicit override (highest priority) - `BUILD_SOURCEVERSION` — set by Azure Pipelines - `GITHUB_SHA` — set by GitHub Actions - When any of these is present and non-empty, the value is embedded into the binary as a compile-time constant via `cargo:rustc-env=PET_COMMIT_SHA=...`. ### 2. JSONRPC response (`crates/pet/src/jsonrpc.rs`) - Added a new optional `commit_sha` field to the `InfoResponse` struct. - `InfoResponse::current()` populates it from the compile-time `PET_COMMIT_SHA` env var (same pattern used for `build_id`). - For local dev builds where no CI env var is set, the field is `None` and omitted from the JSON response. ### 3. JSONRPC documentation (`docs/JSONRPC.md`) - Updated the `InfoResponse` TypeScript interface to document the new `commitSha?: string` field, including where the value is sourced from. ### 4. Tests (`crates/pet/src/jsonrpc.rs`, `crates/pet/tests/jsonrpc_server_test.rs`, `crates/pet/tests/jsonrpc_client.rs`) - Added `commit_sha` field to the test client's `PetInfoResponse` struct. - Added assertions that `commit_sha`, when present, is non-empty (mirrors existing `build_id` assertion pattern). - Added clarifying comments explaining that both `build_id` and `commit_sha` are `None` in local dev builds and only populated in CI. ## How It Works ``` CI build (Azure Pipelines / GitHub Actions) ↓ sets BUILD_SOURCEVERSION or GITHUB_SHA build.rs captures it → bakes into binary as PET_COMMIT_SHA ↓ JSONRPC `info` request → returns { petVersion, buildId, commitSha } ↓ vscode-python-environments extension → includes commitSha in telemetry ↓ Team queries telemetry → correlates regressions to exact PET commit ``` ## Related PRs - [PET PR #470](#470) — Added the `info` JSONRPC request with version and build ID (merged) - [vscode-python-environments PR #1548](microsoft/vscode-python-environments#1548) — Extension-side: passes PET version/build/commit info to telemetry
1 parent 7a432f1 commit 990a8ff

5 files changed

Lines changed: 80 additions & 3 deletions

File tree

crates/pet/build.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,42 @@
44
fn main() {
55
println!("cargo:rerun-if-env-changed=PET_BUILD_ID");
66
println!("cargo:rerun-if-env-changed=BUILD_BUILDID");
7+
println!("cargo:rerun-if-env-changed=PET_COMMIT_SHA");
8+
println!("cargo:rerun-if-env-changed=BUILD_SOURCEVERSION");
9+
println!("cargo:rerun-if-env-changed=GITHUB_SHA");
710

11+
// Filter empties per-candidate so an explicitly-set-but-empty primary var
12+
// doesn't short-circuit the fallback chain (e.g., `PET_BUILD_ID=""`).
813
if let Some(build_id) = std::env::var("PET_BUILD_ID")
914
.ok()
10-
.or_else(|| std::env::var("BUILD_BUILDID").ok())
1115
.filter(|value| !value.is_empty())
16+
.or_else(|| {
17+
std::env::var("BUILD_BUILDID")
18+
.ok()
19+
.filter(|value| !value.is_empty())
20+
})
1221
{
1322
println!("cargo:rustc-env=PET_BUILD_ID={build_id}");
1423
}
1524

25+
// BUILD_SOURCEVERSION is set by Azure Pipelines; GITHUB_SHA by GitHub Actions.
26+
if let Some(commit_sha) = std::env::var("PET_COMMIT_SHA")
27+
.ok()
28+
.filter(|value| !value.is_empty())
29+
.or_else(|| {
30+
std::env::var("BUILD_SOURCEVERSION")
31+
.ok()
32+
.filter(|value| !value.is_empty())
33+
})
34+
.or_else(|| {
35+
std::env::var("GITHUB_SHA")
36+
.ok()
37+
.filter(|value| !value.is_empty())
38+
})
39+
{
40+
println!("cargo:rustc-env=PET_COMMIT_SHA={commit_sha}");
41+
}
42+
1643
#[cfg(target_os = "windows")]
1744
{
1845
if std::env::var("CARGO_BIN_NAME").is_err() {

crates/pet/src/jsonrpc.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ pub struct InfoResponse {
523523
pub pet_version: String,
524524
#[serde(skip_serializing_if = "Option::is_none")]
525525
pub build_id: Option<String>,
526+
#[serde(skip_serializing_if = "Option::is_none")]
527+
pub commit_sha: Option<String>,
526528
}
527529

528530
impl InfoResponse {
@@ -532,6 +534,9 @@ impl InfoResponse {
532534
build_id: option_env!("PET_BUILD_ID")
533535
.filter(|value| !value.is_empty())
534536
.map(ToString::to_string),
537+
commit_sha: option_env!("PET_COMMIT_SHA")
538+
.filter(|value| !value.is_empty())
539+
.map(ToString::to_string),
535540
}
536541
}
537542
}
@@ -1537,14 +1542,45 @@ mod tests {
15371542
}
15381543

15391544
#[test]
1540-
fn test_info_response_uses_package_version_and_optional_build_id() {
1545+
fn test_info_response_uses_package_version_and_optional_build_metadata() {
15411546
let info = InfoResponse::current();
15421547

15431548
assert_eq!(info.pet_version, env!("CARGO_PKG_VERSION"));
1549+
// build_id / commit_sha are populated from env vars set at compile time by CI.
1550+
// For local dev builds they will be None; assert non-empty only when present.
15441551
assert!(info
15451552
.build_id
15461553
.as_deref()
15471554
.is_none_or(|build_id| !build_id.is_empty()));
1555+
assert!(info
1556+
.commit_sha
1557+
.as_deref()
1558+
.is_none_or(|commit_sha| !commit_sha.is_empty()));
1559+
}
1560+
1561+
#[test]
1562+
fn test_info_response_serializes_camel_case_and_omits_none() {
1563+
// Guards the JSON wire format (camelCase rename + skip_serializing_if)
1564+
// independently of whether CI env vars were set at compile time.
1565+
let json = serde_json::to_value(InfoResponse {
1566+
pet_version: "1.2.3".to_string(),
1567+
build_id: Some("42".to_string()),
1568+
commit_sha: Some("abc123".to_string()),
1569+
})
1570+
.unwrap();
1571+
assert_eq!(json["petVersion"], "1.2.3");
1572+
assert_eq!(json["buildId"], "42");
1573+
assert_eq!(json["commitSha"], "abc123");
1574+
1575+
let json = serde_json::to_value(InfoResponse {
1576+
pet_version: "1.2.3".to_string(),
1577+
build_id: None,
1578+
commit_sha: None,
1579+
})
1580+
.unwrap();
1581+
assert_eq!(json["petVersion"], "1.2.3");
1582+
assert!(json.get("buildId").is_none());
1583+
assert!(json.get("commitSha").is_none());
15481584
}
15491585

15501586
#[test]

crates/pet/tests/jsonrpc_client.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ pub struct RefreshResult {
2626
pub struct PetInfoResponse {
2727
pub pet_version: String,
2828
pub build_id: Option<String>,
29+
pub commit_sha: Option<String>,
2930
}
3031

3132
#[derive(Debug, Clone, Deserialize, PartialEq, Eq)]

crates/pet/tests/jsonrpc_server_test.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,16 +111,22 @@ fn assert_single_environment(
111111
}
112112

113113
#[test]
114-
fn info_reports_pet_version_and_optional_build_id() {
114+
fn info_reports_pet_version_and_optional_build_metadata() {
115115
let client = PetJsonRpcClient::spawn().expect("failed to spawn PET server");
116116

117117
let info = client.info().expect("info request failed");
118118

119119
assert_eq!(info.pet_version, env!("CARGO_PKG_VERSION"));
120+
// build_id / commit_sha are populated from env vars set at compile time by CI.
121+
// For local dev builds they will be None; assert non-empty only when present.
120122
assert!(info
121123
.build_id
122124
.as_deref()
123125
.is_none_or(|build_id| !build_id.is_empty()));
126+
assert!(info
127+
.commit_sha
128+
.as_deref()
129+
.is_none_or(|commit_sha| !commit_sha.is_empty()));
124130
}
125131

126132
#[test]

docs/JSONRPC.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,15 @@ interface InfoResponse {
3535
petVersion: string;
3636
/**
3737
* Build identifier baked into the binary when built by CI.
38+
* Sourced from `PET_BUILD_ID` or Azure Pipelines `BUILD_BUILDID`.
3839
*/
3940
buildId?: string;
41+
/**
42+
* Source git commit SHA baked into the binary when built by CI.
43+
* Sourced from `PET_COMMIT_SHA`, Azure Pipelines `BUILD_SOURCEVERSION`,
44+
* or GitHub Actions `GITHUB_SHA`. Absent for local dev builds.
45+
*/
46+
commitSha?: string;
4047
}
4148
```
4249

0 commit comments

Comments
 (0)