Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Commit

Permalink
Fix FxCop violations
Browse files Browse the repository at this point in the history
CA1037 Use StringComparison.Ordinal for StartsWith
CA1063 Seal classes with trivial Dispose methods
CA1305 Use InvariantCulture for ToString and string.Format
CA1309 Use ordinal comparison
CA1507 Use nameof
CA1801 Review unused parameters
CA1823 Remove unused fields
CA1825 Avoid zero length array allocations
CA2007 Call ConfigureAwait
  • Loading branch information
pharring committed Apr 29, 2018
1 parent 388ff8e commit e2db934
Show file tree
Hide file tree
Showing 22 changed files with 106 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public override bool Equals(object obj)
return false;
}

return this.Name == other.Name;
return string.Equals(this.Name, other.Name, System.StringComparison.Ordinal);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/EtwCollector/AITraceEventSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.ApplicationInsights.EtwCollector
/// <summary>
/// A wrapper of <see cref="Microsoft.Diagnostics.Tracing.Session.TraceEventSession"/>.
/// </summary>
internal class AITraceEventSession : ITraceEventSession, IDisposable
internal sealed class AITraceEventSession : ITraceEventSession, IDisposable
{
private TraceEventSession session;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ public string ApplicationName
{
[NonEvent]
get;

[NonEvent]
private set;
}

[Event(NoEventSourcesConfiguredEventId, Level = EventLevel.Warning, Keywords = Keywords.Configuration, Message = "No Sources configured for the {1}")]
Expand Down
2 changes: 1 addition & 1 deletion src/EventSourceListener/EventSourceListeningRequestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public override bool Equals(object obj)
return false;
}

