Integrate the initial node metrics #11481

JanKrivanek · 2025-02-21T17:40:42Z

Context

Connects the exposed data into the telemetry collector, converts them into json format.

Testing

TBD

Also - we need to test the resulting output on Orchard to have an idea on volume/size of resulting data - we may need to aggregate/filter targets a bit?

Notes

Sample of resulting data:

"Tasks":

[
  "TaskA": {
    "ExecTimeMs": 2000,
    "ExecCnt": 5,
    "MemKBs": 0.5322265625,
    "IsCustom": false,
    "IsFromNuget": true
  },
  "75745449d67e47c3288fa8db4bf6961b455bdf65f6597a4e1c897e1740ebdc1a": {
    "ExecTimeMs": 254548000,
    "ExecCnt": 6,
    "MemKBs": 53267.0419921875,
    "IsCustom": true,
    "IsFromNuget": false
  }
]



"Targets":

[
  "TargetA": {
    "WasExecuted": false,
    "IsCustom": false,
    "IsFromNuget": true,
    "IsMetaproj": false
  },
  "ce5582cb3fdd2433a022932a42185f9bd62696888bc8102d0bad33363c43aa96": {
    "WasExecuted": false,
    "IsCustom": true,
    "IsFromNuget": true,
    "IsMetaproj": false
  },
  "TargetB": {
    "WasExecuted": false,
    "IsCustom": false,
    "IsFromNuget": false,
    "IsMetaproj": true
  }
]


"TargetsSummary":

"Loaded": {
  "Total": 3,
  "Microsoft": {
    "Total": 1,
    "FromNuget": 1,
    "FromMetaproj": 0
  },
  "Custom": {
    "Total": 2,
    "FromNuget": 1,
    "FromMetaproj": 1
  }
},
"Executed": {
  "Total": 0
}


"TasksSummary":

"Microsoft": {
  "Total": {
    "TotalExecutionsCount": 5,
    "CumulativeExecutionTimeMs": 2000,
    "CumulativeConsumedMemoryKB": 0.5322265625
  },
  "FromNuget": {
    "TotalExecutionsCount": 5,
    "CumulativeExecutionTimeMs": 2000,
    "CumulativeConsumedMemoryKB": 0.5322265625
  }
},
"Custom": {
  "Total": {
    "TotalExecutionsCount": 6,
    "CumulativeExecutionTimeMs": 254548000,
    "CumulativeConsumedMemoryKB": 53267.0419921875
  }
}

src/Build/TelemetryInfra/TelemetryDataUtils.cs

src/Build.UnitTests/TelemetryTests.cs

src/Build/BackEnd/BuildManager/BuildManager.cs

JanKrivanek · 2025-02-24T14:42:15Z

As discussed offline - left for @JanProvaznik to handle:

VS perf integration tests when opted in
unit/e2e tests - since this is opt-in feature - it's fine to do manual verifications now, and handle unit or/and e2e tests in followup items
verifications that the 'tags' appended to main activity (the "Tasks", "Targets", "TargetsSummary", "TasksSummary") will appear as json subtags - not json encoded strings, that then would be hassle to query out
dump the sample data on OrchardCore or MSBuild or similar mid-size repo, to get the idea of size collected and verify with VS data platform folks that it's fine to send such sample size for sampled-in runs

src/Build/BackEnd/BuildManager/BuildManager.cs

JanProvaznik · 2025-02-24T15:20:34Z

ran $env:MSBUILDNODETELEMETRYFILENAME="telemetry.txt"; $env:MSBUILD_TELEMETRY_SAMPLE_RATE="1.0"; $env:MSBUILD_TELEMETRY_OPTIN="1" $env:MSBUILDOUTPUTNODESTELEMETRY="1"; .\artifacts\bin\bootstrap\net472\MSBuild\Current\Bin\MSBuild.exe ..\OrchardCore\
on restored orchardcore,

can't find the telemetry.txt

1668 targets (half metaproj)
266 tasks
the console flush seems to be same ballpark as the json, this results in 200kb of data

we might want to aggregate metaproj targets, if they're hashed then it's not very useful to have their details

JanProvaznik · 2025-02-24T15:22:28Z

maybe the csv serialization will be needed after all🤔

…raits

