From 73b901242772424c45d183f12cd1a5f6961252bb Mon Sep 17 00:00:00 2001 From: Ryu Yu <11051729+ryuyu@users.noreply.github.com> Date: Fri, 22 Jan 2021 17:38:26 -0800 Subject: [PATCH] [Stats]Enable Alternate storage for downloads.v1.json (#8380) * 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 --- .../Configuration/GalleryConfiguration.cs | 1 + .../Configuration/AppConfiguration.cs | 3 + .../Configuration/FeatureFlagService.cs | 6 ++ .../Configuration/IAppConfiguration.cs | 5 ++ .../IBlobStorageConfiguration.cs | 11 +++ .../Configuration/IFeatureFlagService.cs | 6 ++ .../SimpleBlobStorageConfiguration.cs | 26 +++++++ .../NuGetGallery.Services.csproj | 2 + .../App_Start/DefaultDependenciesModule.cs | 77 ++++++++++++------- .../Services/CloudDownloadCountService.cs | 31 ++++++-- src/NuGetGallery/Web.config | 1 + .../CloudDownloadCountServiceFacts.cs | 16 +++- 12 files changed, 148 insertions(+), 37 deletions(-) create mode 100644 src/NuGetGallery.Services/Configuration/IBlobStorageConfiguration.cs create mode 100644 src/NuGetGallery.Services/Configuration/SimpleBlobStorageConfiguration.cs diff --git a/src/AccountDeleter/Configuration/GalleryConfiguration.cs b/src/AccountDeleter/Configuration/GalleryConfiguration.cs index 7ea3782dc3..ec2c80acc4 100644 --- a/src/AccountDeleter/Configuration/GalleryConfiguration.cs +++ b/src/AccountDeleter/Configuration/GalleryConfiguration.cs @@ -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(); } diff --git a/src/NuGetGallery.Services/Configuration/AppConfiguration.cs b/src/NuGetGallery.Services/Configuration/AppConfiguration.cs index 9539a9ff51..530ad03689 100644 --- a/src/NuGetGallery.Services/Configuration/AppConfiguration.cs +++ b/src/NuGetGallery.Services/Configuration/AppConfiguration.cs @@ -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; } diff --git a/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs b/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs index 62c7fa87ef..59852a422d 100644 --- a/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs +++ b/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs @@ -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"; @@ -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); diff --git a/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs b/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs index 82a2c6d7c6..fe0d162aad 100644 --- a/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs +++ b/src/NuGetGallery.Services/Configuration/IAppConfiguration.cs @@ -75,6 +75,11 @@ public interface IAppConfiguration : IMessageServiceConfiguration /// string AzureStorage_Statistics_ConnectionString { get; set; } + /// + /// The Azure Storage connection string used for statistics. Secondary + /// + string AzureStorage_Statistics_ConnectionString_Alternate { get; set; } + /// /// The Azure Storage connection string used for package uploads, before publishing. /// diff --git a/src/NuGetGallery.Services/Configuration/IBlobStorageConfiguration.cs b/src/NuGetGallery.Services/Configuration/IBlobStorageConfiguration.cs new file mode 100644 index 0000000000..19b052b7a5 --- /dev/null +++ b/src/NuGetGallery.Services/Configuration/IBlobStorageConfiguration.cs @@ -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; } + } +} diff --git a/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs b/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs index 9fb7b91211..5c50c696b7 100644 --- a/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs +++ b/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs @@ -8,6 +8,12 @@ namespace NuGetGallery { public interface IFeatureFlagService { + /// + /// 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. + /// + bool IsAlternateStatisticsSourceEnabled(); + /// /// Whether account deletes are performed asychronously or not. /// If true, account deletes will be attempted to be performed asychronously diff --git a/src/NuGetGallery.Services/Configuration/SimpleBlobStorageConfiguration.cs b/src/NuGetGallery.Services/Configuration/SimpleBlobStorageConfiguration.cs new file mode 100644 index 0000000000..b692f657c9 --- /dev/null +++ b/src/NuGetGallery.Services/Configuration/SimpleBlobStorageConfiguration.cs @@ -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; + } + } +} diff --git a/src/NuGetGallery.Services/NuGetGallery.Services.csproj b/src/NuGetGallery.Services/NuGetGallery.Services.csproj index 552e82c055..7a67a54580 100644 --- a/src/NuGetGallery.Services/NuGetGallery.Services.csproj +++ b/src/NuGetGallery.Services/NuGetGallery.Services.csproj @@ -109,6 +109,7 @@ + @@ -127,6 +128,7 @@ + diff --git a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs index 8800107abb..dd263e06de 100644 --- a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs +++ b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs @@ -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"; @@ -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() + .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() + .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(BindingKeys.PrimaryStatisticsKey); + + builder.Register(c => new SimpleBlobStorageConfiguration(configuration.Current.AzureStorage_Statistics_ConnectionString_Alternate, configuration.Current.AzureStorageReadAccessGeoRedundant)) + .SingleInstance() + .Keyed(BindingKeys.AlternateStatisticsKey); + + builder.Register(c => + { + var primaryConfiguration = c.ResolveKeyed(BindingKeys.PrimaryStatisticsKey); + var alternateConfiguration = c.ResolveKeyed(BindingKeys.AlternateStatisticsKey); + var featureFlagService = c.Resolve(); + var downloadCountService = new CloudDownloadCountService(telemetryService, featureFlagService, primaryConfiguration, alternateConfiguration); + + var dlCountInterceptor = new DownloadCountObjectMaterializedInterceptor(downloadCountService, telemetryService); + ObjectMaterializedInterception.AddInterceptor(dlCountInterceptor); + + return downloadCountService; + }) + .AsSelf() + .As() + .SingleInstance(); + + builder.RegisterType() + .AsSelf() + .As() + .SingleInstance(); + } + private static void RegisterSwitchingDeleteAccountService(ContainerBuilder builder, ConfigurationService configuration) { var asyncAccountDeleteConnectionString = configuration.ServiceBus.AccountDeleter_ConnectionString; @@ -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() - .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() - .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() - .SingleInstance(); - ObjectMaterializedInterception.AddInterceptor(new DownloadCountObjectMaterializedInterceptor(downloadCountService, telemetryService)); - - builder.RegisterType() - .AsSelf() - .As() - .SingleInstance(); + RegisterStatisticsServices(builder, configuration, telemetryService); builder.RegisterInstance(new TableErrorLog(configuration.Current.AzureStorage_Errors_ConnectionString, configuration.Current.AzureStorageReadAccessGeoRedundant)) .As() diff --git a/src/NuGetGallery/Services/CloudDownloadCountService.cs b/src/NuGetGallery/Services/CloudDownloadCountService.cs index 4898ddb73e..7d2ac696d3 100644 --- a/src/NuGetGallery/Services/CloudDownloadCountService.cs +++ b/src/NuGetGallery/Services/CloudDownloadCountService.cs @@ -12,6 +12,7 @@ using Microsoft.WindowsAzure.Storage.RetryPolicies; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using NuGetGallery.Services; namespace NuGetGallery { @@ -25,8 +26,9 @@ 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; @@ -34,12 +36,16 @@ public class CloudDownloadCountService : IDownloadCountService private readonly ConcurrentDictionary> _downloadCounts = new ConcurrentDictionary>(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) @@ -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; } diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config index 4ee3f450ff..a856eea342 100644 --- a/src/NuGetGallery/Web.config +++ b/src/NuGetGallery/Web.config @@ -37,6 +37,7 @@ + diff --git a/tests/NuGetGallery.Facts/Services/CloudDownloadCountServiceFacts.cs b/tests/NuGetGallery.Facts/Services/CloudDownloadCountServiceFacts.cs index 4c6ee88891..a7568274a2 100644 --- a/tests/NuGetGallery.Facts/Services/CloudDownloadCountServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/CloudDownloadCountServiceFacts.cs @@ -11,6 +11,7 @@ using System.Threading; using System.Threading.Tasks; using Moq; +using NuGetGallery.Services; using Xunit; namespace NuGetGallery @@ -144,6 +145,9 @@ public void ReturnsCountWhenVersionExists() public class BaseFacts { internal readonly Mock _telemetryService; + internal readonly Mock _featureFlagService; + internal readonly Mock _primaryBlobStorageConfig; + internal readonly Mock _alternateBlobStorageConfig; internal string _content; internal Func, int> _calculateSum; internal TestableCloudDownloadCountService _target; @@ -151,6 +155,16 @@ public class BaseFacts public BaseFacts() { _telemetryService = new Mock(); + _featureFlagService = new Mock(); + _primaryBlobStorageConfig = new Mock(); + _alternateBlobStorageConfig = new Mock(); + + _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); @@ -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; }