Add custom keying

dotnet · Feb 13, 2025 · a3ad3db · a3ad3db
1 parent 2604eec
commit a3ad3db
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 78 deletions.
diff --git a/src/Build.UnitTests/TelemetryTests.cs b/src/Build.UnitTests/TelemetryTests.cs
@@ -68,17 +68,18 @@ public void WorkerNodeTelemetryCollection_BasicTarget()
                 new BuildParameters() { IsTelemetryEnabled = true }).OverallResult.ShouldBe(BuildResultCode.Success);
 
             workerNodeTelemetryData!.ShouldNotBeNull();
-            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey("C:Build");
-            workerNodeTelemetryData.TargetsExecutionData["C:Build"].ShouldBeTrue();
+            var buildTargetKey = new TaskOrTargetTelemetryKey("Build", true, false);
+            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(buildTargetKey);
+            workerNodeTelemetryData.TargetsExecutionData[buildTargetKey].ShouldBeTrue();
             workerNodeTelemetryData.TargetsExecutionData.Keys.Count.ShouldBe(1);
 
             workerNodeTelemetryData.TasksExecutionData.Keys.Count.ShouldBeGreaterThan(2);
-            ((int)workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.Message"].ExecutionsCount).ShouldBe(2);
-            workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.Message"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
-            ((int)workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.CreateItem"].ExecutionsCount).ShouldBe(1);
-            workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.CreateItem"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
+            ((int)workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.Message"].ExecutionsCount).ShouldBe(2);
+            workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.Message"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
+            ((int)workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.CreateItem"].ExecutionsCount).ShouldBe(1);
+            workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.CreateItem"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
 
-            workerNodeTelemetryData.TasksExecutionData.Keys.ShouldAllBe(k => !k.StartsWith("C:") && !k.StartsWith("N:"));
+            workerNodeTelemetryData.TasksExecutionData.Keys.ShouldAllBe(k => !k.IsCustom && !k.IsFromNugetCache);
             workerNodeTelemetryData.TasksExecutionData.Values
                 .Count(v => v.CumulativeExecutionTime > TimeSpan.Zero || v.ExecutionsCount > 0).ShouldBe(2);
         }
@@ -139,30 +140,30 @@ public void WorkerNodeTelemetryCollection_CustomTargetsAndTasks()
                 new BuildParameters() { IsTelemetryEnabled = true }).OverallResult.ShouldBe(BuildResultCode.Success);
 
             workerNodeTelemetryData!.ShouldNotBeNull();
-            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey("C:Build");
-            workerNodeTelemetryData.TargetsExecutionData["C:Build"].ShouldBeTrue();
-            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey("C:BeforeBuild");
-            workerNodeTelemetryData.TargetsExecutionData["C:BeforeBuild"].ShouldBeTrue();
-            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey("C:NotExecuted");
-            workerNodeTelemetryData.TargetsExecutionData["C:NotExecuted"].ShouldBeFalse();
+            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(new TaskOrTargetTelemetryKey("Build", true, false));
+            workerNodeTelemetryData.TargetsExecutionData[new TaskOrTargetTelemetryKey("Build", true, false)].ShouldBeTrue();
+            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(new TaskOrTargetTelemetryKey("BeforeBuild", true, false));
+            workerNodeTelemetryData.TargetsExecutionData[new TaskOrTargetTelemetryKey("BeforeBuild", true, false)].ShouldBeTrue();
+            workerNodeTelemetryData.TargetsExecutionData.ShouldContainKey(new TaskOrTargetTelemetryKey("NotExecuted", true, false));
+            workerNodeTelemetryData.TargetsExecutionData[new TaskOrTargetTelemetryKey("NotExecuted", true, false)].ShouldBeFalse();
             workerNodeTelemetryData.TargetsExecutionData.Keys.Count.ShouldBe(3);
 
             workerNodeTelemetryData.TasksExecutionData.Keys.Count.ShouldBeGreaterThan(2);
-            ((int)workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.Message"].ExecutionsCount).ShouldBe(3);
-            workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.Message"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
-            ((int)workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.CreateItem"].ExecutionsCount).ShouldBe(1);
-            workerNodeTelemetryData.TasksExecutionData["Microsoft.Build.Tasks.CreateItem"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
+            ((int)workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.Message"].ExecutionsCount).ShouldBe(3);
+            workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.Message"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
+            ((int)workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.CreateItem"].ExecutionsCount).ShouldBe(1);
+            workerNodeTelemetryData.TasksExecutionData[(TaskOrTargetTelemetryKey)"Microsoft.Build.Tasks.CreateItem"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
 
-            ((int)workerNodeTelemetryData.TasksExecutionData["C:Task01"].ExecutionsCount).ShouldBe(2);
-            workerNodeTelemetryData.TasksExecutionData["C:Task01"].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
+            ((int)workerNodeTelemetryData.TasksExecutionData[new TaskOrTargetTelemetryKey("Task01", true, false)].ExecutionsCount).ShouldBe(2);
+            workerNodeTelemetryData.TasksExecutionData[new TaskOrTargetTelemetryKey("Task01", true, false)].CumulativeExecutionTime.ShouldBeGreaterThan(TimeSpan.Zero);
 
-            ((int)workerNodeTelemetryData.TasksExecutionData["C:Task02"].ExecutionsCount).ShouldBe(0);
-            workerNodeTelemetryData.TasksExecutionData["C:Task02"].CumulativeExecutionTime.ShouldBe(TimeSpan.Zero);
+            ((int)workerNodeTelemetryData.TasksExecutionData[new TaskOrTargetTelemetryKey("Task02", true, false)].ExecutionsCount).ShouldBe(0);
+            workerNodeTelemetryData.TasksExecutionData[new TaskOrTargetTelemetryKey("Task02", true, false)].CumulativeExecutionTime.ShouldBe(TimeSpan.Zero);
 
             workerNodeTelemetryData.TasksExecutionData.Values
                 .Count(v => v.CumulativeExecutionTime > TimeSpan.Zero || v.ExecutionsCount > 0).ShouldBe(3);
 
-            workerNodeTelemetryData.TasksExecutionData.Keys.ShouldAllBe(k => !k.StartsWith("N:"));
+            workerNodeTelemetryData.TasksExecutionData.Keys.ShouldAllBe(k => !k.IsFromNugetCache);
         }
     }
 }
diff --git a/src/Build/Telemetry/TelemetryForwarderProvider.cs b/src/Build/Telemetry/TelemetryForwarderProvider.cs
@@ -57,35 +57,19 @@ public class TelemetryForwarder : ITelemetryForwarder
 
         public void AddTask(string name, TimeSpan cumulativeExecutionTime, short executionsCount, long totalMemoryConsumed, bool isCustom, bool isFromNugetCache)
         {
-            name = GetName(name, isCustom, false, isFromNugetCache);
-            _workerNodeTelemetryData.AddTask(name, cumulativeExecutionTime, executionsCount, totalMemoryConsumed);
+            var key = GetKey(name, isCustom, false, isFromNugetCache);
+            _workerNodeTelemetryData.AddTask(key, cumulativeExecutionTime, executionsCount, totalMemoryConsumed);
         }
 
         public void AddTarget(string name, bool wasExecuted, bool isCustom, bool isMetaproj, bool isFromNugetCache)
         {
-            name = GetName(name, isCustom, isMetaproj, isFromNugetCache);
-            _workerNodeTelemetryData.AddTarget(name, wasExecuted);
+            var key = GetKey(name, isCustom, isMetaproj, isFromNugetCache);
+            _workerNodeTelemetryData.AddTarget(key, wasExecuted);
         }
 
-        private static string GetName(string name, bool isCustom, bool isMetaproj, bool isFromNugetCache)
-        {
-            if (isMetaproj)
-            {
-                name = WorkerNodeTelemetryData.MetaProjPrefix + name;
-            }
-
-            if (isCustom)
-            {
-                name = WorkerNodeTelemetryData.CustomPrefix + name;
-            }
-
-            if (isFromNugetCache)
-            {
-                name = WorkerNodeTelemetryData.FromNugetPrefix + name;
-            }
-
-            return name;
-        }
+        private static TaskOrTargetTelemetryKey GetKey(string name, bool isCustom, bool isMetaproj,
+            bool isFromNugetCache)
+            => new TaskOrTargetTelemetryKey(name, isCustom, isFromNugetCache, isMetaproj);
 
         public void FinalizeProcessing(LoggingContext loggingContext)
         {

diff --git a/src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs b/src/Framework.UnitTests/WorkerNodeTelemetryEventArgs_Tests.cs
@@ -18,13 +18,13 @@ public class WorkerNodeTelemetryEventArgs_Tests
         public void SerializationDeserializationTest()
         {
             WorkerNodeTelemetryData td = new WorkerNodeTelemetryData(
-                new Dictionary<string, TaskExecutionStats>()
+                new Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats>()
                 {
-                        { "task1", new TaskExecutionStats(TimeSpan.FromMinutes(1), 5, 1234) },
-                        { "task2", new TaskExecutionStats(TimeSpan.Zero, 0, 0) },
-                        { "task3", new TaskExecutionStats(TimeSpan.FromTicks(1234), 12, 987654321) }
+                        { (TaskOrTargetTelemetryKey)"task1", new TaskExecutionStats(TimeSpan.FromMinutes(1), 5, 1234) },
+                        { (TaskOrTargetTelemetryKey)"task2", new TaskExecutionStats(TimeSpan.Zero, 0, 0) },
+                        { (TaskOrTargetTelemetryKey)"task3", new TaskExecutionStats(TimeSpan.FromTicks(1234), 12, 987654321) }
                 },
-                new Dictionary<string, bool>() { { "target1", false }, { "target2", true }, });
+                new Dictionary<TaskOrTargetTelemetryKey, bool>() { { (TaskOrTargetTelemetryKey)"target1", false }, { (TaskOrTargetTelemetryKey)"target2", true }, });
 
             WorkerNodeTelemetryEventArgs args = new WorkerNodeTelemetryEventArgs(td);
 

diff --git a/src/Framework/Telemetry/IWorkerNodeTelemetryData.cs b/src/Framework/Telemetry/IWorkerNodeTelemetryData.cs
@@ -7,6 +7,6 @@ namespace Microsoft.Build.Framework;
 
 internal interface IWorkerNodeTelemetryData
 {
-    Dictionary<string, TaskExecutionStats> TasksExecutionData { get; }
-    Dictionary<string, bool> TargetsExecutionData { get; }
+    Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats> TasksExecutionData { get; }
+    Dictionary<TaskOrTargetTelemetryKey, bool> TargetsExecutionData { get; }
 }
diff --git a/src/Framework/Telemetry/InternalTelemetryConsumingLogger.cs b/src/Framework/Telemetry/InternalTelemetryConsumingLogger.cs
@@ -50,7 +50,7 @@ private void FlushDataIntoConsoleIfRequested()
         Console.WriteLine("==========================================");
         Console.WriteLine($"Tasks: ({_workerNodeTelemetryData.TasksExecutionData.Count})");
         Console.WriteLine("Custom tasks:");
-        foreach (var task in _workerNodeTelemetryData.TasksExecutionData.Where(t => t.Key.StartsWith(WorkerNodeTelemetryData.CustomPrefix) || t.Key.StartsWith(WorkerNodeTelemetryData.FromNugetPrefix + WorkerNodeTelemetryData.CustomPrefix)))
+        foreach (var task in _workerNodeTelemetryData.TasksExecutionData.Where(t => t.Key.IsCustom))
         {
             Console.WriteLine($"{task.Key}");
         }

diff --git a/src/Framework/Telemetry/TaskExecutionStats.cs b/src/Framework/Telemetry/TaskExecutionStats.cs
@@ -26,6 +26,7 @@ protected bool Equals(TaskExecutionStats other)
            TotalMemoryConsumption == other.TotalMemoryConsumption &&
            ExecutionsCount == other.ExecutionsCount;
 
+    // Needed since we override Equals
     public override int GetHashCode()
     {
         unchecked

diff --git a/src/Framework/Telemetry/TaskOrTargetTelemetryKey.cs b/src/Framework/Telemetry/TaskOrTargetTelemetryKey.cs
@@ -0,0 +1,66 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+
+namespace Microsoft.Build.Framework;
+
+internal struct TaskOrTargetTelemetryKey : IEquatable<TaskOrTargetTelemetryKey>
+{
+    public TaskOrTargetTelemetryKey(string name, bool isCustom, bool isFromNugetCache, bool isFromMetaProject)
+    {
+        Name = name;
+        IsCustom = isCustom;
+        IsFromNugetCache = isFromNugetCache;
+        IsFromMetaProject = isFromMetaProject;
+    }
+
+    public TaskOrTargetTelemetryKey(string name, bool isCustom, bool isFromNugetCache)
+    {
+        Name = name;
+        IsCustom = isCustom;
+        IsFromNugetCache = isFromNugetCache;
+    }
+
+    public TaskOrTargetTelemetryKey(string name) => Name = name;
+
+    public static explicit operator TaskOrTargetTelemetryKey(string key) => new(key);
+
+    public string Name { get; }
+    // Indicate custom targets/task - those must be hashed.
+    public bool IsCustom { get; }
+    // Indicate targets/tasks sourced from nuget cache - those can be custom or MSFT provided ones.
+    public bool IsFromNugetCache { get; }
+    // Indicate targets/tasks generated during build - those must be hashed (as they contain paths).
+    public bool IsFromMetaProject { get; }
+
+    public override bool Equals(object? obj)
+    {
+        if (obj is TaskOrTargetTelemetryKey other)
+        {
+            return Equals(other);
+        }
+        return false;
+    }
+
+    public bool Equals(TaskOrTargetTelemetryKey other)
+        => string.Equals(Name, other.Name, StringComparison.OrdinalIgnoreCase) &&
+           IsCustom == other.IsCustom &&
+           IsFromNugetCache == other.IsFromNugetCache &&
+           IsFromMetaProject == other.IsFromMetaProject;
+
+    // We need hash code and equals - so that we can stuff data into dictionaries
+    public override int GetHashCode()
+    {
+        unchecked
+        {
+            var hashCode = Name.GetHashCode();
+            hashCode = (hashCode * 397) ^ IsCustom.GetHashCode();
+            hashCode = (hashCode * 397) ^ IsFromNugetCache.GetHashCode();
+            hashCode = (hashCode * 397) ^ IsFromMetaProject.GetHashCode();
+            return hashCode;
+        }
+    }
+
+    public override string ToString() => $"{Name},Custom:{IsCustom},IsFromNugetCache:{IsFromNugetCache},IsFromMetaProject:{IsFromMetaProject}";
+}
diff --git a/src/Framework/Telemetry/WorkerNodeTelemetryData.cs b/src/Framework/Telemetry/WorkerNodeTelemetryData.cs
@@ -8,14 +8,7 @@ namespace Microsoft.Build.Framework;
 
 internal class WorkerNodeTelemetryData : IWorkerNodeTelemetryData
 {
-    // Indicate custom targets/task - those must be hashed.
-    public const string CustomPrefix = "C:";
-    // Indicate targets/tasks sourced from nuget cache - those can be custom or MSFT provided ones.
-    public const string FromNugetPrefix = "N:";
-    // Indicate targets/tasks generated during build - those must be hashed (as they contain paths).
-    public const string MetaProjPrefix = "M:";
-
-    public WorkerNodeTelemetryData(Dictionary<string, TaskExecutionStats> tasksExecutionData, Dictionary<string, bool> targetsExecutionData)
+    public WorkerNodeTelemetryData(Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats> tasksExecutionData, Dictionary<TaskOrTargetTelemetryKey, bool> targetsExecutionData)
     {
         TasksExecutionData = tasksExecutionData;
         TargetsExecutionData = targetsExecutionData;
@@ -34,13 +27,13 @@ public void Add(IWorkerNodeTelemetryData other)
         }
     }
 
-    public void AddTask(string name, TimeSpan cumulativeExectionTime, short executionsCount, long totalMemoryConsumption)
+    public void AddTask(TaskOrTargetTelemetryKey task, TimeSpan cumulativeExectionTime, short executionsCount, long totalMemoryConsumption)
     {
         TaskExecutionStats? taskExecutionStats;
-        if (!TasksExecutionData.TryGetValue(name, out taskExecutionStats))
+        if (!TasksExecutionData.TryGetValue(task, out taskExecutionStats))
         {
             taskExecutionStats = new(cumulativeExectionTime, executionsCount, totalMemoryConsumption);
-            TasksExecutionData[name] = taskExecutionStats;
+            TasksExecutionData[task] = taskExecutionStats;
         }
         else
         {
@@ -50,17 +43,17 @@ public void AddTask(string name, TimeSpan cumulativeExectionTime, short executio
         }
     }
 
-    public void AddTarget(string name, bool wasExecuted)
+    public void AddTarget(TaskOrTargetTelemetryKey target, bool wasExecuted)
     {
-        TargetsExecutionData[name] =
+        TargetsExecutionData[target] =
             // we just need to store if it was ever executed
-            wasExecuted || (TargetsExecutionData.TryGetValue(name, out bool wasAlreadyExecuted) && wasAlreadyExecuted);
+            wasExecuted || (TargetsExecutionData.TryGetValue(target, out bool wasAlreadyExecuted) && wasAlreadyExecuted);
     }
 
     public WorkerNodeTelemetryData()
-        : this(new Dictionary<string, TaskExecutionStats>(StringComparer.OrdinalIgnoreCase), new Dictionary<string, bool>(StringComparer.OrdinalIgnoreCase))
+        : this(new Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats>(), new Dictionary<TaskOrTargetTelemetryKey, bool>())
     { }
 
-    public Dictionary<string, TaskExecutionStats> TasksExecutionData { get; }
-    public Dictionary<string, bool> TargetsExecutionData { get; }
+    public Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats> TasksExecutionData { get; }
+    public Dictionary<TaskOrTargetTelemetryKey, bool> TargetsExecutionData { get; }
 }
diff --git a/src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs b/src/Framework/Telemetry/WorkerNodeTelemetryEventArgs.cs
@@ -19,39 +19,56 @@ public WorkerNodeTelemetryEventArgs()
     internal override void WriteToStream(BinaryWriter writer)
     {
         writer.Write7BitEncodedInt(WorkerNodeTelemetryData.TasksExecutionData.Count);
-        foreach (KeyValuePair<string, TaskExecutionStats> entry in WorkerNodeTelemetryData.TasksExecutionData)
+        foreach (KeyValuePair<TaskOrTargetTelemetryKey, TaskExecutionStats> entry in WorkerNodeTelemetryData.TasksExecutionData)
         {
-            writer.Write(entry.Key);
+            WriteToStream(writer, entry.Key);
             writer.Write(entry.Value.CumulativeExecutionTime.Ticks);
             writer.Write(entry.Value.ExecutionsCount);
             writer.Write(entry.Value.TotalMemoryConsumption);
         }
 
         writer.Write7BitEncodedInt(WorkerNodeTelemetryData.TargetsExecutionData.Count);
-        foreach (KeyValuePair<string, bool> entry in WorkerNodeTelemetryData.TargetsExecutionData)
+        foreach (KeyValuePair<TaskOrTargetTelemetryKey, bool> entry in WorkerNodeTelemetryData.TargetsExecutionData)
         {
-            writer.Write(entry.Key);
+            WriteToStream(writer, entry.Key);
             writer.Write(entry.Value);
         }
     }
 
     internal override void CreateFromStream(BinaryReader reader, int version)
     {
         int count = reader.Read7BitEncodedInt();
-        Dictionary<string, TaskExecutionStats> tasksExecutionData = new();
+        Dictionary<TaskOrTargetTelemetryKey, TaskExecutionStats> tasksExecutionData = new();
         for (int i = 0; i < count; i++)
         {
-            tasksExecutionData.Add(reader.ReadString(),
+            tasksExecutionData.Add(ReadFromStream(reader),
                 new TaskExecutionStats(TimeSpan.FromTicks(reader.ReadInt64()), reader.ReadInt16(), reader.ReadInt64()));
         }
 
         count = reader.Read7BitEncodedInt();
-        Dictionary<string, bool> targetsExecutionData = new();
+        Dictionary<TaskOrTargetTelemetryKey, bool> targetsExecutionData = new();
         for (int i = 0; i < count; i++)
         {
-            targetsExecutionData.Add(reader.ReadString(), reader.ReadBoolean());
+            targetsExecutionData.Add(ReadFromStream(reader), reader.ReadBoolean());
         }
 
         WorkerNodeTelemetryData = new WorkerNodeTelemetryData(tasksExecutionData, targetsExecutionData);
     }
+
+    private static void WriteToStream(BinaryWriter writer, TaskOrTargetTelemetryKey key)
+    {
+        writer.Write(key.Name);
+        writer.Write(key.IsCustom);
+        writer.Write(key.IsFromNugetCache);
+        writer.Write(key.IsFromMetaProject);
+    }
+
+    private static TaskOrTargetTelemetryKey ReadFromStream(BinaryReader reader)
+    {
+        return new TaskOrTargetTelemetryKey(
+            reader.ReadString(),
+            reader.ReadBoolean(),
+            reader.ReadBoolean(),
+            reader.ReadBoolean());
+    }
 }