Skip to content

Minimal work to make System.Diagnostics.Activity usable by the dotnet CLI application and codebase #49749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
<PackageVersion Include="System.Composition.Runtime" Version="$(SystemCompositionRuntimePackageVersion)" />
<PackageVersion Include="System.Composition.TypedParts" Version="$(SystemCompositionTypedPartsPackageVersion)" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="$(SystemConfigurationConfigurationManagerPackageVersion)" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="$(SystemDiagnosticsDiagnosticSourcePackageVersion)" />
<PackageVersion Include="System.Formats.Asn1" Version="$(SystemFormatsAsn1Version)" />
<PackageVersion Include="System.IO.Hashing" Version="$(SystemIOHashingPackageVersion)" />
<!-- System.Reflection.Metadata and System.Collections.Immutable cannot be pinned here because of hard dependencies within Roslyn on specific versions that have to work both here and in VS -->
Expand Down
1 change: 1 addition & 0 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
<SystemCompositionHostingPackageVersion>10.0.0-preview.7.25359.101</SystemCompositionHostingPackageVersion>
<SystemCompositionRuntimePackageVersion>10.0.0-preview.7.25359.101</SystemCompositionRuntimePackageVersion>
<SystemCompositionTypedPartsPackageVersion>10.0.0-preview.7.25359.101</SystemCompositionTypedPartsPackageVersion>
<SystemDiagnosticsDiagnosticSourcePackageVersion>10.0.0-preview.7.25359.101</SystemDiagnosticsDiagnosticSourcePackageVersion>
<SystemConfigurationConfigurationManagerPackageVersion>10.0.0-preview.7.25359.101</SystemConfigurationConfigurationManagerPackageVersion>
<SystemReflectionMetadataLoadContextVersion>10.0.0-preview.7.25359.101</SystemReflectionMetadataLoadContextVersion>
<SystemResourcesExtensionsPackageVersion>10.0.0-preview.7.25359.101</SystemResourcesExtensionsPackageVersion>
Expand Down
20 changes: 20 additions & 0 deletions src/Cli/Microsoft.DotNet.Cli.Utils/Activities.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;

namespace Microsoft.DotNet.Cli.Utils;

/// <summary>
/// Contains helpers for working with <see cref="System.Diagnostics.Activity">Activities</see> in the .NET CLI.
/// </summary>
public static class Activities
{

/// <summary>
/// The main entrypoint for creating <see cref="Activity">Activities</see> in the .NET CLI.
/// All activities created in the CLI should use this <see cref="ActivitySource"/>, to allow
/// consumers to easily filter and trace CLI activities.
/// </summary>
public static ActivitySource Source { get; } = new("dotnet-cli", Product.Version);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class ForwardingAppImplementation
private readonly string? _depsFile;
private readonly string? _runtimeConfig;
private readonly string? _additionalProbingPath;
private Dictionary<string, string> _environmentVariables;
private Dictionary<string, string?> _environmentVariables;

private readonly string[] _allArgs;

Expand All @@ -29,7 +29,7 @@ public ForwardingAppImplementation(
string? depsFile = null,
string? runtimeConfig = null,
string? additionalProbingPath = null,
Dictionary<string, string>? environmentVariables = null)
Dictionary<string, string?>? environmentVariables = null)
{
_forwardApplicationPath = forwardApplicationPath;
_argsToForward = argsToForward;
Expand Down Expand Up @@ -86,7 +86,7 @@ public ProcessStartInfo GetProcessStartInfo()
return processInfo;
}