return this.Name == other.Name && this.PrefixMatch == other.PrefixMatch;
return string.Equals(this.Name, other.Name, System.StringComparison.Ordinal) && this.PrefixMatch == other.PrefixMatch;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/EventSourceListener/EventSourceTelemetryModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ private void EnableAsNecessary(EventSource eventSource)
this.EnableEvents(eventSource, EventLevel.LogAlways, (EventKeywords)TplActivities.TaskFlowActivityIdsKeyword);
this.enabledEventSources.Enqueue(eventSource);
}
else if (eventSource.Name == EventSourceListenerEventSource.ProviderName)
else if (string.Equals(eventSource.Name, EventSourceListenerEventSource.ProviderName, StringComparison.Ordinal))
{
// Tracking ourselves does not make much sense
return;
Expand All @@ -236,7 +236,7 @@ private void EnableAsNecessary(EventSource eventSource)
// tasks, which then itself also fires Threadpool events on FrameworkEventSource at unexpected locations, and trigger deadlocks.
// Hence, we like special case this and mask out Threadpool events.
EventKeywords keywords = listeningRequest.Keywords;
if (listeningRequest.Name == "System.Diagnostics.Eventing.FrameworkEventSource")
if (string.Equals(listeningRequest.Name, "System.Diagnostics.Eventing.FrameworkEventSource", StringComparison.Ordinal))
{
// Turn off the Threadpool | ThreadTransfer keyword. Definition is at http://referencesource.microsoft.com/#mscorlib/system/diagnostics/eventing/frameworkeventsource.cs
// However, if keywords was to begin with, then we need to set it to All first, which is 0xFFFFF....
Expand Down
4 changes: 2 additions & 2 deletions src/NLogTarget/ApplicationInsightsTarget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ protected override void Write(LogEventInfo logEvent)
{
if (logEvent == null)
{
throw new ArgumentNullException("logEvent");
throw new ArgumentNullException(nameof(logEvent));
}

this.lastLogEventTime = DateTime.UtcNow;
Expand Down Expand Up @@ -207,7 +207,7 @@ private void PopulatePropertyBag(IDictionary<string, string> propertyBag, string
string value = valueObj.ToString();
if (propertyBag.ContainsKey(key))
{
if (value == propertyBag[key])
if (string.Equals(value, propertyBag[key], StringComparison.Ordinal))
{
return;
}
Expand Down
8 changes: 4 additions & 4 deletions src/TraceListener/ApplicationInsightsTraceListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public override void TraceEvent(TraceEventCache eventCache, string source, Trace
}

var trace = new TraceTelemetry(message);
this.CreateTraceData(eventCache, eventType, id, trace);
this.CreateTraceData(eventType, id, trace);
this.TelemetryClient.Track(trace);
}

Expand Down Expand Up @@ -145,7 +145,7 @@ public override void TraceData(TraceEventCache eventCache, string source, TraceE

string message = string.Join(", ", data.Select(d => d == null ? string.Empty : d.ToString()));
var trace = new TraceTelemetry(message);
this.CreateTraceData(eventCache, eventType, id, trace);
this.CreateTraceData(eventType, id, trace);
this.TelemetryClient.Track(trace);
}

Expand All @@ -167,7 +167,7 @@ public override void Write(string message)
}

var trace = new TraceTelemetry(message);
this.CreateTraceData(new TraceEventCache(), TraceEventType.Verbose, null, trace);
this.CreateTraceData(TraceEventType.Verbose, null, trace);
this.TelemetryClient.Track(trace);
}

Expand All @@ -188,7 +188,7 @@ public override void Flush()
this.TelemetryClient.Flush();
}

private void CreateTraceData(TraceEventCache eventCache, TraceEventType eventType, int? id, TraceTelemetry trace)
private void CreateTraceData(TraceEventType eventType, int? id, TraceTelemetry trace)
{
trace.SeverityLevel = this.GetSeverityLevel(eventType);

Expand Down
2 changes: 1 addition & 1 deletion test/CommonTestShared/SdkVersionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

public static class SdkVersionHelper
{
public static string GetExpectedSdkVersion(Type assemblyType, string prefix)
public static string GetExpectedSdkVersion(string prefix)
{
#if NET45 || NET46
string versionStr = typeof(SdkVersionHelper).Assembly.GetCustomAttributes(false).OfType<AssemblyFileVersionAttribute>().First().Version;
Expand Down
3 changes: 2 additions & 1 deletion test/CommonTestShared/TelemetrySender.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace Microsoft.ApplicationInsights.CommonTestShared
{
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Net;
Expand All @@ -25,7 +26,7 @@ public static string ValidateEndpointSend(ITelemetry telemetryItem)

HttpClient client = new HttpClient();
var result = client.PostAsync(
"https://dc.services.visualstudio.com/v2/validate",
new Uri("https://dc.services.visualstudio.com/v2/validate"),
new ByteArrayContent(Encoding.UTF8.GetBytes(json))).GetAwaiter().GetResult();

if (result.StatusCode != HttpStatusCode.OK)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace Microsoft.ApplicationInsights.DiagnosticSourceListener.Tests
using Microsoft.ApplicationInsights.Extensibility.Implementation;
using Microsoft.VisualStudio.TestTools.UnitTesting;

using static System.Globalization.CultureInfo;

[TestClass]
[TestCategory("DiagnosticSourceListener")]
public sealed class DiagnosticSourceTelemetryModuleTests : IDisposable
Expand Down Expand Up @@ -53,8 +55,8 @@ public void ReportsSingleEvent()
Assert.AreEqual("Hey!", telemetry.Message);
Assert.AreEqual(testDiagnosticSource.Name, telemetry.Properties["DiagnosticSource"]);
Assert.AreEqual(SeverityLevel.Information, telemetry.SeverityLevel);
Assert.AreEqual(1234.ToString(), telemetry.Properties["Prop1"]);
string expectedVersion = SdkVersionHelper.GetExpectedSdkVersion(typeof(DiagnosticSourceTelemetryModule), prefix: "dsl:");
Assert.AreEqual(1234.ToString(InvariantCulture), telemetry.Properties["Prop1"]);
string expectedVersion = SdkVersionHelper.GetExpectedSdkVersion(prefix: "dsl:");
Assert.AreEqual(expectedVersion, telemetry.Context.GetInternalContext().SdkVersion);
}
}
Expand Down
57 changes: 30 additions & 27 deletions test/EtwCollector.Net451.Tests/EtwTelemetryModuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ namespace Microsoft.ApplicationInsights.EtwTelemetryCollector.Tests
using Microsoft.Diagnostics.Tracing.Session;
using Microsoft.VisualStudio.TestTools.UnitTesting;

using static System.Globalization.CultureInfo;

[TestClass]
public class EtwTelemetryModuleTests : IDisposable
public sealed class EtwTelemetryModuleTests : IDisposable
{
private const int NoEventSourcesConfiguredEventId = 1;
private const int FailedToEnableProvidersEventId = 2;
private const int ModuleInitializationFailedEventId = 3;
private const int AccessDeniedEventId = 4;

Expand Down Expand Up @@ -53,7 +54,9 @@ private TelemetryConfiguration GetTestTelemetryConfiguration(bool resetChannel =
}

[ClassInitialize]
#pragma warning disable CA1801 // Review unused parameters
public static void InitializeClass(TestContext testContext)
#pragma warning restore CA1801 // Review unused parameters
{
// Only users with administrative privileges, users in the Performance Log Users group,
// and services running as LocalSystem, LocalService, or NetworkService can enable trace providers
Expand Down Expand Up @@ -253,7 +256,7 @@ public async Task ReportSingleEvent()

TestProvider.Log.Info("Hello!");
int expectedEventCount = 2;
await WaitForItems(adapterHelper.Channel, expectedEventCount);
await WaitForItems(adapterHelper.Channel, expectedEventCount).ConfigureAwait(false);

// The very 1st event is for the manifest.
Assert.AreEqual(expectedEventCount, this.adapterHelper.Channel.SentItems.Length);
Expand All @@ -278,7 +281,7 @@ public async Task ReportMultipleEvents()
TestProvider.Log.Info("World!");

int expectedEventCount = 3;
await WaitForItems(adapterHelper.Channel, expectedEventCount);
await WaitForItems(adapterHelper.Channel, expectedEventCount).ConfigureAwait(false);

// The very 1st event is for the manifest.
Assert.AreEqual(expectedEventCount, this.adapterHelper.Channel.SentItems.Length);
Expand Down Expand Up @@ -308,7 +311,7 @@ public async Task ReportsAllProperties()

// There's going to be a delay around 2000ms before the events reaches the channel.
int expectedEventCount = 2;
await this.WaitForItems(this.adapterHelper.Channel, expectedEventCount);
await this.WaitForItems(this.adapterHelper.Channel, expectedEventCount).ConfigureAwait(false);

Assert.AreEqual(expectedEventCount, this.adapterHelper.Channel.SentItems.Length);
TraceTelemetry actual = (TraceTelemetry)this.adapterHelper.Channel.SentItems[1];
Expand All @@ -317,11 +320,11 @@ public async Task ReportsAllProperties()
Assert.AreEqual("Blah blah", actual.Message);
Assert.AreEqual(SeverityLevel.Verbose, actual.SeverityLevel);
Assert.AreEqual(eventId.ToString(), actual.Properties["uniqueId"]);
Assert.AreEqual(TestProvider.ComplexEventId.ToString(), actual.Properties["EventId"]);
Assert.AreEqual(TestProvider.ComplexEventId.ToString(InvariantCulture), actual.Properties["EventId"]);
Assert.AreEqual(nameof(TestProvider.Complex) + "/Extension", actual.Properties["EventName"]);
Assert.AreEqual(activityId.ToString(), actual.Properties["ActivityID"]);
Assert.AreEqual("0x8000F00000000001", actual.Properties["Keywords"]);
Assert.AreEqual(((int)EventChannel.Debug).ToString(), actual.Properties["Channel"]);
Assert.AreEqual(((int)EventChannel.Debug).ToString(InvariantCulture), actual.Properties["Channel"]);
Assert.AreEqual("Extension", actual.Properties["Opcode"]);
Assert.AreEqual("0x00000020", actual.Properties["Task"]);
}
Expand All @@ -344,7 +347,7 @@ public async Task ReportSeverityLevel()
TestProvider.Log.Warning(1, 2);

int expectedEventCount = 3;
await this.WaitForItems(this.adapterHelper.Channel, expectedEventCount);
await this.WaitForItems(this.adapterHelper.Channel, expectedEventCount).ConfigureAwait(false);

// The very 1st event is for the manifest.
Assert.AreEqual(expectedEventCount, this.adapterHelper.Channel.SentItems.Length);
Expand All @@ -368,7 +371,7 @@ public async Task HandlesDuplicatePropertyNames()
TestProvider.Log.Tricky(7, "TrickyEvent", "Actual message");

int expectedEventCount = 2;
await this.WaitForItems(this.adapterHelper.Channel, expectedEventCount);
await this.WaitForItems(this.adapterHelper.Channel, expectedEventCount).ConfigureAwait(false);

Assert.AreEqual(expectedEventCount, this.adapterHelper.Channel.SentItems.Length);
TraceTelemetry telemetry = (TraceTelemetry)this.adapterHelper.Channel.SentItems[1];
Expand All @@ -378,8 +381,8 @@ public async Task HandlesDuplicatePropertyNames()
Assert.AreEqual("Actual message", telemetry.Properties["Message"]);
Assert.AreEqual("7", telemetry.Properties["EventId"]);
Assert.AreEqual("Tricky", telemetry.Properties["EventName"]);
Assert.AreEqual("7", telemetry.Properties[telemetry.Properties.Keys.First(key => key.StartsWith("EventId") && key != "EventId")]);
Assert.AreEqual("TrickyEvent", telemetry.Properties[telemetry.Properties.Keys.First(key => key.StartsWith("EventName") && key != "EventName")]);
Assert.AreEqual("7", telemetry.Properties[telemetry.Properties.Keys.First(key => key.StartsWith("EventId", StringComparison.Ordinal) && !string.Equals(key, "EventId", StringComparison.Ordinal))]);
Assert.AreEqual("TrickyEvent", telemetry.Properties[telemetry.Properties.Keys.First(key => key.StartsWith("EventName", StringComparison.Ordinal) && !string.Equals(key, "EventName", StringComparison.Ordinal))]);
}
}

Expand All @@ -397,34 +400,34 @@ public async Task ReactsToConfigurationChanges()

TestProvider.Log.Info("Hey!");
TestProvider.Log.Warning(1, 2);
await this.WaitForItems(this.adapterHelper.Channel, 3);
await this.WaitForItems(this.adapterHelper.Channel, 3).ConfigureAwait(false);

// Now request reporting events only with certain keywords
listeningRequest.Keywords = (ulong)TestProvider.Keywords.NonRoutine;
module.Initialize(GetTestTelemetryConfiguration(resetChannel: false));
await Task.Delay(500);
await Task.Delay(500).ConfigureAwait(false);

TestProvider.Log.Info("Hey again!");
TestProvider.Log.Warning(3, 4);
await this.WaitForItems(this.adapterHelper.Channel, 5);
await this.WaitForItems(this.adapterHelper.Channel, 5).ConfigureAwait(false);

List<TraceTelemetry> expectedTelemetry = new List<TraceTelemetry>();
TraceTelemetry traceTelemetry = new TraceTelemetry("Hey!", SeverityLevel.Information);
traceTelemetry.Properties["information"] = "Hey!";
expectedTelemetry.Add(traceTelemetry);
traceTelemetry = new TraceTelemetry("Warning!", SeverityLevel.Warning);
traceTelemetry.Properties["i1"] = 1.ToString();
traceTelemetry.Properties["i2"] = 2.ToString();
traceTelemetry.Properties["i1"] = 1.ToString(InvariantCulture);
traceTelemetry.Properties["i2"] = 2.ToString(InvariantCulture);
expectedTelemetry.Add(traceTelemetry);
// Note that second informational event is not expected
traceTelemetry = new TraceTelemetry("Warning!", SeverityLevel.Warning);
traceTelemetry.Properties["i1"] = 3.ToString();
traceTelemetry.Properties["i2"] = 4.ToString();
traceTelemetry.Properties["i1"] = 3.ToString(InvariantCulture);
traceTelemetry.Properties["i2"] = 4.ToString(InvariantCulture);
expectedTelemetry.Add(traceTelemetry);

CollectionAssert.AreEqual(
expectedTelemetry,
this.adapterHelper.Channel.SentItems.Where(item => !((TraceTelemetry)item).Properties["EventId"].Equals("65534")).ToList(),
this.adapterHelper.Channel.SentItems.Where(item => !((TraceTelemetry)item).Properties["EventId"].Equals("65534", StringComparison.Ordinal)).ToList(),
new TraceTelemetryComparer(),
"Reported events are not what was expected");
}
Expand All @@ -450,7 +453,7 @@ public async Task DoNotReportTplEvents()
// Wait 2 seconds to see if any events arrive asynchronously through the ETW module.
// This is a long time but unfortunately there is no good way to make ETW infrastructure "go faster"
// and we want to make sure that no TPL events sneak through.
await this.WaitForItems(this.adapterHelper.Channel, 1, TimeSpan.FromSeconds(2));
await this.WaitForItems(this.adapterHelper.Channel, 1, TimeSpan.FromSeconds(2)).ConfigureAwait(false);

Assert.AreEqual(0, this.adapterHelper.Channel.SentItems.Length);
}
Expand All @@ -476,21 +479,21 @@ public async Task ReportsHierarchicalActivities()
});
}

