Skip to content

Commit

Permalink
Reduce AWS sdk calls (justeattakeaway#289)
Browse files Browse the repository at this point in the history
* Cache cloudformation calls

* Don't check topics exist before creating

* Newer queue data source wraps first

To avoid duplicating fetches

* ioc lifecycle changes for caching

* remove duplicate ioc registration

* remove unused method

* formatting

* Revert "remove duplicate ioc registration"

This reverts commit ac9a4cd.
  • Loading branch information
tomhaigh authored May 8, 2019
1 parent 303326f commit 82e3a9d
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void Setup()
.ReturnsAsync(_thirdPage);


_queueSource = new QueueDataV2Source(cloudWatchMock.Object);
_queueSource = new QueueDataV2Source(new QueueSource(cloudWatchMock.Object));
}

[Test]
Expand Down
50 changes: 5 additions & 45 deletions Watchman.AwsResources/Services/Sqs/QueueDataV2Source.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,56 +8,16 @@ namespace Watchman.AwsResources.Services.Sqs
{
public class QueueDataV2Source : ResourceSourceBase<QueueDataV2>
{
private readonly IAmazonCloudWatch _amazonCloudWatch;
private readonly QueueSource _queueSource;

public QueueDataV2Source(IAmazonCloudWatch amazonCloudWatch)
public QueueDataV2Source(QueueSource queueSource)
{
_amazonCloudWatch = amazonCloudWatch;
_queueSource = queueSource;
}

private async Task<IList<string>> ReadActiveQueueNames()
private Task<IList<string>> ReadActiveQueueNames()
{
var metrics = await ReadQueueMetrics();

return metrics
.SelectMany(ExtractQueueNames)
.OrderBy(qn => qn)
.Distinct()
.ToList();
}

private async Task<List<Metric>> ReadQueueMetrics()
{
var metrics = new List<Metric>();
string token = null;
do
{
var request = new ListMetricsRequest
{
MetricName = "ApproximateAgeOfOldestMessage",
NextToken = token
};
var response = await _amazonCloudWatch.ListMetricsAsync(request);

if (response != null)
{
token = response.NextToken;
metrics.AddRange(response.Metrics);
}
else
{
token = null;
}
} while (token != null);

return metrics;
}

private static IEnumerable<string> ExtractQueueNames(Metric metric)
{
return metric.Dimensions
.Where(d => d.Name == "QueueName")
.Select(d => d.Value);
return _queueSource.GetResourceNamesAsync();
}

protected override string GetResourceName(QueueDataV2 resource)
Expand Down
35 changes: 3 additions & 32 deletions Watchman.Engine.Tests/Sns/SnsTopicCreatorTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Threading;
using System.Threading;
using Amazon.SimpleNotificationService;
using Amazon.SimpleNotificationService.Model;
using Moq;
Expand Down Expand Up @@ -28,28 +28,7 @@ public async Task HappyPathShouldCreateTopic()

client.Verify(c => c.CreateTopicAsync("test1-Alerts", It.IsAny<CancellationToken>()), Times.Once);
}

[Test]
public async Task WhenTopicExistsShouldNotCreateTopic()
{
var client = new Mock<IAmazonSimpleNotificationService>();
client.Setup(c => c.FindTopicAsync(It.IsAny<string>()))
.ReturnsAsync(TestFindTopicResponse());

MockCreateTopic(client,TestCreateTopicResponse());

var logger = new Mock<IAlarmLogger>();
var snsTopicCreator = new SnsTopicCreator(client.Object, logger.Object);

var topicArn = await snsTopicCreator.EnsureSnsTopic("test1", false);

Assert.That(topicArn, Is.Not.Null);
Assert.That(topicArn, Is.EqualTo("existingTopicArn-abc-1234"));

client.Verify(c => c.CreateTopicAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()),
Times.Never);
}


[Test]
public async Task DryRunShouldNotCreateTopic()
{
Expand All @@ -74,15 +53,7 @@ private static CreateTopicResponse TestCreateTopicResponse()
TopicArn = "testResponse-abc123"
};
}

private Topic TestFindTopicResponse()
{
return new Topic
{
TopicArn = "existingTopicArn-abc-1234"
};
}


private void MockCreateTopic(Mock<IAmazonSimpleNotificationService> client,
CreateTopicResponse response)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,15 @@ public CloudFormationStackDeployer(
_s3Location = s3Location;
}

private async Task<IEnumerable<string>> AllStacks()
private IReadOnlyList<string> _stacks;

private async Task<IReadOnlyList<string>> AllStacks()
{
if (_stacks != null)
{
return _stacks;
}

string nextToken = null;
var results = new List<string>();

Expand All @@ -96,6 +103,8 @@ private async Task<IEnumerable<string>> AllStacks()
results.AddRange(allStacks.StackSummaries.Select(x => x.StackName));
} while (nextToken != null);

_stacks = results;

