Skip to content

Bump Microsoft.ApplicationInsights to 3.1.2 and add local telemetry export - #9725

Open
Evangelink wants to merge 6 commits into
mainfrom
dev/amauryleve/bump-appinsights
Open

Bump Microsoft.ApplicationInsights to 3.1.2 and add local telemetry export#9725
Evangelink wants to merge 6 commits into
mainfrom
dev/amauryleve/bump-appinsights

Conversation

@Evangelink

Copy link
Copy Markdown
Member

What

Bumps Microsoft.ApplicationInsights from 2.23.0 to 3.1.2 and adapts our telemetry extension to the 3.x API. Also adds a network-free local telemetry exporter so collected telemetry can be verified after the migration.

Fixes part of #7465.

Why

AppInsights 2.x was pinned because 3.x initially dropped netstandard2.0 (see ApplicationInsights-dotnet#3091). That restriction is gone — 3.1.2 ships netstandard2.0, net462, net8.0, net9.0, net10.0. 3.x is an OpenTelemetry shim (backed by Azure Monitor Exporter), which brings a modern, maintained System.Diagnostics.DiagnosticSource transitively.

Changes

  • Directory.Packages.props: Microsoft.ApplicationInsights 2.23.03.1.2. Removed the explicit System.Diagnostics.DiagnosticSource 6.0.0 pin — its purpose (override AI 2.x's unmaintained transitive 5.0.0) is obsolete; a modern version now flows transitively via Azure.Monitor.OpenTelemetry.ExporterAzure.Core. Keeping the pin caused an NU1605 downgrade error.
  • AppInsightTelemetryClient: 3.x removed the metrics parameter from TrackEvent. We now fold numeric measurements into the event's properties as invariant-culture strings and call the 2-arg overload. This keeps a single correlated, context-enriched customEvent and sidesteps the new OpenTelemetry metric-name syntax restriction (property keys have no such restriction, so no keys need renaming).
  • Local export: new LocalFileTelemetryClient, selected via TESTINGPLATFORM_TELEMETRY_LOCALEXPORTPATH through the existing ITelemetryClientFactory seam. When set, events are written to a file as JSON Lines instead of being shipped to AppInsights — the "local exporter" for verification, no network traffic. Documented in PACKAGE.md.
  • Telemetry schema version bumped 2021 (see below).

⚠️ Dashboard / Kusto query impact

In AI 2.x, numeric measurements were sent in customMeasurements. In 3.x they are folded into customDimensions (as strings). Dashboard queries that read numeric values must be updated, e.g. runStart timings, total ran/total passed counts, mstest.setting.*_timeout, parallelization_workers, etc.

To make the cutover queryable, the platform telemetry schema version (TelemetryProperties.VersionValue, emitted on every event as the telemetry version dimension) is bumped 2021. Dashboards can branch on it:

customEvents
| extend telemetryVersion = toint(customDimensions['telemetry version'])
| extend runStart = iff(telemetryVersion >= 21,
                        todouble(customDimensions['run start']),   // new: AI 3.x, folded into customDimensions
                        toreal(customMeasurements['run start']))    // old: AI 2.x

Testing

  • Telemetry project builds on all TFMs (netstandard2.0/net8.0/net9.0).
  • Microsoft.Testing.Extensions.UnitTests pass on net9.0 (612) and net472 (611), including 4 new LocalFileTelemetryClient/factory tests.
  • Runtime-smoke-verified that the real AI 3.x AppInsightTelemetryClient constructs, tracks, and flushes without throwing (throwaway test, removed).

Open questions for reviewers

  • Live ingestion under VS 2022 / old .NET SDK runtimes: 3.x loads on netstandard2.0/net462, but the actual push now goes through the heavier Azure.Monitor.OpenTelemetry.ExporterAzure.Core stack. Local export bypasses the network, so it does not validate real ingestion. A live smoke test against a (test) AI resource on those hosts is still needed before we're confident — this is the VS-2022 concern from Microsoft.Testing.Extensions.Telemetry blocked from upgrading to latest Microsoft.ApplicationInsights #7465.
  • Dependency surface: this pulls OpenTelemetry + Azure.Core into a shipping package on all TFMs. Acceptable?
  • Should we add a local-export acceptance test (asset that references the Telemetry extension, opts in, sets the export path, asserts the JSON-Lines output)? It would be the only end-to-end coverage of the live 3.x code path. Happy to add in this PR or a follow-up.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

…xport

Microsoft.ApplicationInsights 3.x is an OpenTelemetry shim whose TrackEvent
overload no longer accepts a metrics dictionary. AppInsightTelemetryClient now
folds numeric measurements into the event's properties (invariant-culture
strings) so telemetry stays a single correlated, context-enriched customEvent
and avoids the OpenTelemetry metric-name-syntax restriction. The obsolete
explicit System.Diagnostics.DiagnosticSource pin is removed (a modern version
now flows transitively via Azure.Monitor.OpenTelemetry.Exporter -> Azure.Core).

Adds a network-free local exporter: setting TESTINGPLATFORM_TELEMETRY_LOCALEXPORTPATH
writes collected events to a file as JSON Lines via LocalFileTelemetryClient,
selected through the existing ITelemetryClientFactory seam, so telemetry can be
verified locally after the bump.

Fixes part of #7465

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 09:00
…t change

Numeric measurements now land in customDimensions (folded into event properties)
rather than customMeasurements. Bumping the schema version stamped on every event
lets telemetry dashboards branch old vs new query shapes on the boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 upgrades Microsoft.ApplicationInsights from 2.23.0 to 3.1.2 in the Microsoft.Testing.Extensions.Telemetry MTP extension and adapts the code to the 3.x (OpenTelemetry-shim) API. Since 3.x removed the metrics parameter from TrackEvent, numeric measurements are now folded into the event's properties (i.e. customDimensions) as invariant-culture strings. It also introduces a network-free LocalFileTelemetryClient (selected via TESTINGPLATFORM_TELEMETRY_LOCALEXPORTPATH) that writes events to disk as JSON Lines for local verification, and removes the now-obsolete explicit System.Diagnostics.DiagnosticSource 6.0.0 pin.

Changes:

  • Bump Microsoft.ApplicationInsights to 3.1.2 and drop the explicit System.Diagnostics.DiagnosticSource pin (now flows transitively).
  • Adapt AppInsightTelemetryClient.TrackEvent to fold numeric metrics into event properties for the 3.x 2-arg overload.
  • Add opt-in LocalFileTelemetryClient + factory wiring via a new env var, with unit tests and PACKAGE.md docs.
Show a summary per file
File Description
Directory.Packages.props Bumps AppInsights to 3.1.2 and removes the DiagnosticSource pin/comment.
.../Microsoft.Testing.Extensions.Telemetry.csproj Removes the explicit System.Diagnostics.DiagnosticSource package reference.
.../AppInsightTelemetryClient.cs Folds numeric metrics into properties to fit the 3.x TrackEvent API.
.../AppInsightTelemetryClientFactory.cs Adds optional local-export path; returns LocalFileTelemetryClient when set.
.../AppInsightsProvider.cs Adds the TESTINGPLATFORM_TELEMETRY_LOCALEXPORTPATH env-var constant.
.../AppInsightsTelemetryProviderExtensions.cs Reads the local-export env var and passes it to the factory.
.../LocalFileTelemetryClient.cs New JSON-Lines local telemetry client with manual JSON escaping.
.../PACKAGE.md Documents the new local-export verification option.
.../LocalFileTelemetryClientTests.cs New unit tests for the local client and factory selection.

Review details

  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Medium

Copilot AI review requested due to automatic review settings July 8, 2026 09:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment thread src/Platform/Microsoft.Testing.Extensions.Telemetry/LocalFileTelemetryClient.cs Outdated
Evangelink and others added 2 commits July 28, 2026 10:07
- Bump System.Threading.Tasks.Extensions 4.5.4 -> 4.6.3. Microsoft.ApplicationInsights
  3.1.2 -> Azure.Monitor.OpenTelemetry.Exporter -> Azure.Core 1.54.0 requires >= 4.6.3,
  which made every .NETFramework project referencing MSTest.TestAdapter fail restore with
  NU1605 (Windows-only, because the Linux/macOS legs skip the net462 targets).
- Declare the new/changed internal APIs (LocalFileTelemetryClient,
  AppInsightTelemetryClientFactory ctor, AppInsightsProvider.LocalExportPathEnvVar,
  TelemetryProperties.VersionValue = "21") in the InternalAPI.Unshipped.txt baselines
  introduced by #9752, which landed on main after this PR was opened.
- Add the required UTF-8 BOM to LocalFileTelemetryClient.cs and
  LocalFileTelemetryClientTests.cs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1e947c36-e52a-4ba8-8185-cd338ae24060
Copilot AI review requested due to automatic review settings July 28, 2026 08:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (5)

src/Platform/Microsoft.Testing.Platform/Telemetry/TelemetryProperties.cs:17

  • VersionValue comes from Microsoft.Testing.Platform, not from the telemetry exporter that determines where metrics land. The package dependency range permits an older 2.x telemetry extension to run with this newer Platform version, so such runs would report version 21 while still writing customMeasurements; the dashboard branch described by this PR would then read the wrong column. Tie the cutover marker to the telemetry client/extension that performs the conversion, or otherwise enforce package pairing so version 21 reliably identifies the new layout.
    public const string VersionValue = "21";

src/Platform/Microsoft.Testing.Extensions.Telemetry/LocalFileTelemetryClient.cs:59

  • The local format keeps measurements in a separate numeric metrics object, whereas the live 3.x path folds them into string-valued properties (AppInsightTelemetryClient.cs:43-49). Consequently this exporter cannot verify the schema-21/customDimensions representation advertised in PACKAGE.md and the PR description. Reuse one normalization step for both clients, then update the JSON contract and tests to reflect the actual exported shape.
        builder.Append("},\"metrics\":{");
        first = true;
        foreach (KeyValuePair<string, double> metric in metrics)

src/Platform/Microsoft.Testing.Extensions.Telemetry/LocalFileTelemetryClient.cs:67

  • This emits NaN, Infinity, or -Infinity as bare tokens when a telemetry producer supplies a non-finite double, which makes the promised JSON Lines record invalid JSON. The interface accepts arbitrary doubles, so serialize non-finite values using an explicitly valid representation (or reject them before writing) and cover that boundary case.
            AppendJsonKey(builder, metric.Key);
            builder.Append(metric.Value.ToString("R", CultureInfo.InvariantCulture));

src/Platform/Microsoft.Testing.Extensions.Telemetry/LocalFileTelemetryClient.cs:75

  • The single-consumer guarantee is only per provider process. The export-path environment variable is inherited by spawned test-host processes, so several providers can call AppendAllText on the same file concurrently; on Windows the competing append opens can throw IOException, and the provider then drops those events. Use an inter-process-safe append strategy, or write per-process files and merge them.
        // TrackEvent is only ever invoked from the provider's single-consumer ingest loop, so no
        // synchronization is needed here (mirroring AppInsightTelemetryClient).
        File.AppendAllText(_filePath, builder.ToString() + Environment.NewLine);

src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf:625

  • This unrelated change replaces an existing French translation with the English source and resets it to state="new", so French analyzer users regress to English. Revert this XLF hunk; the owning RESX source is unchanged, and repository localization guidance requires XLF updates to be generated rather than edited independently.
        <target state="new">Methods marked with '[GlobalTestInitialize]' or '[GlobalTestCleanup]' should follow the following layout to be valid:
-it can't be declared on a generic class
-it should be 'public'
-it should be 'static'
-it should not be 'async void'
-it should not be a special method (finalizer, operator...).
-it should not be generic
-it should take one parameter of type 'TestContext'
-return type should be 'void', 'Task' or 'ValueTask'

The type declaring these methods should also respect the following rules:
-The type should be a class
-The class should be 'public'
-The class should be marked with '[TestClass]' (or a derived attribute)
-the class should not be generic.</target>
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@Evangelink

Copy link
Copy Markdown
Member Author

Pushed a fix for the red pipeline (build 1529395 is now fully green). The branch was also merged with main (it was ~315 commits behind).

Root cause of Build Windows Debug / Build Windows ReleaseNU1605 package downgrade:

MSTest.TestAdapter -> Microsoft.Testing.Extensions.Telemetry -> Microsoft.ApplicationInsights 3.1.2
  -> Azure.Monitor.OpenTelemetry.Exporter 1.8.0 -> Azure.Core 1.54.0 -> System.Threading.Tasks.Extensions (>= 4.6.3)
MSTest.TestAdapter -> System.Threading.Tasks.Extensions (>= 4.5.4)

SystemThreadingTasksExtensionsVersion was pinned at 4.5.4, below what Azure.Core (new transitive dep of AI 3.x) requires. It only broke Windows because the reference is conditioned on net462/UAP, so the Linux/macOS legs never restored it. Bumped the pin to 4.6.3, which matches the existing System.Memory 4.6.3 pin — the shipped MSTest.TestAdapter nuspec now declares System.Threading.Tasks.Extensions 4.6.3 for .NETFramework4.6.2/UAP10.0.16299.

Two follow-on issues surfaced once restore succeeded:

  • RS0051Add InternalAPI tracking to all projects with PublicAPI #9752 added InternalAPI.*.txt tracking after this PR was opened. Declared the new/changed internal APIs (LocalFileTelemetryClient + members, the AppInsightTelemetryClientFactory(string?) ctor, AppInsightsProvider.LocalExportPathEnvVar, and TelemetryProperties.VersionValue = "21") in the corresponding InternalAPI.Unshipped.txt baselines.
  • Missing UTF-8 BOM on LocalFileTelemetryClient.cs and LocalFileTelemetryClientTests.cs (required by .editorconfig).

No product behavior was changed beyond the dependency version bump.

Evangelink and others added 2 commits July 28, 2026 13:00
…ements)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1e947c36-e52a-4ba8-8185-cd338ae24060
Copilot AI review requested due to automatic review settings July 28, 2026 11:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Comments suppressed due to low confidence (2)