await this.WaitForItems(this.adapterHelper.Channel, 12);
await this.WaitForItems(this.adapterHelper.Channel, 12).ConfigureAwait(false);

ITelemetry[] capturedItems = this.adapterHelper.Channel.SentItems;
TraceTelemetry requestStopEvent = capturedItems.OfType<TraceTelemetry>().FirstOrDefault((i) =>
i.Properties.TryGetValue("EventName", out string eventName)
&& eventName == "Request/Stop");
&& string.Equals(eventName, "Request/Stop", StringComparison.Ordinal));
Assert.IsNotNull(requestStopEvent, "Request/Stop event not found");
Assert.IsTrue(requestStopEvent.Properties.TryGetValue("ActivityID", out string activityID), "Event does not have ActivityID property");
Assert.IsTrue(activityID.StartsWith("//"), "The activity ID is not a hierarchical one");
Assert.IsTrue(activityID.StartsWith("//", StringComparison.Ordinal), "The activity ID is not a hierarchical one");
}
}

private async Task PerformActivityAsync(int requestId)
private Task PerformActivityAsync(int requestId)
{
await Task.Run(async () =>
return Task.Run(async () =>
{
TestProvider.Log.RequestStart(requestId);
await Task.Delay(50).ConfigureAwait(false);
Expand Down Expand Up @@ -520,7 +523,7 @@ private async Task WaitForItems(CustomTelemetryChannel channel, int count = 1, T
return;
}

itemsCaptured = await channel.WaitForItemsCaptured(timeout.Value);
itemsCaptured = await channel.WaitForItemsCaptured(timeout.Value).ConfigureAwait(false);
if (itemsCaptured == null)
{
// Timed out waiting for new events to arrive.
Expand All @@ -536,7 +539,7 @@ public void Cleanup()
try
{
// This clean up is there to clean up possible left over trace event sessions during the Debug of the unit tests.
foreach (var name in TraceEventSession.GetActiveSessionNames().Where(n => n.StartsWith("ApplicationInsights-")))
foreach (var name in TraceEventSession.GetActiveSessionNames().Where(n => n.StartsWith("ApplicationInsights-", StringComparison.Ordinal)))
{
TraceEventSession.GetActiveSession(name).Stop();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected override void OnEventWritten(EventWrittenEventArgs eventData)

protected override void OnEventSourceCreated(EventSource eventSource)
{
if (eventSource.Name == "Microsoft-ApplicationInsights-Extensibility-EventSourceListener")
if (string.Equals(eventSource.Name, "Microsoft-ApplicationInsights-Extensibility-EventSourceListener", System.StringComparison.Ordinal))
{
EnableEvents(eventSource, EventLevel.LogAlways);
}
Expand Down
Loading

0 comments on commit e2db934

Please sign in to comment.