Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Text;

namespace Microsoft.Dotnet.Installation;

public record DotnetInstallRoot(
string Path,
InstallArchitecture Architecture);
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Formats.Tar;
using System.IO;
using System.IO.Compression;
Expand Down Expand Up @@ -30,6 +31,8 @@ public ArchiveDotnetExtractor(DotnetInstallRequest request, ReleaseVersion resol

public void Prepare()
{
using var activity = InstallationActivitySource.ActivitySource.StartActivity("DotnetInstaller.Prepare");

using var releaseManifest = new ReleaseManifest();
var archiveName = $"dotnet-{Guid.NewGuid()}";
_archivePath = Path.Combine(scratchDownloadDirectory, archiveName + DnupUtilities.GetArchiveFileExtensionForPlatform());
Expand Down Expand Up @@ -90,6 +93,8 @@ public void Commit()

public void Commit(IEnumerable<ReleaseVersion> existingSdkVersions)
{
using var activity = InstallationActivitySource.ActivitySource.StartActivity("DotnetInstaller.Commit");
Copy link
Member

Choose a reason for hiding this comment

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

Should we be using a try-catch pattern to log exceptions which are thrown? I don't know if that's done automatically.

catch (Exception ex)
{
    activity?.SetStatus(ActivityStatusCode.Error);
    activity?.AddEvent(new ActivityEvent("exception", tags: new ActivityTagsCollection {
        { "exception.type", ex.GetType().FullName },
        { "exception.message", ex.Message },
        { "exception.stacktrace", ex.ToString() }
    }));
    throw;
}

Copy link
Member

Choose a reason for hiding this comment

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

The good news is that this is automatically done for us. The BCL listeners already catch and finalize activities where exceptions are thrown in much the same way that you've suggested here. The whole system does so much correct out of the box for you, it's really a joy.

Copy link
Member

Choose a reason for hiding this comment

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

That is awesome!

Copy link
Contributor

Choose a reason for hiding this comment

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

The BCL listeners already catch and finalize activities where exceptions are thrown in much the same way that you've suggested here.

I searched for ActivityStatusCode.Error references at https://source.dot.net/ but I don't see any activity listeners setting that. AFAICT, it's instead done by the code that has an ActivitySource and starts and stops an Activity. I therefore don't agree with "this is automatically done for us".


if (_archivePath == null || !File.Exists(_archivePath))
{
throw new InvalidOperationException("Archive not found. Make sure Prepare() was called successfully.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;
using Microsoft.Deployment.DotNet.Releases;

Expand All @@ -19,6 +20,12 @@ public DotnetInstaller(IProgressTarget progressTarget)

public void Install(DotnetInstallRoot dotnetRoot, InstallComponent component, ReleaseVersion version)
{
using var activity = InstallationActivitySource.ActivitySource.StartActivity("DotnetInstaller.Install");
activity?.SetTag("install.root", dotnetRoot.Path);
activity?.SetTag("install.arch", dotnetRoot.Architecture.ToString());
activity?.SetTag("install.component", component.ToString());
activity?.SetTag("install.version", version.ToString());

var installRequest = new DotnetInstallRequest(dotnetRoot, new UpdateChannel(version.ToString()), component, new InstallRequestOptions());

using ArchiveDotnetExtractor installer = new(installRequest, version, _progressTarget);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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;
using System.Reflection;

namespace Microsoft.Dotnet.Installation.Internal;

internal static class InstallationActivitySource
{
private static readonly ActivitySource s_activitySource = new("Microsoft.Dotnet.Installer",
typeof(InstallationActivitySource).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion ?? "1.0.0");

public static ActivitySource ActivitySource => s_activitySource;
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ static IEnumerable<string> GetChannelsForProduct(Product product)
.Select(v => $"{v.Major}.{v.Minor}.{(v.Patch / 100)}xx")
.Distinct()
.ToList()
];
];
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.Deployment.DotNet.Releases" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" />
</ItemGroup>

</Project>