Skip to content

Commit

Permalink
[Stats]Enable Alternate storage for downloads.v1.json (#8380)
Browse files Browse the repository at this point in the history
* Add FeatureFlag for secondary stats.

* Updating Naming.

* Add new Configs. Update downloadcountservice suport alternate endpoint.

* Update DependencyInjection to support alternate configuration.

* Fix tests for build

* Add setting for alternate stats storage

* Stub new config to account deleter.

* read out warning message (#8382)

* Add icons to project to ensure bundling (#8385)

* Add FeatureFlag for secondary stats.

* Updating Naming.

* Add new Configs. Update downloadcountservice suport alternate endpoint.

* Update DependencyInjection to support alternate configuration.

* Fix tests for build

* Add setting for alternate stats storage

* Stub new config to account deleter.

Co-authored-by: lyndaidaii <64443925+lyndaidaii@users.noreply.github.com>
Co-authored-by: Drew Gillies <drewgil@microsoft.com>
  • Loading branch information
3 people authored Jan 23, 2021
1 parent 35ca7d7 commit 73b9012
Show file tree
Hide file tree
Showing 12 changed files with 148 additions and 37 deletions.
1 change: 1 addition & 0 deletions src/AccountDeleter/Configuration/GalleryConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public string SiteRoot
public string AzureStorage_Packages_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
public string AzureStorage_FlatContainer_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
public string AzureStorage_Statistics_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
public string AzureStorage_Statistics_ConnectionString_Alternate { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
public string AzureStorage_Uploads_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
public string AzureStorage_Revalidation_ConnectionString { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
public bool AzureStorageReadAccessGeoRedundant { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
Expand Down
3 changes: 3 additions & 0 deletions src/NuGetGallery.Services/Configuration/AppConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ public class AppConfiguration : IAppConfiguration
[DisplayName("AzureStorage.Statistics.ConnectionString")]
public string AzureStorage_Statistics_ConnectionString { get; set; }

[DisplayName("AzureStorage.Statistics.ConnectionString.Alternate")]
public string AzureStorage_Statistics_ConnectionString_Alternate { get; set; }

[DisplayName("AzureStorage.Uploads.ConnectionString")]
public string AzureStorage_Uploads_ConnectionString { get; set; }

Expand Down
6 changes: 6 additions & 0 deletions src/NuGetGallery.Services/Configuration/FeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class FeatureFlagService : IFeatureFlagService
private const string GalleryPrefix = "NuGetGallery.";

private const string ABTestingFlightName = GalleryPrefix + "ABTesting";
private const string AlternateStatisticsSourceFeatureName = GalleryPrefix + "AlternateStatisticsSource";
private const string AsyncAccountDeleteFeatureName = GalleryPrefix + "AsyncAccountDelete";
private const string SelfServiceAccountDeleteFeatureName = GalleryPrefix + "SelfServiceAccountDelete";
private const string EmbeddedIconFlightName = GalleryPrefix + "EmbeddedIcons";
Expand Down Expand Up @@ -68,6 +69,11 @@ public FeatureFlagService(IFeatureFlagClient client)
_client = client ?? throw new ArgumentNullException(nameof(client));
}

public bool IsAlternateStatisticsSourceEnabled()
{
return _client.IsEnabled(AlternateStatisticsSourceFeatureName, defaultValue: false);
}

public bool IsAsyncAccountDeleteEnabled()
{
return _client.IsEnabled(AsyncAccountDeleteFeatureName, defaultValue: false);
Expand Down
5 changes: 5 additions & 0 deletions src/NuGetGallery.Services/Configuration/IAppConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ public interface IAppConfiguration : IMessageServiceConfiguration
/// </summary>
string AzureStorage_Statistics_ConnectionString { get; set; }

/// <summary>
/// The Azure Storage connection string used for statistics. Secondary
/// </summary>
string AzureStorage_Statistics_ConnectionString_Alternate { get; set; }

/// <summary>
/// The Azure Storage connection string used for package uploads, before publishing.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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.

namespace NuGetGallery.Services
{
public interface IBlobStorageConfiguration
{
string ConnectionString { get; }
bool ReadAccessGeoRedundant { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ namespace NuGetGallery
{
public interface IFeatureFlagService
{
/// <summary>
/// Whether downloads.v1.json hould be pulled from primary or secondary location.
/// If true, the secondary location will be used to download downloads.v1.json.
/// </summary>
bool IsAlternateStatisticsSourceEnabled();

/// <summary>
/// Whether account deletes are performed asychronously or not.
/// If true, account deletes will be attempted to be performed asychronously
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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.

namespace NuGetGallery.Services
{
public class SimpleBlobStorageConfiguration : IBlobStorageConfiguration
{
public string ConnectionString
{
get;
private set;
}

public bool ReadAccessGeoRedundant
{
get;
private set;
}

public SimpleBlobStorageConfiguration(string connectionString, bool readAccessGeoRedundant)
{
ConnectionString = connectionString;
ReadAccessGeoRedundant = readAccessGeoRedundant;
}
}
}
2 changes: 2 additions & 0 deletions src/NuGetGallery.Services/NuGetGallery.Services.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
<Compile Include="Configuration\FeatureFlagService.cs" />
<Compile Include="Configuration\IABTestConfiguration.cs" />
<Compile Include="Configuration\IAppConfiguration.cs" />
<Compile Include="Configuration\IBlobStorageConfiguration.cs" />
<Compile Include="Configuration\ICacheConfiguration.cs" />
<Compile Include="Configuration\ICertificatesConfiguration.cs" />
<Compile Include="Configuration\IFeatureFlagService.cs" />
Expand All @@ -127,6 +128,7 @@
<Compile Include="Configuration\SecretReader\EmptySecretReaderFactory.cs" />
<Compile Include="Configuration\SecretReader\SecretReaderFactory.cs" />
<Compile Include="Configuration\ServiceBusConfiguration.cs" />
<Compile Include="Configuration\SimpleBlobStorageConfiguration.cs" />
<Compile Include="Configuration\SymbolsConfiguration.cs" />
<Compile Include="Configuration\TyposquattingConfiguration.cs" />
<Compile Include="Diagnostics\DiagnosticsService.cs" />
Expand Down
77 changes: 49 additions & 28 deletions src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ public static class BindingKeys
public const string AsyncDeleteAccountName = "AsyncDeleteAccountService";
public const string SyncDeleteAccountName = "SyncDeleteAccountService";

public const string PrimaryStatisticsKey = "PrimaryStatisticsKey";
public const string AlternateStatisticsKey = "AlternateStatisticsKey";

public const string AccountDeleterTopic = "AccountDeleterBindingKey";
public const string PackageValidationTopic = "PackageValidationBindingKey";
public const string SymbolsPackageValidationTopic = "SymbolsPackageValidationBindingKey";
Expand Down Expand Up @@ -698,6 +701,51 @@ private static void RegisterDeleteAccountService(ContainerBuilder builder, Confi
}
}

private static void RegisterStatisticsServices(ContainerBuilder builder, IGalleryConfigurationService configuration, ITelemetryService telemetryService)
{
// when running on Windows Azure, we use a back-end job to calculate stats totals and store in the blobs
builder.RegisterInstance(new JsonAggregateStatsService(configuration.Current.AzureStorage_Statistics_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
.AsSelf()
.As<IAggregateStatsService>()
.SingleInstance();

// when running on Windows Azure, pull the statistics from the warehouse via storage
builder.RegisterInstance(new CloudReportService(configuration.Current.AzureStorage_Statistics_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
.AsSelf()
.As<IReportService>()
.SingleInstance();

// when running on Windows Azure, download counts come from the downloads.v1.json blob
builder.Register(c => new SimpleBlobStorageConfiguration(configuration.Current.AzureStorage_Statistics_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
.SingleInstance()
.Keyed<IBlobStorageConfiguration>(BindingKeys.PrimaryStatisticsKey);

builder.Register(c => new SimpleBlobStorageConfiguration(configuration.Current.AzureStorage_Statistics_ConnectionString_Alternate, configuration.Current.AzureStorageReadAccessGeoRedundant))
.SingleInstance()
.Keyed<IBlobStorageConfiguration>(BindingKeys.AlternateStatisticsKey);

builder.Register(c =>
{
var primaryConfiguration = c.ResolveKeyed<IBlobStorageConfiguration>(BindingKeys.PrimaryStatisticsKey);
var alternateConfiguration = c.ResolveKeyed<IBlobStorageConfiguration>(BindingKeys.AlternateStatisticsKey);
var featureFlagService = c.Resolve<IFeatureFlagService>();
var downloadCountService = new CloudDownloadCountService(telemetryService, featureFlagService, primaryConfiguration, alternateConfiguration);
var dlCountInterceptor = new DownloadCountObjectMaterializedInterceptor(downloadCountService, telemetryService);
ObjectMaterializedInterception.AddInterceptor(dlCountInterceptor);
return downloadCountService;
})
.AsSelf()
.As<IDownloadCountService>()
.SingleInstance();

builder.RegisterType<JsonStatisticsService>()
.AsSelf()
.As<IStatisticsService>()
.SingleInstance();
}

private static void RegisterSwitchingDeleteAccountService(ContainerBuilder builder, ConfigurationService configuration)
{
var asyncAccountDeleteConnectionString = configuration.ServiceBus.AccountDeleter_ConnectionString;
Expand Down Expand Up @@ -1368,34 +1416,7 @@ private static void ConfigureForAzureStorage(ContainerBuilder builder, IGalleryC
}
}

// when running on Windows Azure, we use a back-end job to calculate stats totals and store in the blobs
builder.RegisterInstance(new JsonAggregateStatsService(configuration.Current.AzureStorage_Statistics_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
.AsSelf()
.As<IAggregateStatsService>()
.SingleInstance();

// when running on Windows Azure, pull the statistics from the warehouse via storage
builder.RegisterInstance(new CloudReportService(configuration.Current.AzureStorage_Statistics_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
.AsSelf()
.As<IReportService>()
.SingleInstance();

// when running on Windows Azure, download counts come from the downloads.v1.json blob
var downloadCountService = new CloudDownloadCountService(
telemetryService,
configuration.Current.AzureStorage_Statistics_ConnectionString,
configuration.Current.AzureStorageReadAccessGeoRedundant);

builder.RegisterInstance(downloadCountService)
.AsSelf()
.As<IDownloadCountService>()
.SingleInstance();
ObjectMaterializedInterception.AddInterceptor(new DownloadCountObjectMaterializedInterceptor(downloadCountService, telemetryService));

builder.RegisterType<JsonStatisticsService>()
.AsSelf()
.As<IStatisticsService>()
.SingleInstance();
RegisterStatisticsServices(builder, configuration, telemetryService);

builder.RegisterInstance(new TableErrorLog(configuration.Current.AzureStorage_Errors_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant))
.As<ErrorLog>()
Expand Down
31 changes: 23 additions & 8 deletions src/NuGetGallery/Services/CloudDownloadCountService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.WindowsAzure.Storage.RetryPolicies;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using NuGetGallery.Services;

namespace NuGetGallery
{
Expand All @@ -25,21 +26,26 @@ public class CloudDownloadCountService : IDownloadCountService
private const string TelemetryOriginForRefreshMethod = "CloudDownloadCountService.Refresh";

private readonly ITelemetryService _telemetryService;
private readonly string _connectionString;
private readonly bool _readAccessGeoRedundant;
private readonly IFeatureFlagService _featureFlagService;
private readonly IBlobStorageConfiguration _primaryStorageConfiguration;
private readonly IBlobStorageConfiguration _alternateBlobStorageConfiguration;

private readonly object _refreshLock = new object();
private bool _isRefreshing;

private readonly ConcurrentDictionary<string, ConcurrentDictionary<string, int>> _downloadCounts
= new ConcurrentDictionary<string, ConcurrentDictionary<string, int>>(StringComparer.OrdinalIgnoreCase);

public CloudDownloadCountService(ITelemetryService telemetryService, string connectionString, bool readAccessGeoRedundant)
public CloudDownloadCountService(
ITelemetryService telemetryService,
IFeatureFlagService featureFlagService,
IBlobStorageConfiguration primaryBlobStorageConfiguration,
IBlobStorageConfiguration alternateBlobStorageConfiguration)
{
_telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));

_connectionString = connectionString;
_readAccessGeoRedundant = readAccessGeoRedundant;
_featureFlagService = featureFlagService ?? throw new ArgumentNullException(nameof(featureFlagService));
_primaryStorageConfiguration = primaryBlobStorageConfiguration ?? throw new ArgumentNullException(nameof(primaryBlobStorageConfiguration));
_alternateBlobStorageConfiguration = alternateBlobStorageConfiguration;
}

public bool TryGetDownloadCountForPackageRegistration(string id, out int downloadCount)
Expand Down Expand Up @@ -248,10 +254,19 @@ private void RefreshCore()

private CloudBlockBlob GetBlobReference()
{
var storageAccount = CloudStorageAccount.Parse(_connectionString);
string connectionString = _primaryStorageConfiguration.ConnectionString;
bool readAccessGeoRedundant = _primaryStorageConfiguration.ReadAccessGeoRedundant;

if (_alternateBlobStorageConfiguration != null && _featureFlagService.IsAlternateStatisticsSourceEnabled())
{
connectionString = _alternateBlobStorageConfiguration.ConnectionString;
readAccessGeoRedundant = _alternateBlobStorageConfiguration.ReadAccessGeoRedundant;
}

var storageAccount = CloudStorageAccount.Parse(connectionString);
var blobClient = storageAccount.CreateCloudBlobClient();

if (_readAccessGeoRedundant)
if (readAccessGeoRedundant)
{
blobClient.DefaultRequestOptions.LocationMode = LocationMode.PrimaryThenSecondary;
}
Expand Down
1 change: 1 addition & 0 deletions src/NuGetGallery/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
<add key="Gallery.AzureStorage.Errors.ConnectionString" value=""/>
<add key="Gallery.AzureStorage.Packages.ConnectionString" value=""/>
<add key="Gallery.AzureStorage.Statistics.ConnectionString" value=""/>
<add key="Gallery.AzureStorage.Statistics.ConnectionString.Alternate" value=""/>
<add key="Gallery.AzureStorage.Uploads.ConnectionString" value=""/>
<add key="Gallery.AzureStorage.Revalidation.ConnectionString" value=""/>
<!-- The various Azure Storage connection strings to use. If the Gallery.StorageType is AzureStorage, all must be defined. -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading;
using System.Threading.Tasks;
using Moq;
using NuGetGallery.Services;
using Xunit;

namespace NuGetGallery
Expand Down Expand Up @@ -144,13 +145,26 @@ public void ReturnsCountWhenVersionExists()
public class BaseFacts
{
internal readonly Mock<ITelemetryService> _telemetryService;
internal readonly Mock<IFeatureFlagService> _featureFlagService;
internal readonly Mock<IBlobStorageConfiguration> _primaryBlobStorageConfig;
internal readonly Mock<IBlobStorageConfiguration> _alternateBlobStorageConfig;
internal string _content;
internal Func<IDictionary<string, int>, int> _calculateSum;
internal TestableCloudDownloadCountService _target;

public BaseFacts()
{
_telemetryService = new Mock<ITelemetryService>();
_featureFlagService = new Mock<IFeatureFlagService>();
_primaryBlobStorageConfig = new Mock<IBlobStorageConfiguration>();
_alternateBlobStorageConfig = new Mock<IBlobStorageConfiguration>();

_primaryBlobStorageConfig.Setup(ps => ps.ConnectionString).Returns("primary");
_primaryBlobStorageConfig.Setup(ps => ps.ReadAccessGeoRedundant).Returns(true);

_alternateBlobStorageConfig.Setup(ps => ps.ConnectionString).Returns("secondary");
_alternateBlobStorageConfig.Setup(ps => ps.ReadAccessGeoRedundant).Returns(true);

_content = "[[\"NuGet.Versioning\",[\"4.6.0\",23],[\"4.6.2\",42]]";
_calculateSum = null;
_target = new TestableCloudDownloadCountService(this);
Expand All @@ -162,7 +176,7 @@ public class TestableCloudDownloadCountService : CloudDownloadCountService
private readonly BaseFacts _baseFacts;

public TestableCloudDownloadCountService(BaseFacts baseFacts)
: base(baseFacts._telemetryService.Object, "UseDevelopmentStorage=true", readAccessGeoRedundant: true)
: base(baseFacts._telemetryService.Object, baseFacts._featureFlagService.Object, baseFacts._primaryBlobStorageConfig.Object, baseFacts._alternateBlobStorageConfig.Object)
{
_baseFacts = baseFacts;
}
Expand Down

0 comments on commit 73b9012

Please sign in to comment.