Skip to content

Commit

Permalink
Fixing integer overflow issue in RTS metrics (Azure#6539)
Browse files Browse the repository at this point in the history
  • Loading branch information
mathewc authored Aug 19, 2020
1 parent 9ff9c33 commit eed976d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 4 deletions.
4 changes: 3 additions & 1 deletion src/WebJobs.Script.WebHost/Scale/TableEntityConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ public static bool TryGetEntityProperty(JProperty property, out EntityProperty e
entityProperty = new EntityProperty(value.ToObject<Guid>());
return true;
case JTokenType.Integer:
entityProperty = new EntityProperty(value.ToObject<int>());
// to handle both ints and longs, we normalize integer values
// to type long
entityProperty = new EntityProperty(value.ToObject<long>());
return true;
case JTokenType.String:
entityProperty = new EntityProperty(value.ToObject<string>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace Microsoft.Azure.WebJobs.Script.WebHost
public class TableStorageScaleMetricsRepository : IScaleMetricsRepository
{
internal const string TableNamePrefix = "AzureFunctionsScaleMetrics";
private const string MonitorIdPropertyName = "MonitorId";
internal const string MonitorIdPropertyName = "MonitorId";
private const string SampleTimestampPropertyName = "SampleTimestamp";
private const int MetricsPurgeDelaySeconds = 30;
private const int DefaultTableCreationRetries = 3;
Expand Down Expand Up @@ -272,7 +272,7 @@ internal static TableOperation CreateMetricsInsertOperation(ScaleMetrics metrics

// Use an inverted ticks rowkey to order the table in descending order, allowing us to easily
// query for latest logs. Adding a guid as part of the key to ensure uniqueness.
string rowKey = string.Format("{0:D19}-{1}", DateTime.MaxValue.Ticks - now.Value.Ticks, Guid.NewGuid());
string rowKey = GetRowKey(now.Value);

var entity = TableEntityConverter.ToEntity(metrics, hostId, rowKey, metrics.Timestamp);
entity.Properties.Add(MonitorIdPropertyName, EntityProperty.GeneratePropertyForString(descriptor.Id));
Expand All @@ -285,6 +285,11 @@ internal static TableOperation CreateMetricsInsertOperation(ScaleMetrics metrics
return TableOperation.Insert(entity);
}

internal static string GetRowKey(DateTime now)
{
return string.Format("{0:D19}-{1}", DateTime.MaxValue.Ticks - now.Ticks, Guid.NewGuid());
}

internal async Task<IEnumerable<DynamicTableEntity>> ExecuteQuerySafeAsync(CloudTable metricsTable, TableQuery query)
{
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,78 @@ public async Task WriteMetricsAsync_PersistsMetrics()
Assert.Equal(0, result.Count);
}

[Fact]
public async Task ReadWriteMetrics_IntegerConversion_HandlesLongs()
{
var monitor1 = new TestScaleMonitor1();
var monitors = new IScaleMonitor[] { monitor1 };

// first write a couple entities manually to the table to simulate
// the change in entity property type (int -> long)
// this shows that the table can have entities of both formats with
// no versioning issues

// add an entity with Count property of type int
var entity = new DynamicTableEntity
{
RowKey = TableStorageScaleMetricsRepository.GetRowKey(DateTime.UtcNow),
PartitionKey = TestHostId,
Properties = new Dictionary<string, EntityProperty>()
};
var expectedIntCountValue = int.MaxValue;
entity.Properties.Add("Timestamp", new EntityProperty(DateTime.UtcNow));
entity.Properties.Add("Count", new EntityProperty(expectedIntCountValue));
entity.Properties.Add(TableStorageScaleMetricsRepository.MonitorIdPropertyName, EntityProperty.GeneratePropertyForString(monitor1.Descriptor.Id));
var batch = new TableBatchOperation();
batch.Add(TableOperation.Insert(entity));

// add an entity with Count property of type long
entity = new DynamicTableEntity
{
RowKey = TableStorageScaleMetricsRepository.GetRowKey(DateTime.UtcNow),
PartitionKey = TestHostId,
Properties = new Dictionary<string, EntityProperty>()
};
var expectedLongCountValue = long.MaxValue;
entity.Properties.Add("Timestamp", new EntityProperty(DateTime.UtcNow));
entity.Properties.Add("Count", new EntityProperty(expectedLongCountValue));
entity.Properties.Add(TableStorageScaleMetricsRepository.MonitorIdPropertyName, EntityProperty.GeneratePropertyForString(monitor1.Descriptor.Id));
batch.Add(TableOperation.Insert(entity));

await _repository.ExecuteBatchSafeAsync(batch);

// push a long max value through serialization
var metricsMap = new Dictionary<IScaleMonitor, ScaleMetrics>();
metricsMap.Add(monitor1, new TestScaleMetrics1 { Count = long.MaxValue });
await _repository.WriteMetricsAsync(metricsMap);

// add one more
metricsMap = new Dictionary<IScaleMonitor, ScaleMetrics>();
metricsMap.Add(monitor1, new TestScaleMetrics1 { Count = 12345 });
await _repository.WriteMetricsAsync(metricsMap);

// read the metrics back
var result = await _repository.ReadMetricsAsync(monitors);
Assert.Equal(1, result.Count);
var monitorMetricsList = result[monitor1];
Assert.Equal(4, monitorMetricsList.Count);

// verify the explicitly written int record was read correctly
var currSample = (TestScaleMetrics1)monitorMetricsList[0];
Assert.Equal(expectedIntCountValue, currSample.Count);

// verify the explicitly written long record was read correctly
currSample = (TestScaleMetrics1)monitorMetricsList[1];
Assert.Equal(expectedLongCountValue, currSample.Count);

// verify the final roundtripped values
currSample = (TestScaleMetrics1)monitorMetricsList[2];
Assert.Equal(long.MaxValue, currSample.Count);

currSample = (TestScaleMetrics1)monitorMetricsList[3];
Assert.Equal(12345, currSample.Count);
}

[Fact]
public async Task ReadMetricsAsync_FiltersExpiredMetrics()
{
Expand Down
2 changes: 1 addition & 1 deletion test/WebJobs.Script.Tests.Shared/TestScaleMonitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public class TestScaleMonitor1 : TestScaleMonitor<TestScaleMetrics1>

public class TestScaleMetrics1 : ScaleMetrics
{
public int Count { get; set; }
public long Count { get; set; }
}

public class TestScaleMonitor2 : TestScaleMonitor<TestScaleMetrics2>
Expand Down

0 comments on commit eed976d

Please sign in to comment.