return results;
}

Expand Down
11 changes: 0 additions & 11 deletions Watchman.Engine/Sns/SnsCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,5 @@ public async Task<string> EnsureSnsTopic(AlertingGroup alertingGroup, bool dryRu
}
return snsTopicArn;
}

public async Task<string> EnsureSnsTopic(IServiceAlertingGroup alertingGroup, bool dryRun)
{
var snsTopicArn = await _snsTopicCreator.EnsureSnsTopic(alertingGroup.GroupParameters.Name, dryRun);

if (!dryRun)
{
await _snsSubscriptionCreator.EnsureSnsSubscriptions(alertingGroup.GroupParameters.Targets, snsTopicArn);
}
return snsTopicArn;
}
}
}
10 changes: 3 additions & 7 deletions Watchman.Engine/Sns/SnsTopicCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,16 @@ public SnsTopicCreator(IAmazonSimpleNotificationService snsClient,
public async Task<string> EnsureSnsTopic(string alertingGroupName, bool dryRun)
{
var topicName = alertingGroupName + "-Alerts";
var topic = await _snsClient.FindTopicAsync(topicName);

if (topic != null)
{
_logger.Detail($"Found SNS topic {topicName} with ARN {topic.TopicArn}");
return topic.TopicArn;
}

if (dryRun)
{
_logger.Info($"Skipped: Created SNS topic {topicName}");
return topicName;
}

// https://docs.aws.amazon.com/sns/latest/api/API_CreateTopic.html
// "This action is idempotent, so if the requester already owns a topic with the specified name,
// that topic's ARN is returned without creating a new topic."
var createResponse = await _snsClient.CreateTopicAsync(topicName);
_logger.Info($"Created SNS topic {topicName} with ARN {createResponse.TopicArn}");
return createResponse.TopicArn;
Expand Down
43 changes: 43 additions & 0 deletions Watchman.Tests/CloudFormationDeploymentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -275,5 +275,48 @@ await context.Get<AlarmLoaderAndGenerator>()
Assert.That(count, Is.Not.LessThan(approxExpectedPerStack * 0.8));
}
}

[Test]
public async Task StacksAreNotListedMultipleTimes()
{
var config = ConfigHelper.CreateBasicConfiguration("test", "group-suffix",
new AlertingGroupServices()
{
AutoScaling = new AwsServiceAlarms<AutoScalingResourceConfig>()
{
Resources = new List<ResourceThresholds<AutoScalingResourceConfig>>()
{
new ResourceThresholds<AutoScalingResourceConfig>()
{
Pattern = ".*"
}
}
}
},
numberOfCloudFormationStacks: 5);

var cloudFormation = new FakeCloudFormation();

var context = new TestingIocBootstrapper()
.WithCloudFormation(cloudFormation.Instance)
.WithConfig(config);

context.GetMock<IAmazonAutoScaling>().HasAutoScalingGroups(new[]
{
new AutoScalingGroup()
{
AutoScalingGroupName = $"group-asg",
DesiredCapacity = 40
}
});

await context.Get<AlarmLoaderAndGenerator>()
.LoadAndGenerateAlarms(RunMode.GenerateAlarms);

var stacks = cloudFormation.Stacks();

Assert.That(stacks.Count, Is.GreaterThan(1));
Assert.That(cloudFormation.CallsToListStacks, Is.EqualTo(1));
}
}
}
2 changes: 2 additions & 0 deletions Watchman.Tests/Fakes/FakeCloudFormation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class FakeCloudFormation

private Mock<IAmazonCloudFormation> fake = new Mock<IAmazonCloudFormation>();

public int CallsToListStacks { get; private set; } = 0;
public IAmazonCloudFormation Instance => fake.Object;

public FakeCloudFormation()
Expand Down Expand Up @@ -45,6 +46,7 @@ public FakeCloudFormation()
.ReturnsAsync(new UpdateStackResponse());

fake.Setup(x => x.ListStacksAsync(It.IsAny<ListStacksRequest>(), It.IsAny<CancellationToken>()))
.Callback(() => { CallsToListStacks++;})
.ReturnsAsync(() => new ListStacksResponse()
{
StackSummaries = _submitted
Expand Down
10 changes: 8 additions & 2 deletions Watchman/IoC/ApplicationRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ public ApplicationRegistry(StartupParameters parameters)

private void ConfigureResourceSources()
{
For<IResourceSource<TableDescription>>().Use<TableDescriptionSource>();
For<IResourceSource<QueueData>>().Use<QueueSource>();
// These cache table names etc
For<IResourceSource<TableDescription>>()
.Use<TableDescriptionSource>()
.Singleton();

For<IResourceSource<QueueData>>()
.Use<QueueSource>()
.Singleton();
}

private void ConfigureInternalDependencies(StartupParameters parameters)
Expand Down

0 comments on commit 82e3a9d

Please sign in to comment.