Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 855a566

Browse files
shanashana
authored
Merge pull request #1467 from github/fixes/1466-metrics-fields-missing
Fix metrics, take 2135413
2 parents 9d5580e + a8ac83a commit 855a566

File tree

10 files changed

+1201
-376
lines changed

10 files changed

+1201
-376
lines changed

‎src/GitHub.Exports/Models/UsageModel.cs‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
using System;
2-
using System.Reflection;
3-
4-
namespace GitHub.Models
1+
namespace GitHub.Models
52
{
6-
public class UsageModel
3+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1815:OverrideEqualsAndOperatorEqualsOnValueTypes", Justification = "It'll use reflection by default and we're fine with that")]
4+
public struct UsageModel
75
{
86
public bool IsGitHubUser { get; set; }
97
public bool IsEnterpriseUser { get; set; }
@@ -37,30 +35,33 @@ public class UsageModel
3735
public int NumberOfPRDetailsViewFile { get; set; }
3836
public int NumberOfPRDetailsCompareWithSolution { get; set; }
3937
public int NumberOfPRDetailsOpenFileInSolution { get; set; }
38+
public int NumberOfPRDetailsNavigateToEditor { get; set; }
4039
public int NumberOfPRReviewDiffViewInlineCommentOpen { get; set; }
4140
public int NumberOfPRReviewDiffViewInlineCommentPost { get; set; }
4241

4342
public UsageModel Clone(bool includeWeekly, bool includeMonthly)
4443
{
45-
var result = new UsageModel();
46-
var properties = result.GetType().GetRuntimeProperties();
47-
48-
foreach (var property in properties)
49-
{
50-
var cloneValue = property.PropertyType == typeof(int);
51-
52-
if (property.Name == nameof(result.NumberOfStartupsWeek))
53-
cloneValue = includeWeekly;
54-
else if (property.Name == nameof(result.NumberOfStartupsMonth))
55-
cloneValue = includeMonthly;
44+
var result = this;
45+
if (!includeWeekly)
46+
result.NumberOfStartupsWeek = 0;
47+
if (!includeMonthly)
48+
result.NumberOfStartupsMonth = 0;
49+
return result;
50+
}
5651

57-
if (cloneValue)
58-
{
59-
var value = property.GetValue(this);
60-
property.SetValue(result, value);
61-
}
62-
}
52+
public UsageModel ClearCounters(bool clearWeekly, bool clearMonthly)
53+
{
54+
var result = new UsageModel();
55+
if (!clearWeekly)
56+
result.NumberOfStartupsWeek = NumberOfStartupsWeek;
57+
if (!clearMonthly)
58+
result.NumberOfStartupsMonth = NumberOfStartupsMonth;
6359

60+
result.IsGitHubUser = IsGitHubUser;
61+
result.IsEnterpriseUser = IsEnterpriseUser;
62+
result.AppVersion = AppVersion;
63+
result.VSVersion = VSVersion;
64+
result.Lang = Lang;
6465
return result;
6566
}
6667
}

‎src/GitHub.Exports/SimpleJson.cs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
using System.Runtime.Serialization;
6868
using System.Text;
6969
using GitHub.Reflection;
70+
using System.Diagnostics;
7071

7172
// ReSharper disable LoopCanBeConvertedToQuery
7273
// ReSharper disable RedundantExplicitArrayCreation
@@ -1838,7 +1839,13 @@ public static ConstructorDelegate GetConstructorByReflection(ConstructorInfo con
18381839
public static ConstructorDelegate GetConstructorByReflection(Type type, params Type[] argsType)
18391840
{
18401841
ConstructorInfo constructorInfo = GetConstructorInfo(type, argsType);
1841-
return constructorInfo == null ? null : GetConstructorByReflection(constructorInfo);
1842+
// if it's a value type (i.e., struct), it won't have a default constructor, so use Activator instead
1843+
return constructorInfo == null ? (type.IsValueType ? GetConstructorForValueType(type) : null) : GetConstructorByReflection(constructorInfo);
1844+
}
1845+
1846+
static ConstructorDelegate GetConstructorForValueType(Type type)
1847+
{
1848+
return delegate (object[] args) { return Activator.CreateInstance(type); };
18421849
}
18431850

18441851
#if !SIMPLE_JSON_NO_LINQ_EXPRESSION
@@ -1865,7 +1872,8 @@ public static ConstructorDelegate GetConstructorByExpression(ConstructorInfo con
18651872
public static ConstructorDelegate GetConstructorByExpression(Type type, params Type[] argsType)
18661873
{
18671874
ConstructorInfo constructorInfo = GetConstructorInfo(type, argsType);
1868-
return constructorInfo == null ? null : GetConstructorByExpression(constructorInfo);
1875+
// if it's a value type (i.e., struct), it won't have a default constructor, so use Activator instead
1876+
return constructorInfo == null ? (type.IsValueType ? GetConstructorForValueType(type) : null) : GetConstructorByExpression(constructorInfo);
18691877
}
18701878

18711879
#endif
@@ -1925,6 +1933,9 @@ public static SetDelegate GetSetMethod(PropertyInfo propertyInfo)
19251933
#if SIMPLE_JSON_NO_LINQ_EXPRESSION
19261934
return GetSetMethodByReflection(propertyInfo);
19271935
#else
1936+
// if it's a struct, we want to use reflection, as linq expressions modify copies of the object and not the real thing
1937+
if (propertyInfo.DeclaringType.IsValueType)
1938+
return GetSetMethodByReflection(propertyInfo);
19281939
return GetSetMethodByExpression(propertyInfo);
19291940
#endif
19301941
}
@@ -1934,6 +1945,9 @@ public static SetDelegate GetSetMethod(FieldInfo fieldInfo)
19341945
#if SIMPLE_JSON_NO_LINQ_EXPRESSION
19351946
return GetSetMethodByReflection(fieldInfo);
19361947
#else
1948+
// if it's a struct, we want to use reflection, as linq expressions modify copies of the object and not the real thing
1949+
if (fieldInfo.DeclaringType.IsValueType)
1950+
return GetSetMethodByReflection(fieldInfo);
19371951
return GetSetMethodByExpression(fieldInfo);
19381952
#endif
19391953
}

‎src/GitHub.Logging/Logging/LogManager.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ static Logger CreateLogger()
2020

2121
return new LoggerConfiguration()
2222
.Enrich.WithThreadId()
23+
#if DEBUG
24+
.MinimumLevel.Debug()
25+
#else
2326
.MinimumLevel.Information()
27+
#endif
2428
.WriteTo.File(logPath,
2529
fileSizeLimitBytes: null,
2630
outputTemplate: outputTemplate)

‎src/GitHub.VisualStudio/Services/UsageService.cs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,15 @@
88
using GitHub.Helpers;
99
using GitHub.Models;
1010
using Task = System.Threading.Tasks.Task;
11+
using GitHub.Logging;
12+
using Serilog;
1113

1214
namespace GitHub.Services
1315
{
1416
[Export(typeof(IUsageService))]
1517
public class UsageService : IUsageService
1618
{
19+
static readonly ILogger log = LogManager.ForContext<UsageService>();
1720
const string StoreFileName = "ghfvs.usage";
1821
static readonly Calendar cal = CultureInfo.InvariantCulture.Calendar;
1922
readonly IGitHubServiceProvider serviceProvider;
@@ -32,12 +35,12 @@ public bool IsSameDay(DateTimeOffset lastUpdated)
3235

3336
public bool IsSameWeek(DateTimeOffset lastUpdated)
3437
{
35-
return GetIso8601WeekOfYear(lastUpdated) == GetIso8601WeekOfYear(DateTimeOffset.Now);
38+
return GetIso8601WeekOfYear(lastUpdated) == GetIso8601WeekOfYear(DateTimeOffset.Now) && lastUpdated.Year == DateTimeOffset.Now.Year;
3639
}
3740

3841
public bool IsSameMonth(DateTimeOffset lastUpdated)
3942
{
40-
return lastUpdated.Month == DateTimeOffset.Now.Month;
43+
return lastUpdated.Month == DateTimeOffset.Now.Month && lastUpdated.Year == DateTimeOffset.Now.Year;
4144
}
4245

4346
public IDisposable StartTimer(Func<Task> callback, TimeSpan dueTime, TimeSpan period)
@@ -65,8 +68,9 @@ public async Task<UsageData> ReadLocalData()
6568
SimpleJson.DeserializeObject<UsageData>(json) :
6669
new UsageData { Model = new UsageModel() };
6770
}
68-
catch
71+
catch(Exception ex)
6972
{
73+
log.Error(ex, "Error deserializing usage");
7074
return new UsageData { Model = new UsageModel() };
7175
}
7276
}
@@ -79,9 +83,9 @@ public async Task WriteLocalData(UsageData data)
7983
var json = SimpleJson.SerializeObject(data);
8084
await WriteAllTextAsync(storePath, json);
8185
}
82-
catch
86+
catch(Exception ex)
8387
{
84-
// log.Warn("Failed to write usage data", ex);
88+
log.Error(ex,"Failed to write usage data");
8589
}
8690
}
8791