src/Platform/Microsoft.Testing.Extensions.Telemetry/LocalFileTelemetryClient.cs:60

  • This local output preserves measurements in a separate numeric metrics object, while the migrated AppInsightTelemetryClient puts those same values into properties as strings. Consequently the documented local verification path does not expose the schema that will actually reach customDimensions, which is the central migration behavior this PR needs to verify. Serialize the post-migration shape here as well and update the test expectation.
        builder.Append("},\"metrics\":{");
        first = true;
        foreach (KeyValuePair<string, double> metric in metrics)
        {

src/Platform/Microsoft.Testing.Extensions.Telemetry/LocalFileTelemetryClient.cs:75

  • The single-consumer guarantee only applies within one provider process. This environment-supplied path can be shared by concurrently running testhost processes, and simultaneous File.AppendAllText calls can fail with sharing violations (the provider catches the exception and drops the event). Use cross-process-safe serialization or define per-process output files and a merge strategy.
        // TrackEvent is only ever invoked from the provider's single-consumer ingest loop, so no
        // synchronization is needed here (mirroring AppInsightTelemetryClient).
        File.AppendAllText(_filePath, builder.ToString() + Environment.NewLine);
  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment on lines +13 to +17
// Bump this whenever the telemetry schema changes so dashboards can branch old vs new.
// 21: Microsoft.ApplicationInsights 3.x (OpenTelemetry shim) removed TrackEvent's metrics
// parameter, so numeric measurements are now folded into event properties
// (customDimensions) instead of customMeasurements. See #7465.
public const string VersionValue = "21";

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I traced this and the analysis holds. TelemetryManager (platform) builds TelemetryInformation from TelemetryProperties.VersionValue (TelemetryManager.cs:63), and AppInsightsProvider (extension) is what stamps that value as the telemetry version dimension (AppInsightsProvider.cs:182). Since the two ship as separately-resolvable NuGets, a newer platform paired with the 2.x telemetry extension - the combination TelemetryForwardCompatibilityTests explicitly pins at 2.0.0 and regression-tests - would emit telemetry version = 21 while its AppInsightTelemetryClient still routes numeric values into customMeasurements. A dashboard branching on >= 21 would then read absent customDimensions keys and silently yield nulls.

I am deliberately not flipping the constant here. The bump to "21" was a deliberate commit (faf88bf) made in response to an earlier review thread asking for exactly this signal, so reverting it on review feedback alone would trade one wrong signal for no signal and re-open that thread. The convergent fix is to let the component whose serialization actually changed own the value, but each shape is a schema decision with downstream dashboard consequences:

  1. Revert the platform constant to "20" and have the 3.x extension stamp its own "21" at AppInsightsProvider.cs:182. Keeps a single dimension, but a future genuine platform-schema bump would be clobbered by the extension's literal.
  2. Revert the platform constant to "20" and add a separate extension-owned dimension describing where numeric values live. Keeps telemetry version meaning "platform schema" and is unambiguous, at the cost of a new dimension for dashboards to read.

Either is a handful of lines, but picking between them changes the Kusto contract documented in the PR description, so I would rather not choose unsupervised. Leaving this unresolved for a maintainer call - happy to implement whichever you prefer.

Note

Auto-replied by the GitHub Copilot app.

@Evangelink

Copy link
Copy Markdown
Member Author

Merged latest main to clear the merge conflict that appeared after #10268 / #10281 landed. The auto-merge of src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf combined both sides and produced an invalid trans-unit with two <target> elements (one state="new", one state="translated"), so I restored main's version of that file.

The diff against main is now exactly this PR's intended changes, and build 1529517 is green on all six legs. PR is MERGEABLE.

@Evangelink
Evangelink marked this pull request as ready for review July 28, 2026 12:21
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9725

GradeTestMutationNotesHow to improve
C (70–79) new LocalFileTelemetryClientTests.
Factory_
WithLocalExportPath_
CreatesLocalFileClient
1/3 killed Only the return type is checked; mutations that pass the wrong file path or ignore it entirely survive. Call TrackEvent on the returned client and assert the expected file is written at the given path.
B (80–89) new LocalFileTelemetryClientTests.
TrackEvent_
EscapesSpecialCharacters
3/4 killed Covers quote, backslash, and newline escaping plus the single-line constraint, but omits \b, \f, \r, \t and control-char (\u00xx) branches. Add assertions for at least one control character (e.g., a tab \t or carriage return \r) to kill the remaining escape-branch mutations.
B (80–89) new LocalFileTelemetryClientTests.
TrackEvent_
WritesJsonLine_
WithEventPropertiesAndMetrics
4/5 killed Kills all field-presence mutations, but a mutation that moves properties outside their JSON wrapper object would survive the substring checks. Assert the literal "properties":{"is ci":"true"} substring to pin the nesting structure and kill wrapper-removal mutations.
A (90–100) new LocalFileTelemetryClientTests.
TrackEvent_
AppendsOneLinePerEvent
3/3 killed Kills write-once, order-swap, and wrong-separator mutations; each line is verified independently.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 33.3 AIC · ⌖ 7.59 AIC · ⊞ 10.3K · [◷]( · )

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.

2 participants