Skip to content

Add msgo/module and msgo/ci telemetry counters#2283

Open
gdams wants to merge 4 commits into
microsoft/mainfrom
dev/gadams/telemetry
Open

Add msgo/module and msgo/ci telemetry counters#2283
gdams wants to merge 4 commits into
microsoft/mainfrom
dev/gadams/telemetry

Conversation

@gdams
Copy link
Copy Markdown
Member

@gdams gdams commented May 18, 2026

Summary

Add two new Microsoft-specific telemetry counters to incrementConfig() in telemetrystats.go, and vendor the updated go-infra/telemetry dependency (which includes the new DetectCI function).

fixes: https://github.com/microsoft/go-lab/issues/382

New counters

msgo/module:<module_path>

Reports the Go module path (from go.mod) of the project being built.

This counter is gated behind the MS_GO_INTERNAL_CUSTOMER environment variable and is only collected for first-party (1P) Microsoft customers who have explicitly opted in. It is not collected for general users of the Microsoft build of Go.

msgo/ci:<system>

Detects the CI system in use by inspecting environment variables via telemetry.DetectCI(os.Environ()). Reports values like azdo, github, gitlab, appveyor, etc. Only incremented when a CI system is detected.

Dependency changes

  • Updated vendored github.com/microsoft/go-infra/telemetry to include the new ci.go file (DetectCI function) and updated config.json.

Patch changes

  • Patch 0001 (Vendor external dependencies): Updated go-infra/telemetry dependency version, added ci.go to vendor.
  • Patch 0009 (Add appinsights telemetry): Added the two new counters and their required imports.

Copilot AI review requested due to automatic review settings May 18, 2026 13:56
@gdams gdams requested a review from a team as a code owner May 18, 2026 13:56
@gdams gdams changed the title Add msgo/module, msgo/ci, and msgo/systemcrypto telemetry counters Add msgo/module and msgo/ci telemetry counters May 18, 2026
@gdams gdams force-pushed the dev/gadams/telemetry branch from 157fe41 to b3ed91d Compare May 18, 2026 14:03
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

Note

Copilot was unable to run its full agentic suite in this review.

Adds three Microsoft-specific telemetry counters (msgo/module, msgo/ci, msgo/systemcrypto) to the Microsoft build of Go, and vendors the updated go-infra/telemetry dependency providing the new DetectCI function.

Changes:

  • Add three new counters in incrementConfig() with msgo/module gated behind MS_GO_INTERNAL_CUSTOMER env var.
  • Vendor updated github.com/microsoft/go-infra/telemetry with new ci.go (DetectCI).
  • Update config.json to register the new counter names.
Show a summary per file
File Description
patches/0009-Add-appinsights-telemetry.patch Adds the three new counters to telemetrystats.go with required imports.
patches/0001-Vendor-external-dependencies.patch Vendors new ci.go, bumps dependency version, updates config.json.
go Bumps the go submodule commit.

Copilot's findings

Comments suppressed due to low confidence (4)

patches/0001-Vendor-external-dependencies.patch:1

  • Typo in counter name: mssgo/systemcrypto should be msgo/systemcrypto to match the actual counter incremented in telemetrystats.go. As-is, the counter emitted by the code will not match any registered name in config.json.
    patches/0001-Vendor-external-dependencies.patch:1
  • The Jenkins detection (BUILD_ID + BUILD_URL) is checked before Google Cloud Build, but BUILD_ID is also set by Google Cloud Build. More importantly, the Google Cloud Build check is unreachable if Jenkins's BUILD_URL happens to be set, and conversely many GCB builds where PROJECT_ID is exported alongside an environment that has BUILD_URL could be misattributed. Consider checking GCB-unique variables (e.g. BUILDER_OUTPUT) and reordering / using more specific markers for each system to reduce false positives.
    patches/0001-Vendor-external-dependencies.patch:1
  • The PR description mentions the counter as msgo/ci, but no entry for msgo/module:* review is needed since it requires module path values. Verify that msgo/module:* is acceptable in config.json schema (free-form wildcard counter) — if the upload schema requires enumerated values, * may not be honored.
    patches/0001-Vendor-external-dependencies.patch:1
  • AWS_REGION is commonly set in many AWS environments (not just CodeBuild). CODEBUILD_BUILD_ID alone is sufficient and CodeBuild-specific; the extra AWS_REGION requirement risks false negatives where a CodeBuild runner doesn't export AWS_REGION. Consider dropping the AWS_REGION check.
  • Files reviewed: 2/2 changed files
  • Comments generated: 3

Comment thread patches/0009-Add-appinsights-telemetry.patch Outdated
Comment thread patches/0009-Add-appinsights-telemetry.patch Outdated
Comment thread patches/0009-Add-appinsights-telemetry.patch
Add three new Microsoft-specific telemetry counters:

- msgo/module: Reports the Go module path of the project being built.
  This counter is only collected when the MS_GO_INTERNAL_CUSTOMER
  environment variable is set, restricting it to first-party (1P)
  Microsoft customers who have explicitly opted in.

- msgo/ci: Detects and reports the CI system in use (e.g. azdo, github,
  gitlab) using the go-infra telemetry.DetectCI function, which inspects
  environment variables to identify the CI platform.

- msgo/systemcrypto: Reports whether system crypto (e.g. OpenSSL,
  CNG) is enabled or disabled on platforms that support it.

Also vendors the updated go-infra/telemetry dependency which includes
the new DetectCI function (ci.go) and updated config.json.
Comment thread patches/0009-Add-appinsights-telemetry.patch Outdated
Copy link
Copy Markdown
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

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

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.

4 participants