Skip to content
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

Add support for vs code markers #3399

Merged
merged 8 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 7 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 build/packages.targets
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
<PackageReference Update="Microsoft.VisualStudio.Threading" Version="$(VSThreadingVersion)" />
<PackageReference Update="Microsoft.VisualStudio.Threading.Analyzers" Version="$(VSThreadingVersion)" />
<PackageReference Update="Microsoft.VisualStudio.Workspace.VSIntegration" Version="16.3.43" />
<PackageReference Update="Microsoft.VisualStudio.Utilities" Version="16.5.29714.20" />
<PackageReference Update="Microsoft.Web.Xdt" Version="2.1.2" />
<PackageReference Update="Newtonsoft.Json" Version="$(NewtonsoftJsonPackageVersion)" />
<PackageReference Update="System.Collections.Immutable" Version="1.5.0" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,4 @@
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void OutputConsoleLogger.Log(ILogMessage message)', validate parameter 'message' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.VisualStudio.Common.OutputConsoleLogger.Log(NuGet.Common.ILogMessage)")]
[assembly: SuppressMessage("Build", "CA1823:Unused field 'LogEntrySource'.", Justification = "<Pending>", Scope = "member", Target = "~F:NuGet.VisualStudio.Common.OutputConsoleLogger.LogEntrySource")]
[assembly: SuppressMessage("Build", "CA1063:Provide an overridable implementation of Dispose(bool) on 'OutputConsoleLogger' or mark the type as sealed. A call to Dispose(false) should only clean up native resources. A call to Dispose(true) should clean up both managed and native resources.", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.VisualStudio.Common.OutputConsoleLogger")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "Need to unify event names to be same as ones produced from telemetry.", Scope = "member", Target = "~M:NuGet.VisualStudio.NuGetVSTelemetryService.StartActivity(System.String)~System.IDisposable")]
Copy link
Member

Choose a reason for hiding this comment

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

this rule is getting annoying :)

I get why it's trying to promote and why it's less error prone, but there's so much infrastructure that depends normalizes to lowercase when the culture is known.

Thoughts @NuGet/nuget-client Should we disable this in a future PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think it is a good candidate for repo-wide suppression 🙂

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
<PackageReference Include="Microsoft.VisualStudio.ImageCatalog" />
<PackageReference Include="Microsoft.VisualStudio.Threading" />
<PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers" />
<PackageReference Include="Microsoft.VisualStudio.Utilities" />
<PackageReference Include="Newtonsoft.Json" NoWarn="NU1605" />
<PackageReference Include="VSLangProj" />
<PackageReference Include="System.Collections.Immutable" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using Microsoft.Internal.VisualStudio.Diagnostics;
using NuGet.Common;
using NuGet.VisualStudio.Telemetry;

Expand Down Expand Up @@ -33,5 +34,33 @@ public virtual void EmitTelemetryEvent(TelemetryEvent telemetryData)

_telemetrySession.PostEvent(telemetryData);
}

public virtual IDisposable StartActivity(string activityName)
{
if (activityName == null)
{
throw new ArgumentNullException(nameof(activityName));
}

return new EtwLogActivity((VSTelemetrySession.VSEventNamePrefix + activityName).ToLowerInvariant().Replace('/', '_'));
}

private class EtwLogActivity : IDisposable
{
private readonly VsEtwActivity _activity;

public EtwLogActivity(string activityName)
{
if (VsEtwLogging.IsProviderEnabled(VsEtwKeywords.Ide, VsEtwLevel.Information))
{
_activity = VsEtwLogging.CreateActivity(activityName, VsEtwKeywords.Ide, VsEtwLevel.Information);
}
}

void IDisposable.Dispose()
{
_activity?.Dispose();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.


using System;

namespace NuGet.Common
{
/// <summary> Abstraction of NuGet telemetry service. </summary>
public interface INuGetTelemetryService
{
/// <summary> Send a <see cref="TelemetryEvent"/> to VS telemetry. </summary>
/// <param name="telemetryData"> Telemetry event to send. </param>
void EmitTelemetryEvent(TelemetryEvent telemetryData);
srdjanjovcic marked this conversation as resolved.
Show resolved Hide resolved

/// <summary> Log a start of telemetry activity to the event log. </summary>
/// <param name="activityName"> Name of telemetry activity to log. </param>
/// <returns> <see cref="IDisposable"/> which will log end activity marker. </returns>
IDisposable StartActivity(string activityName);
srdjanjovcic marked this conversation as resolved.
Show resolved Hide resolved
}
}
7 changes: 7 additions & 0 deletions src/NuGet.Core/NuGet.Common/Telemetry/TelemetryActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class TelemetryActivity : IDisposable
private readonly Stopwatch _stopwatch;
private readonly Stopwatch _intervalWatch = new Stopwatch();
private readonly List<Tuple<string, TimeSpan>> _intervalList;
private readonly IDisposable _telemetryActivity;

public TelemetryEvent TelemetryEvent { get; set; }

Expand Down Expand Up @@ -49,6 +50,11 @@ private TelemetryActivity(Guid parentId, TelemetryEvent telemetryEvent, Guid ope
_startTime = DateTime.UtcNow;
_stopwatch = Stopwatch.StartNew();
_intervalList = new List<Tuple<string, TimeSpan>>();

if (telemetryEvent != null)
{
_telemetryActivity = NuGetTelemetryService?.StartActivity(telemetryEvent.Name);
}
}

public void StartIntervalMeasure()
Expand All @@ -64,6 +70,7 @@ public void EndIntervalMeasure(string propertyName)

public void Dispose()
{
_telemetryActivity?.Dispose();
_stopwatch.Stop();

if (NuGetTelemetryService != null && TelemetryEvent != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,25 @@ namespace NuGet.Common.Test
{
public class TelemetryActivityTests
{
private readonly Mock<INuGetTelemetryService> _telemetryService = new Mock<INuGetTelemetryService>();
private readonly Mock<INuGetTelemetryService> _telemetryService = new Mock<INuGetTelemetryService>(MockBehavior.Strict);
private TelemetryEvent _telemetryEvent;

private readonly Mock<IDisposable> _activity = new Mock<IDisposable>(MockBehavior.Strict);
private bool _activityDisposed;
private string _activityName;

public TelemetryActivityTests()
{
_telemetryService.Setup(x => x.EmitTelemetryEvent(It.IsAny<TelemetryEvent>()))
.Callback<TelemetryEvent>(x => _telemetryEvent = x);

_telemetryService.Setup(x => x.StartActivity(It.IsAny<string>()))
.Callback<string>(x => _activityName = x)
.Returns(_activity.Object);

_activity.Setup(x => x.Dispose())
.Callback(() => _activityDisposed = true);

TelemetryActivity.NuGetTelemetryService = _telemetryService.Object;
}

Expand Down Expand Up @@ -125,6 +136,26 @@ public void Dispose_Always_EmitsDuration()
Assert.InRange(duration, 0d, 10d);
}

[Fact]
public void Create_will_start_activity()
{
using (var telemetry = TelemetryActivity.Create(CreateNewTelemetryEvent()))
{
}

Assert.Equal("testEvent", _activityName);
}

[Fact]
public void Dispose_will_dispose_activity()
{
using (var telemetry = TelemetryActivity.Create(CreateNewTelemetryEvent()))
{
}

Assert.True(_activityDisposed);
}

private static TelemetryEvent CreateNewTelemetryEvent()
{
return new TelemetryEvent("testEvent", new Dictionary<string, object>());
Expand Down