JanKrivanek

As a co-author I cannot approve - but all changes by @JanProvaznik looks good to go!

src/Build/TelemetryInfra/TelemetryDataUtils.cs

JanProvaznik

it breaks VS C++ scenarios, needs more testing. https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=11074124&view=ms.vss-test-web.build-test-results-tab&runId=122902221&resultId=100000&paneView=debug

JanKrivanek · 2025-02-26T11:50:52Z

it breaks VS C++ scenarios, needs more testing. https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=11074124&view=ms.vss-test-web.build-test-results-tab&runId=122902221&resultId=100000&paneView=debug

This was a recent change of 'ExecutionsCount' type change from short to int, that wasn't reflected on the receiving side (it was caught by the failing WorkerNodeTelemetryEventArgs_Tests unit test, that is green now)

src/Framework/Telemetry/TaskExecutionStats.cs

JanProvaznik · 2025-02-26T12:17:15Z

strategy to get this in 17.14p2:

experimental insertion + always on override
meanwhile merge and run real insertion
completereal insert if both suceed
if no, revert from main :( and reopen
address all comments and issues in followup pr

src/Framework/Telemetry/TaskExecutionStats.cs

YuliiaKovalova · 2025-02-26T12:10:55Z

src/Build/TelemetryInfra/TelemetryDataUtils.cs

+                        // We do not want decimals
+                        writer.WriteNumber("CumulativeExecutionTimeMs", (long)stats.CumulativeExecutionTime.TotalMilliseconds);
+                        // We do not want decimals
+                        writer.WriteNumber("CumulativeConsumedMemoryKB", stats.TotalMemoryConsumption / 1024);


can we use consistent naming for the variables and json keys, so it can be nameof(prop), instead on the hardcoded strings?

src/Build/TelemetryInfra/TelemetryDataUtils.cs

YuliiaKovalova · 2025-02-26T12:12:48Z

src/Build/TelemetryInfra/TelemetryDataUtils.cs

+                WriteStat(writer, value.CustomTasksInfo, "Custom");
+                writer.WriteEndObject();
+
+                void WriteStat(Utf8JsonWriter writer, TasksInfo tasksInfo, string name)


is it dupe of line 222?

YuliiaKovalova · 2025-02-26T12:13:49Z

src/Build/TelemetryInfra/TelemetryDataUtils.cs

+                    }
+                }
+
+                void UpdateStatistics(


please add short description for the method

YuliiaKovalova · 2025-02-26T12:16:29Z

src/Build/TelemetryInfra/TelemetryDataUtils.cs

+
+        private static JsonSerializerOptions _serializerOptions = CreateSerializerOptions();
+
+        private static JsonSerializerOptions CreateSerializerOptions()


why we can't make it as a static property without a separation ?

src/Build/TelemetryInfra/InternalTelemetryConsumingLogger.cs

src/Framework/Microsoft.Build.Framework.csproj

YuliiaKovalova · 2025-02-26T12:26:22Z

src/Build/TelemetryInfra/TelemetryDataUtils.cs

+{
+    internal static class TelemetryDataUtils
+    {
+        public static IActivityTelemetryDataHolder? AsActivityDataHolder(this IWorkerNodeTelemetryData? telemetryData, bool includeTasksDetails, bool includeTargetDetails)


please document this method

YuliiaKovalova

agreed to fix the comment in the separate PR

Integrate the initial node metrics

85ac838

JanProvaznik reviewed Feb 24, 2025

View reviewed changes

src/Build/TelemetryInfra/TelemetryDataUtils.cs Outdated Show resolved Hide resolved

src/Build.UnitTests/TelemetryTests.cs Show resolved Hide resolved

src/Build/BackEnd/BuildManager/BuildManager.cs Outdated Show resolved Hide resolved

Add node telemetry data summarization

e852270

JanKrivanek commented Feb 24, 2025

View reviewed changes

src/Build/BackEnd/BuildManager/BuildManager.cs Outdated Show resolved Hide resolved

JanKrivanek mentioned this pull request Feb 24, 2025

Integrate the initial Tasks/Targets metric to the new telemetry infra #11480

Closed

JanProvaznik mentioned this pull request Feb 24, 2025

add e2e test for node telemetry #11485

Open

JanProvaznik requested a review from MichalPavlik February 24, 2025 16:28

JanProvaznik added 3 commits February 24, 2025 17:38

fix flushing node data to json

c74c63f

Refactor telemetry task details exclusion logic in BuildManager and T…

4fb8a4a

…raits

syntax

ed55896

JanProvaznik self-assigned this Feb 24, 2025

JanProvaznik added 4 commits February 25, 2025 13:37

fix

2c91c4e

Merge branch 'main' into telemetry/integrate

ce212c4

fix schema for summaries

ed45e8a

more wrapping objects

c3cea19

JanKrivanek commented Feb 25, 2025

View reviewed changes

JanKrivanek commented Feb 26, 2025

View reviewed changes

src/Build/TelemetryInfra/TelemetryDataUtils.cs Outdated Show resolved Hide resolved

JanProvaznik requested changes Feb 26, 2025

View reviewed changes

JanKrivanek added 2 commits February 26, 2025 12:47

Fix failing tests

51aaf20

Remove indentation

aa2d584

JanKrivanek requested a review from JanProvaznik February 26, 2025 11:49

YuliiaKovalova reviewed Feb 26, 2025

View reviewed changes

src/Framework/Telemetry/TaskExecutionStats.cs Show resolved Hide resolved

JanProvaznik approved these changes Feb 26, 2025

View reviewed changes

YuliiaKovalova reviewed Feb 26, 2025

View reviewed changes

src/Framework/Microsoft.Build.Framework.csproj Show resolved Hide resolved

YuliiaKovalova reviewed Feb 26, 2025

View reviewed changes

YuliiaKovalova approved these changes Feb 26, 2025

View reviewed changes

JanProvaznik mentioned this pull request Feb 26, 2025

Telemetry code quality/documentation followup #11506

Open

JanProvaznik merged commit 7e8d5f5 into main Feb 26, 2025
10 checks passed

JanProvaznik deleted the telemetry/integrate branch February 26, 2025 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate the initial node metrics #11481

Integrate the initial node metrics #11481

JanKrivanek commented Feb 21, 2025 •

edited

Loading

JanKrivanek commented Feb 24, 2025

JanProvaznik commented Feb 24, 2025 •

edited

Loading

JanProvaznik commented Feb 24, 2025

JanKrivanek left a comment

JanProvaznik left a comment •

edited

Loading

JanKrivanek commented Feb 26, 2025

JanProvaznik commented Feb 26, 2025 •

edited

Loading

YuliiaKovalova Feb 26, 2025

YuliiaKovalova Feb 26, 2025

YuliiaKovalova Feb 26, 2025

YuliiaKovalova Feb 26, 2025

YuliiaKovalova Feb 26, 2025

YuliiaKovalova left a comment


		private static JsonSerializerOptions _serializerOptions = CreateSerializerOptions();

		private static JsonSerializerOptions CreateSerializerOptions()

Integrate the initial node metrics #11481

Integrate the initial node metrics #11481

Conversation

JanKrivanek commented Feb 21, 2025 • edited Loading

Context

Testing

Notes

JanKrivanek commented Feb 24, 2025

JanProvaznik commented Feb 24, 2025 • edited Loading

JanProvaznik commented Feb 24, 2025

JanKrivanek left a comment

Choose a reason for hiding this comment

JanProvaznik left a comment • edited Loading

Choose a reason for hiding this comment

JanKrivanek commented Feb 26, 2025

JanProvaznik commented Feb 26, 2025 • edited Loading

YuliiaKovalova Feb 26, 2025

Choose a reason for hiding this comment

YuliiaKovalova Feb 26, 2025

Choose a reason for hiding this comment

YuliiaKovalova Feb 26, 2025

Choose a reason for hiding this comment

YuliiaKovalova Feb 26, 2025

Choose a reason for hiding this comment

YuliiaKovalova Feb 26, 2025

Choose a reason for hiding this comment

YuliiaKovalova left a comment

Choose a reason for hiding this comment

JanKrivanek commented Feb 21, 2025 •

edited

Loading

JanProvaznik commented Feb 24, 2025 •

edited

Loading

JanProvaznik left a comment •

edited

Loading

JanProvaznik commented Feb 26, 2025 •

edited

Loading