From 2268b1e187d941638e6723b9e69ed03a46b2929f Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 14 Dec 2021 16:09:31 +0100 Subject: [PATCH] Package telemetry (#11738) * Add TelemetryService Currently it only allows you to get a list of the name of all the installed packages * Create model class for package telemetry And move the telemetry report data out of ReportSiteTask * Add version as an option in package manifest * Use TelemetryService to generate telemetry data Instead of doing it directly in the ReportSiteTask * Seal TelemetryService This should not be overwritten * Add option for package creators to opt out * Add global setting to restrict package telemetry * Add TelemetryService unit tests * Add ManifestParser tests for new properties * Clean * Update src/Umbraco.Core/Telemetry/TelemetryService.cs Co-authored-by: Bjarke Berg * Create interface for telemetry service * Use IOptionsMonitor instead of IOptions However I chose to use CurrentValue since according to microsoft: "Some file systems, such as Docker containers and network shares, may not reliably send change notifications.", additionally TelemetryService only runs once pr. day, so it shouldn't be too much of an issue that it doesn't cache the result. * Use is false instead of negation It's a bit more readable * Track restrict package telemetry value * Save RestrictPackageTelemetry in report data Not packages, since it'll be the same for all packages * Fix TelemetryService unit tests * Clean * Update src/Umbraco.Core/Telemetry/ITelemetryService.cs Co-authored-by: Bjarke Berg * Remove RestrictPackageTelemetry Co-authored-by: Bjarke Berg --- .../Configuration/Models/GlobalSettings.cs | 1 + .../DependencyInjection/UmbracoBuilder.cs | 4 + src/Umbraco.Core/Manifest/PackageManifest.cs | 13 ++ .../Telemetry/ITelemetryService.cs | 15 ++ .../Telemetry/Models/PackageTelemetry.cs | 26 ++++ .../Telemetry/Models/TelemetryReportData.cs | 34 +++++ .../Telemetry/TelemetryService.cs | 86 +++++++++++ .../HostedServices/ReportSiteTask.cs | 42 +----- .../Manifest/ManifestParserTests.cs | 22 +++ .../Telemetry/TelemetryServiceTests.cs | 135 ++++++++++++++++++ 10 files changed, 343 insertions(+), 35 deletions(-) create mode 100644 src/Umbraco.Core/Telemetry/ITelemetryService.cs create mode 100644 src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs create mode 100644 src/Umbraco.Core/Telemetry/Models/TelemetryReportData.cs create mode 100644 src/Umbraco.Core/Telemetry/TelemetryService.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/TelemetryServiceTests.cs diff --git a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs index 94b8d88ed3..97fb91b0ec 100644 --- a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs @@ -130,6 +130,7 @@ namespace Umbraco.Cms.Core.Configuration.Models /// Gets or sets a value for the main dom lock. /// public string MainDomLock { get; set; } = string.Empty; + public string Id { get; set; } = string.Empty; /// diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 36d7918531..1af25e16e8 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -36,6 +36,7 @@ using Umbraco.Cms.Core.Runtime; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Sync; +using Umbraco.Cms.Core.Telemetry; using Umbraco.Cms.Core.Templates; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Web.Common.DependencyInjection; @@ -259,6 +260,9 @@ namespace Umbraco.Cms.Core.DependencyInjection // Register ValueEditorCache used for validation Services.AddSingleton(); + + // Register telemetry service used to gather data about installed packages + Services.AddUnique(); } } } diff --git a/src/Umbraco.Core/Manifest/PackageManifest.cs b/src/Umbraco.Core/Manifest/PackageManifest.cs index 753ec0613a..ae6ffdc8ba 100644 --- a/src/Umbraco.Core/Manifest/PackageManifest.cs +++ b/src/Umbraco.Core/Manifest/PackageManifest.cs @@ -48,6 +48,19 @@ namespace Umbraco.Cms.Core.Manifest /// [IgnoreDataMember] public string Source { get; set; } + + /// + /// Gets or sets the version of the package + /// + [DataMember(Name = "version")] + public string Version { get; set; } = string.Empty; + + /// + /// Gets or sets a value indicating whether telemetry is allowed + /// + [DataMember(Name = "allowPackageTelemetry")] + public bool AllowPackageTelemetry { get; set; } = true; + [DataMember(Name = "bundleOptions")] public BundleOptions BundleOptions { get; set; } diff --git a/src/Umbraco.Core/Telemetry/ITelemetryService.cs b/src/Umbraco.Core/Telemetry/ITelemetryService.cs new file mode 100644 index 0000000000..60070481f3 --- /dev/null +++ b/src/Umbraco.Core/Telemetry/ITelemetryService.cs @@ -0,0 +1,15 @@ +using Umbraco.Cms.Core.Telemetry.Models; + +namespace Umbraco.Cms.Core.Telemetry +{ + /// + /// Service which gathers the data for telemetry reporting + /// + public interface ITelemetryService + { + /// + /// Try and get the + /// + bool TryGetTelemetryReportData(out TelemetryReportData telemetryReportData); + } +} diff --git a/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs b/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs new file mode 100644 index 0000000000..8b7aa4bc0c --- /dev/null +++ b/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs @@ -0,0 +1,26 @@ +using System.Runtime.Serialization; + +namespace Umbraco.Cms.Core.Telemetry.Models +{ + /// + /// Serializable class containing information about an installed package. + /// + [DataContract(Name = "packageTelemetry")] + public class PackageTelemetry + { + /// + /// Gets or sets the name of the installed package. + /// + [DataMember(Name = "name")] + public string Name { get; set; } + + /// + /// Gets or sets the version of the installed package. + /// + /// + /// This may be an empty string if no version is specified, or if package telemetry has been restricted. + /// + [DataMember(Name = "version")] + public string Version { get; set; } + } +} diff --git a/src/Umbraco.Core/Telemetry/Models/TelemetryReportData.cs b/src/Umbraco.Core/Telemetry/Models/TelemetryReportData.cs new file mode 100644 index 0000000000..d19e24695b --- /dev/null +++ b/src/Umbraco.Core/Telemetry/Models/TelemetryReportData.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.Runtime.Serialization; + +namespace Umbraco.Cms.Core.Telemetry.Models +{ + /// + /// Serializable class containing telemetry information. + /// + [DataContract] + public class TelemetryReportData + { + /// + /// Gets or sets a random GUID to prevent an instance posting multiple times pr. day. + /// + [DataMember(Name = "id")] + public Guid Id { get; set; } + + /// + /// Gets or sets the Umbraco CMS version. + /// + [DataMember(Name = "version")] + public string Version { get; set; } + + /// + /// Gets or sets an enumerable containing information about packages. + /// + /// + /// Contains only the name and version of the packages, unless no version is specified. + /// + [DataMember(Name = "packages")] + public IEnumerable Packages { get; set; } + } +} diff --git a/src/Umbraco.Core/Telemetry/TelemetryService.cs b/src/Umbraco.Core/Telemetry/TelemetryService.cs new file mode 100644 index 0000000000..63e4e1ff49 --- /dev/null +++ b/src/Umbraco.Core/Telemetry/TelemetryService.cs @@ -0,0 +1,86 @@ +using System; +using System.Collections.Generic; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Manifest; +using Umbraco.Cms.Core.Telemetry.Models; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Core.Telemetry +{ + /// + internal class TelemetryService : ITelemetryService + { + private readonly IOptionsMonitor _globalSettings; + private readonly IManifestParser _manifestParser; + private readonly IUmbracoVersion _umbracoVersion; + + /// + /// Initializes a new instance of the class. + /// + public TelemetryService( + IOptionsMonitor globalSettings, + IManifestParser manifestParser, + IUmbracoVersion umbracoVersion) + { + _manifestParser = manifestParser; + _umbracoVersion = umbracoVersion; + _globalSettings = globalSettings; + } + + /// + public bool TryGetTelemetryReportData(out TelemetryReportData telemetryReportData) + { + if (TryGetTelemetryId(out Guid telemetryId) is false) + { + telemetryReportData = null; + return false; + } + + telemetryReportData = new TelemetryReportData + { + Id = telemetryId, + Version = _umbracoVersion.SemanticVersion.ToSemanticStringWithoutBuild(), + Packages = GetPackageTelemetry() + }; + return true; + } + + private bool TryGetTelemetryId(out Guid telemetryId) + { + // Parse telemetry string as a GUID & verify its a GUID and not some random string + // since users may have messed with or decided to empty the app setting or put in something random + if (Guid.TryParse(_globalSettings.CurrentValue.Id, out var parsedTelemetryId) is false) + { + telemetryId = Guid.Empty; + return false; + } + + telemetryId = parsedTelemetryId; + return true; + } + + private IEnumerable GetPackageTelemetry() + { + List packages = new (); + IEnumerable manifests = _manifestParser.GetManifests(); + + foreach (PackageManifest manifest in manifests) + { + if (manifest.AllowPackageTelemetry is false) + { + continue; + } + + packages.Add(new PackageTelemetry + { + Name = manifest.PackageName, + Version = manifest.Version ?? string.Empty + }); + } + + return packages; + } + } +} diff --git a/src/Umbraco.Infrastructure/HostedServices/ReportSiteTask.cs b/src/Umbraco.Infrastructure/HostedServices/ReportSiteTask.cs index 6eab3a60bc..7f88d063d8 100644 --- a/src/Umbraco.Infrastructure/HostedServices/ReportSiteTask.cs +++ b/src/Umbraco.Infrastructure/HostedServices/ReportSiteTask.cs @@ -1,33 +1,27 @@ using System; using System.Net.Http; -using System.Runtime.Serialization; using System.Text; using System.Threading.Tasks; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using Newtonsoft.Json; -using Umbraco.Cms.Core.Configuration; -using Umbraco.Cms.Core.Configuration.Models; -using Umbraco.Extensions; +using Umbraco.Cms.Core.Telemetry; +using Umbraco.Cms.Core.Telemetry.Models; namespace Umbraco.Cms.Infrastructure.HostedServices { public class ReportSiteTask : RecurringHostedServiceBase { private readonly ILogger _logger; - private readonly IUmbracoVersion _umbracoVersion; - private readonly IOptions _globalSettings; + private readonly ITelemetryService _telemetryService; private static HttpClient s_httpClient; public ReportSiteTask( ILogger logger, - IUmbracoVersion umbracoVersion, - IOptions globalSettings) + ITelemetryService telemetryService) : base(TimeSpan.FromDays(1), TimeSpan.FromMinutes(1)) { _logger = logger; - _umbracoVersion = umbracoVersion; - _globalSettings = globalSettings; + _telemetryService = telemetryService; s_httpClient = new HttpClient(); } @@ -37,14 +31,8 @@ namespace Umbraco.Cms.Infrastructure.HostedServices /// public override async Task PerformExecuteAsync(object state) { - // Try & get a value stored in umbracoSettings.config on the backoffice XML element ID attribute - var backofficeIdentifierRaw = _globalSettings.Value.Id; - - // Parse as a GUID & verify its a GUID and not some random string - // In case of users may have messed or decided to empty the file contents or put in something random - if (Guid.TryParse(backofficeIdentifierRaw, out var telemetrySiteIdentifier) == false) + if (_telemetryService.TryGetTelemetryReportData(out TelemetryReportData telemetryReportData) is false) { - // Some users may have decided to mess with the XML attribute and put in something else _logger.LogWarning("No telemetry marker found"); return; @@ -52,7 +40,6 @@ namespace Umbraco.Cms.Infrastructure.HostedServices try { - if (s_httpClient.BaseAddress is null) { // Send data to LIVE telemetry @@ -64,9 +51,6 @@ namespace Umbraco.Cms.Infrastructure.HostedServices #if DEBUG // Send data to DEBUG telemetry service s_httpClient.BaseAddress = new Uri("https://telemetry.rainbowsrock.net/"); - - - #endif } @@ -75,8 +59,7 @@ namespace Umbraco.Cms.Infrastructure.HostedServices using (var request = new HttpRequestMessage(HttpMethod.Post, "installs/")) { - var postData = new TelemetryReportData { Id = telemetrySiteIdentifier, Version = _umbracoVersion.SemanticVersion.ToSemanticStringWithoutBuild() }; - request.Content = new StringContent(JsonConvert.SerializeObject(postData), Encoding.UTF8, "application/json"); //CONTENT-TYPE header + request.Content = new StringContent(JsonConvert.SerializeObject(telemetryReportData), Encoding.UTF8, "application/json"); //CONTENT-TYPE header // Make a HTTP Post to telemetry service // https://telemetry.umbraco.com/installs/ @@ -94,16 +77,5 @@ namespace Umbraco.Cms.Infrastructure.HostedServices _logger.LogDebug("There was a problem sending a request to the Umbraco telemetry service"); } } - [DataContract] - private class TelemetryReportData - { - [DataMember(Name = "id")] - public Guid Id { get; set; } - - [DataMember(Name = "version")] - public string Version { get; set; } - } - - } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/ManifestParserTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/ManifestParserTests.cs index c93e5087b7..fda216e51a 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/ManifestParserTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/ManifestParserTests.cs @@ -484,5 +484,27 @@ javascript: ['~/test.js',/*** some note about stuff asd09823-4**09234*/ '~/test2 Assert.AreEqual("Content", manifest.Sections[0].Name); Assert.AreEqual("World", manifest.Sections[1].Name); } + + [Test] + public void CanParseManifest_Version() + { + const string json = @"{""name"": ""VersionPackage"", ""version"": ""1.0.0""}"; + PackageManifest manifest = _parser.ParseManifest(json); + + Assert.Multiple(() => + { + Assert.AreEqual("VersionPackage", manifest.PackageName); + Assert.AreEqual("1.0.0", manifest.Version); + }); + } + + [Test] + public void CanParseManifest_TrackingAllowed() + { + const string json = @"{""allowPackageTelemetry"": false }"; + PackageManifest manifest = _parser.ParseManifest(json); + + Assert.IsFalse(manifest.AllowPackageTelemetry); + } } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/TelemetryServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/TelemetryServiceTests.cs new file mode 100644 index 0000000000..1c92569695 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/TelemetryServiceTests.cs @@ -0,0 +1,135 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.Options; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Manifest; +using Umbraco.Cms.Core.Semver; +using Umbraco.Cms.Core.Telemetry; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Telemetry +{ + [TestFixture] + public class TelemetryServiceTests + { + [TestCase("0F1785C5-7BA0-4C52-AB62-863BD2C8F3FE", true)] + [TestCase("This is not a guid", false)] + [TestCase("", false)] + public void OnlyParsesIfValidId(string guidString, bool shouldSucceed) + { + var globalSettings = CreateGlobalSettings(guidString); + var version = CreateUmbracoVersion(9, 1, 1); + var sut = new TelemetryService(globalSettings, Mock.Of(), version); + + var result = sut.TryGetTelemetryReportData(out var telemetry); + + Assert.AreEqual(shouldSucceed, result); + if (shouldSucceed) + { + // When toString is called on a GUID it will to lower, so do the same to our guidString + Assert.AreEqual(guidString.ToLower(), telemetry.Id.ToString()); + } + else + { + Assert.IsNull(telemetry); + } + } + + [Test] + public void ReturnsSemanticVersionWithoutBuild() + { + var globalSettings = CreateGlobalSettings(); + var version = CreateUmbracoVersion(9, 1, 1, "-rc", "-ad2f4k2d"); + + var sut = new TelemetryService(globalSettings, Mock.Of(), version); + + var result = sut.TryGetTelemetryReportData(out var telemetry); + + Assert.IsTrue(result); + Assert.AreEqual("9.1.1-rc", telemetry.Version); + } + + [Test] + public void CanGatherPackageTelemetry() + { + var globalSettings = CreateGlobalSettings(); + var version = CreateUmbracoVersion(9, 1, 1); + var versionPackageName = "VersionPackage"; + var packageVersion = "1.0.0"; + var noVersionPackageName = "NoVersionPackage"; + PackageManifest[] manifests = + { + new () { PackageName = versionPackageName, Version = packageVersion }, + new () { PackageName = noVersionPackageName } + }; + var manifestParser = CreateManifestParser(manifests); + var sut = new TelemetryService(globalSettings, manifestParser, version); + + var success = sut.TryGetTelemetryReportData(out var telemetry); + + Assert.IsTrue(success); + Assert.Multiple(() => + { + Assert.AreEqual(2, telemetry.Packages.Count()); + var versionPackage = telemetry.Packages.FirstOrDefault(x => x.Name == versionPackageName); + Assert.AreEqual(versionPackageName, versionPackage.Name); + Assert.AreEqual(packageVersion, versionPackage.Version); + + var noVersionPackage = telemetry.Packages.FirstOrDefault(x => x.Name == noVersionPackageName); + Assert.AreEqual(noVersionPackageName, noVersionPackage.Name); + Assert.AreEqual(string.Empty, noVersionPackage.Version); + }); + } + + [Test] + public void RespectsAllowPackageTelemetry() + { + var globalSettings = CreateGlobalSettings(); + var version = CreateUmbracoVersion(9, 1, 1); + PackageManifest[] manifests = + { + new () { PackageName = "DoNotTrack", AllowPackageTelemetry = false }, + new () { PackageName = "TrackingAllowed", AllowPackageTelemetry = true } + }; + var manifestParser = CreateManifestParser(manifests); + var sut = new TelemetryService(globalSettings, manifestParser, version); + + var success = sut.TryGetTelemetryReportData(out var telemetry); + + Assert.IsTrue(success); + Assert.Multiple(() => + { + Assert.AreEqual(1, telemetry.Packages.Count()); + Assert.AreEqual("TrackingAllowed", telemetry.Packages.First().Name); + }); + } + + + private IManifestParser CreateManifestParser(IEnumerable manifests) + { + var manifestParserMock = new Mock(); + manifestParserMock.Setup(x => x.GetManifests()).Returns(manifests); + return manifestParserMock.Object; + } + + private IUmbracoVersion CreateUmbracoVersion(int major, int minor, int patch, string prerelease = "", string build = "") + { + var version = new SemVersion(major, minor, patch, prerelease, build); + return Mock.Of(x => x.SemanticVersion == version); + } + + private IOptionsMonitor CreateGlobalSettings(string guidString = null) + { + if (guidString is null) + { + guidString = Guid.NewGuid().ToString(); + } + + var globalSettings = new GlobalSettings { Id = guidString }; + return Mock.Of>(x => x.CurrentValue == globalSettings); + } + } +}