‎src/GitHub.VisualStudio/Services/UsageTracker.cs‎

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,13 @@ public void Dispose()
4444
public async Task IncrementCounter(Expression<Func<UsageModel, int>> counter)
4545
{
4646
var usage = await LoadUsage();
47+
// because Model is a struct, it needs to be boxed in order for reflection to work
48+
object model = usage.Model;
4749
var property = (MemberExpression)counter.Body;
4850
var propertyInfo = (PropertyInfo)property.Member;
49-
var value = (int)propertyInfo.GetValue(usage.Model);
50-
propertyInfo.SetValue(usage.Model, value + 1);
51+
var value = (int)propertyInfo.GetValue(model);
52+
propertyInfo.SetValue(model, value + 1);
53+
usage.Model = (UsageModel)model;
5154
await service.WriteLocalData(usage);
5255
}
5356

@@ -75,21 +78,25 @@ async Task Initialize()
7578
async Task IncrementLaunchCount()
7679
{
7780
var usage = await LoadUsage();
78-
++usage.Model.NumberOfStartups;
79-
++usage.Model.NumberOfStartupsWeek;
80-
++usage.Model.NumberOfStartupsMonth;
81+
var model = usage.Model;
82+
model.NumberOfStartups++;
83+
model.NumberOfStartupsWeek++;
84+
model.NumberOfStartupsMonth++;
85+
usage.Model = model;
8186
await service.WriteLocalData(usage);
8287
}
8388