public ForwardingAppImplementation WithEnvironmentVariable(string name, string value)
public ForwardingAppImplementation WithEnvironmentVariable(string name, string? value)
{
_environmentVariables.Add(name, value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static string MSBuildVersion
// True if, given current state of the class, MSBuild would be executed in its own process.
public bool ExecuteMSBuildOutOfProc => _forwardingApp != null;

private readonly Dictionary<string, string> _msbuildRequiredEnvironmentVariables = GetMSBuildRequiredEnvironmentVariables();
private readonly Dictionary<string, string?> _msbuildRequiredEnvironmentVariables = GetMSBuildRequiredEnvironmentVariables();

private readonly List<string> _msbuildRequiredParameters = [ "-maxcpucount", "--verbosity:m" ];

Expand Down Expand Up @@ -112,7 +112,7 @@ private static string EmitProperty(KeyValuePair<string, string> property, string
: $"--{label}:{property.Key}={property.Value}";
}

public void EnvironmentVariable(string name, string value)
public void EnvironmentVariable(string name, string? value)
{
if (_forwardingApp != null)
{
Expand Down Expand Up @@ -153,7 +153,7 @@ public int ExecuteInProc(string[] arguments)
Dictionary<string, string?> savedEnvironmentVariables = [];
try
{
foreach (KeyValuePair<string, string> kvp in _msbuildRequiredEnvironmentVariables)
foreach (KeyValuePair<string, string?> kvp in _msbuildRequiredEnvironmentVariables)
{
savedEnvironmentVariables[kvp.Key] = Environment.GetEnvironmentVariable(kvp.Key);
Environment.SetEnvironmentVariable(kvp.Key, kvp.Value);
Expand Down Expand Up @@ -218,7 +218,7 @@ private static string GetDotnetPath()
return new Muxer().MuxerPath;
}

internal static Dictionary<string, string> GetMSBuildRequiredEnvironmentVariables()
internal static Dictionary<string, string?> GetMSBuildRequiredEnvironmentVariables()
{
return new()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
<PackageReference Include="Microsoft.Build" ExcludeAssets="runtime" PrivateAssets="all" />
<PackageReference Include="Microsoft.Build.Utilities.Core" ExcludeAssets="runtime" PrivateAssets="all" />
<PackageReference Include="System.CommandLine" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" />
<PackageReference Include="System.IO.Hashing" />
</ItemGroup>

Expand Down
2 changes: 1 addition & 1 deletion src/Cli/dotnet/Commands/MSBuild/MSBuildForwardingApp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public MSBuildForwardingApp(MSBuildArgs msBuildArgs, string? msbuildPath = null,

public IEnumerable<string> MSBuildArguments { get { return _forwardingAppWithoutLogging.GetAllArguments(); } }

public void EnvironmentVariable(string name, string value)
public void EnvironmentVariable(string name, string? value)
{
_forwardingAppWithoutLogging.EnvironmentVariable(name, value);
}
Expand Down
108 changes: 80 additions & 28 deletions src/Cli/dotnet/Telemetry/Telemetry.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable

using System.Collections.Frozen;
using System.Diagnostics;
using Microsoft.ApplicationInsights;
using Microsoft.ApplicationInsights.Extensibility;
Expand All @@ -14,27 +13,27 @@ namespace Microsoft.DotNet.Cli.Telemetry;

public class Telemetry : ITelemetry
{
internal static string CurrentSessionId = null;
internal static string? CurrentSessionId = null;
internal static bool DisabledForTests = false;
private readonly int _senderCount;
private TelemetryClient _client = null;
private Dictionary<string, string> _commonProperties = null;
private Dictionary<string, double> _commonMeasurements = null;
private Task _trackEventTask = null;
private TelemetryClient? _client = null;
private FrozenDictionary<string, string>? _commonProperties = null;
private FrozenDictionary<string, double>? _commonMeasurements = null;
private Task? _trackEventTask = null;

private const string ConnectionString = "InstrumentationKey=74cc1c9e-3e6e-4d05-b3fc-dde9101d0254";

public bool Enabled { get; }

public Telemetry() : this(null) { }

public Telemetry(IFirstTimeUseNoticeSentinel sentinel) : this(sentinel, null) { }
public Telemetry(IFirstTimeUseNoticeSentinel? sentinel) : this(sentinel, null) { }

public Telemetry(
IFirstTimeUseNoticeSentinel sentinel,
string sessionId,
IFirstTimeUseNoticeSentinel? sentinel,
string? sessionId,
bool blockThreadInitialization = false,
IEnvironmentProvider environmentProvider = null,
IEnvironmentProvider? environmentProvider = null,
int senderCount = 3)
{

Expand Down Expand Up @@ -78,7 +77,7 @@ internal static void EnableForTests()
DisabledForTests = false;
}

private static bool PermissionExists(IFirstTimeUseNoticeSentinel sentinel)
private static bool PermissionExists(IFirstTimeUseNoticeSentinel? sentinel)
{
if (sentinel == null)
{
Expand All @@ -97,9 +96,17 @@ public void TrackEvent(string eventName, IDictionary<string, string> properties,
}

//continue the task in different threads
_trackEventTask = _trackEventTask.ContinueWith(
x => TrackEventTask(eventName, properties, measurements)
);
if (_trackEventTask == null)
{
_trackEventTask = Task.Run(() => TrackEventTask(eventName, properties, measurements));
return;
}
else
{
_trackEventTask = _trackEventTask.ContinueWith(
x => TrackEventTask(eventName, properties, measurements)
);
}
}

public void Flush()
Expand Down Expand Up @@ -148,7 +155,7 @@ private void InitializeTelemetry()
_client.Context.Device.OperatingSystem = CLIRuntimeEnvironment.OperatingSystem;

_commonProperties = new TelemetryCommonProperties().GetTelemetryCommonProperties();
_commonMeasurements = [];
_commonMeasurements = FrozenDictionary<string, double>.Empty;
}
catch (Exception e)
{
Expand All @@ -170,39 +177,84 @@ private void TrackEventTask(

try
{
Dictionary<string, string> eventProperties = GetEventProperties(properties);
Dictionary<string, double> eventMeasurements = GetEventMeasures(measurements);
var eventProperties = GetEventProperties(properties);
var eventMeasurements = GetEventMeasures(measurements);

eventProperties ??= new Dictionary<string, string>();
eventProperties.Add("event id", Guid.NewGuid().ToString());

_client.TrackEvent(PrependProducerNamespace(eventName), eventProperties, eventMeasurements);
Activity.Current?.AddEvent(CreateActivityEvent(eventName, eventProperties, eventMeasurements));
}
catch (Exception e)
{
Debug.Fail(e.ToString());
}
}

private static string PrependProducerNamespace(string eventName)
private static ActivityEvent CreateActivityEvent(
string eventName,
IDictionary<string, string>? properties,
IDictionary<string, double>? measurements)
{
return "dotnet/cli/" + eventName;
var tags = MakeTags(properties, measurements);
return new ActivityEvent(
PrependProducerNamespace(eventName),
tags: tags);
}

private Dictionary<string, double> GetEventMeasures(IDictionary<string, double> measurements)
private static ActivityTagsCollection? MakeTags(
Copy link
Preview

Copilot AI Jul 12, 2025

Choose a reason for hiding this comment

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

[nitpick] The branching in MakeTags is repetitive. Consider consolidating the four branches into a single enumeration of properties and measurements to simplify the logic.

Copilot uses AI. Check for mistakes.

IDictionary<string, string>? properties,
IDictionary<string, double>? measurements)
{
Dictionary<string, double> eventMeasurements = new(_commonMeasurements);
if (measurements != null)
if (properties == null && measurements == null)
{
foreach (KeyValuePair<string, double> measurement in measurements)
{
eventMeasurements[measurement.Key] = measurement.Value;
}
return null;
}
else if (properties != null && measurements == null)
{
return [.. properties.Select(p => new KeyValuePair<string, object?>(p.Key, p.Value))];
}
else if (properties == null && measurements != null)
{
return [.. measurements.Select(m => new KeyValuePair<string, object?>(m.Key, m.Value.ToString()))];
}
else return [ .. properties!.Select(p => new KeyValuePair<string, object?>(p.Key, p.Value)),
.. measurements!.Select(m => new KeyValuePair<string, object?>(m.Key, m.Value.ToString())) ];
}

private static string PrependProducerNamespace(string eventName) => $"dotnet/cli/{eventName}";

private IDictionary<string, double>? GetEventMeasures(IDictionary<string, double>? measurements)
{
if (measurements is null)
{
return _commonMeasurements;
}
if (_commonMeasurements == null)
{
return measurements;
}
Comment on lines +234 to +237
Copy link
Contributor

Choose a reason for hiding this comment

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

_commonMeasurements is more likely to be FrozenDictionary<string, double>.Empty than null. This might be able to return measurements in that case too.


IDictionary<string, double> eventMeasurements = new Dictionary<string, double>(_commonMeasurements);
foreach (KeyValuePair<string, double> measurement in measurements)
{
eventMeasurements[measurement.Key] = measurement.Value;
}
return eventMeasurements;
}

private Dictionary<string, string> GetEventProperties(IDictionary<string, string> properties)
private IDictionary<string, string>? GetEventProperties(IDictionary<string, string>? properties)
{
if (properties is null)
{
return _commonProperties;
}
Comment on lines +249 to +252
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning _commonProperties as-is from here is wrong because the caller TrackEventTask would attempt to add "event id", causing the FrozenDictionary to throw NotSupportedException.

if (_commonProperties == null)
{
return properties;
}

var eventProperties = new Dictionary<string, string>(_commonProperties);
if (properties != null)
{
Expand Down
5 changes: 3 additions & 2 deletions src/Cli/dotnet/Telemetry/TelemetryCommonProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#nullable disable

using System.Collections.Frozen;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Configurer;
using RuntimeEnvironment = Microsoft.DotNet.Cli.Utils.RuntimeEnvironment;
Expand Down Expand Up @@ -52,7 +53,7 @@ internal class TelemetryCommonProperties(
private const string MachineIdCacheKey = "MachineId";
private const string IsDockerContainerCacheKey = "IsDockerContainer";

public Dictionary<string, string> GetTelemetryCommonProperties()
public FrozenDictionary<string, string> GetTelemetryCommonProperties()
{
return new Dictionary<string, string>
{
Expand Down Expand Up @@ -82,7 +83,7 @@ public Dictionary<string, string> GetTelemetryCommonProperties()
{ProductType, ExternalTelemetryProperties.GetProductType()},
{LibcRelease, ExternalTelemetryProperties.GetLibcRelease()},
{LibcVersion, ExternalTelemetryProperties.GetLibcVersion()}
};
}.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase);
}

private string GetMachineId()
Expand Down
Loading