8489
async Task<UsageData> LoadUsage()
8590
{
8691
await Initialize();
8792

88-
var result = await service.ReadLocalData();
89-
result.Model.Lang = CultureInfo.InstalledUICulture.IetfLanguageTag;
90-
result.Model.AppVersion = AssemblyVersionInformation.Version;
91-
result.Model.VSVersion = vsservices.VSVersion;
92-
return result;
93+
var usage = await service.ReadLocalData();
94+
var model = usage.Model;
95+
model.Lang = CultureInfo.InstalledUICulture.IetfLanguageTag;
96+
model.AppVersion = AssemblyVersionInformation.Version;
97+
model.VSVersion = vsservices.VSVersion;
98+
usage.Model = model;
99+
return usage;
93100
}
94101

95102
async Task TimerTick()
@@ -120,52 +127,39 @@ async Task TimerTick()
120127
// Only send stats once a day.
121128
if (!service.IsSameDay(usage.LastUpdated))
122129
{
130+
usage.Model = UpdateModelUserData(usage.Model);
131+
123132
await SendUsage(usage.Model, includeWeekly, includeMonthly);
124-
ClearCounters(usage.Model, includeWeekly, includeMonthly);
133+
134+
usage.Model = usage.Model.ClearCounters(includeWeekly, includeMonthly);
125135
usage.LastUpdated = DateTimeOffset.Now.UtcDateTime;
126136
await service.WriteLocalData(usage);
127137
}
128138
}
129139

130-
async Task SendUsage(UsageModel usage, bool weekly, bool monthly)
140+
async Task SendUsage(UsageModel usage, bool includeWeekly, bool includeMonthly)
131141
{
132142
if (client == null)
133143
{
134144
throw new GitHubLogicException("SendUsage should not be called when there is no IMetricsService");
135145
}
136146

137-
if (connectionManager.Connections.Any(x => x.HostAddress.IsGitHubDotCom()))
138-
{
139-
usage.IsGitHubUser = true;
140-
}
141-
142-
if (connectionManager.Connections.Any(x => !x.HostAddress.IsGitHubDotCom()))
143-
{
144-
usage.IsEnterpriseUser = true;
145-
}
146-
147-
var model = usage.Clone(weekly, monthly);
147+
var model = usage.Clone(includeWeekly, includeMonthly);
148148
await client.PostUsage(model);
149149
}
150150

151-
static void ClearCounters(UsageModel usage, bool weekly, bool monthly)
151+
UsageModel UpdateModelUserData(UsageModel model)
152152
{
153-
var properties = usage.GetType().GetRuntimeProperties();
154-
155-
foreach (var property in properties)
153+
if (connectionManager.Connections.Any(x => x.HostAddress.IsGitHubDotCom()))
156154
{
157-
var setValue = property.PropertyType == typeof(int);
158-
159-
if (property.Name == nameof(usage.NumberOfStartupsWeek))
160-
setValue = weekly;
161-
else if (property.Name == nameof(usage.NumberOfStartupsMonth))
162-
setValue = monthly;
155+
model.IsGitHubUser = true;
156+
}
163157

164-
if (setValue)
165-
{
166-
property.SetValue(usage, 0);
167-
}
158+
if (connectionManager.Connections.Any(x => !x.HostAddress.IsGitHubDotCom()))
159+
{
160+
model.IsEnterpriseUser = true;
168161
}
162+
return model;
169163
}
170164
}
171165
}

0 commit comments

Comments
 (0)