From c59f171bfcadce40cbcd4367cdb700d4ca79a6a0 Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 22 Apr 2024 12:14:08 +0100 Subject: [PATCH] Add missing IStatsDPublisherWithTags registration - Add missing IoC registration for `IStatsDPublisherWithTags`. - Mention `IStatsDPublisherWithTags` in the README. - Tidy up some redundant checks in the unit tests as the container always implements `IDisposable`.. Resolves #661. --- README.md | 4 +- .../StatsDServiceCollectionExtensions.cs | 5 +- .../WhenRegisteringStatsD.cs | 273 ++++++++---------- 3 files changed, 129 insertions(+), 153 deletions(-) diff --git a/README.md b/README.md index 5626677e..0cf68f8a 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,9 @@ We use this library within our components to publish [StatsD](https://github.com #### Publishing statistics -`IStatsDPublisher` is the interface that you will use to send stats. The concrete class that implements `IStatsDPublisher` is `StatsDPublisher`. The `StatsDPublisher` constructor takes an instance of `StatsDConfiguration`. +`IStatsDPublisher` is the interface that you will use to send stats. There is also a `IStatsDPublisherWithTags` interface that can be used to send stats with tags. + +The concrete class that implements `IStatsDPublisher` is `StatsDPublisher`. The `StatsDPublisher` constructor takes an instance of `StatsDConfiguration`. For the configuration's values, you will always need the StatsD server host name or IP address. Optionally, you can also change the port from the default (`8125`). You can also prepend a prefix to all stats. These values often come from configuration as the host name and/or prefix may vary between test and production environments. diff --git a/src/JustEat.StatsD/StatsDServiceCollectionExtensions.cs b/src/JustEat.StatsD/StatsDServiceCollectionExtensions.cs index 77553d59..5012ec8a 100644 --- a/src/JustEat.StatsD/StatsDServiceCollectionExtensions.cs +++ b/src/JustEat.StatsD/StatsDServiceCollectionExtensions.cs @@ -100,7 +100,10 @@ private static IServiceCollection AddStatsDCore(this IServiceCollection services { services.TryAddSingleton(ResolveEndPointSource); services.TryAddSingleton(ResolveStatsDTransport); + services.TryAddSingleton(ResolveStatsDPublisher); + services.TryAddSingleton((p) => p.GetRequiredService()); + services.TryAddSingleton((p) => p.GetRequiredService()); return services; } @@ -115,7 +118,7 @@ private static IEndPointSource ResolveEndPointSource(IServiceProvider provider) config.DnsLookupInterval); } - private static IStatsDPublisher ResolveStatsDPublisher(IServiceProvider provider) + private static StatsDPublisher ResolveStatsDPublisher(IServiceProvider provider) { var config = provider.GetRequiredService(); var socketProtocol = provider.GetRequiredService(); diff --git a/tests/JustEat.StatsD.Tests/WhenRegisteringStatsD.cs b/tests/JustEat.StatsD.Tests/WhenRegisteringStatsD.cs index 94cefe51..a75af805 100644 --- a/tests/JustEat.StatsD.Tests/WhenRegisteringStatsD.cs +++ b/tests/JustEat.StatsD.Tests/WhenRegisteringStatsD.cs @@ -15,39 +15,35 @@ public static void CanRegisterServicesWithNoConfigurationIfConfigurationAlreadyR Host = "localhost" }; - var provider = Configure(services => - { - services.AddSingleton(config); + using var provider = Configure(services => + { + services.AddSingleton(config); - // Act - services.AddStatsD(); - }); + // Act + services.AddStatsD(); + }); - try - { - // Assert - var configuration = provider.GetRequiredService(); - configuration.ShouldNotBeNull(); - configuration.ShouldBe(config); + // Assert + var configuration = provider.GetRequiredService(); + configuration.ShouldNotBeNull(); + configuration.ShouldBe(config); - var source = provider.GetRequiredService(); - source.ShouldNotBeNull(); + var source = provider.GetRequiredService(); + source.ShouldNotBeNull(); - var transport = provider.GetRequiredService(); - transport.ShouldNotBeNull(); - transport.ShouldBeOfType(); + var transport = provider.GetRequiredService(); + transport.ShouldNotBeNull(); + transport.ShouldBeOfType(); - var publisher = provider.GetRequiredService(); - publisher.ShouldNotBeNull(); - publisher.ShouldBeOfType(); - } - finally - { - if (provider is IDisposable disposable) - { - disposable.Dispose(); - } - } + var publisher = provider.GetRequiredService(); + publisher.ShouldNotBeNull(); + publisher.ShouldBeOfType(); + + var publisherWithTags = provider.GetRequiredService(); + publisherWithTags.ShouldNotBeNull(); + publisherWithTags.ShouldBeOfType(); + + publisherWithTags.ShouldBeSameAs(publisher); } [Fact] @@ -56,38 +52,28 @@ public static void CanRegisterServicesWithAHost() // Arrange string host = "localhost"; - var provider = Configure(services => - { - // Act - services.AddStatsD(host); - }); - - try - { - // Assert - var configuration = provider.GetRequiredService(); - configuration.ShouldNotBeNull(); - configuration.Host.ShouldBe(host); - configuration.Prefix.ShouldBeEmpty(); - - var source = provider.GetRequiredService(); - source.ShouldNotBeNull(); - - var transport = provider.GetRequiredService(); - transport.ShouldNotBeNull(); - transport.ShouldBeOfType(); - - var publisher = provider.GetRequiredService(); - publisher.ShouldNotBeNull(); - publisher.ShouldBeOfType(); - } - finally + using var provider = Configure(services => { - if (provider is IDisposable disposable) - { - disposable.Dispose(); - } - } + // Act + services.AddStatsD(host); + }); + + // Assert + var configuration = provider.GetRequiredService(); + configuration.ShouldNotBeNull(); + configuration.Host.ShouldBe(host); + configuration.Prefix.ShouldBeEmpty(); + + var source = provider.GetRequiredService(); + source.ShouldNotBeNull(); + + var transport = provider.GetRequiredService(); + transport.ShouldNotBeNull(); + transport.ShouldBeOfType(); + + var publisher = provider.GetRequiredService(); + publisher.ShouldNotBeNull(); + publisher.ShouldBeOfType(); } [Fact] @@ -97,38 +83,28 @@ public static void CanRegisterServicesWithAHostAndPrefix() string host = "localhost"; string prefix = "myprefix"; - var provider = Configure(services => - { - // Act - services.AddStatsD(host, prefix); - }); - - try + using var provider = Configure(services => { - // Assert - var configuration = provider.GetRequiredService(); - configuration.ShouldNotBeNull(); - configuration.Host.ShouldBe(host); - configuration.Prefix.ShouldBe(prefix); - - var source = provider.GetRequiredService(); - source.ShouldNotBeNull(); - - var transport = provider.GetRequiredService(); - transport.ShouldNotBeNull(); - transport.ShouldBeOfType(); - - var publisher = provider.GetRequiredService(); - publisher.ShouldNotBeNull(); - publisher.ShouldBeOfType(); - } - finally - { - if (provider is IDisposable disposable) - { - disposable.Dispose(); - } - } + // Act + services.AddStatsD(host, prefix); + }); + + // Assert + var configuration = provider.GetRequiredService(); + configuration.ShouldNotBeNull(); + configuration.Host.ShouldBe(host); + configuration.Prefix.ShouldBe(prefix); + + var source = provider.GetRequiredService(); + source.ShouldNotBeNull(); + + var transport = provider.GetRequiredService(); + transport.ShouldNotBeNull(); + transport.ShouldBeOfType(); + + var publisher = provider.GetRequiredService(); + publisher.ShouldNotBeNull(); + publisher.ShouldBeOfType(); } [Fact] @@ -140,48 +116,38 @@ public static void CanRegisterServicesWithFactoryMethod() StatsDHost = "localhost" }; - var provider = Configure(services => + using var provider = Configure(services => + { + services.AddSingleton(options); + + // Act + services.AddStatsD(serviceProvider => { - services.AddSingleton(options); - - // Act - services.AddStatsD(serviceProvider => - { - var myOptions = serviceProvider.GetRequiredService(); - - return new StatsDConfiguration - { - Host = myOptions.StatsDHost - }; - }); + var myOptions = serviceProvider.GetRequiredService(); + + return new StatsDConfiguration + { + Host = myOptions.StatsDHost + }; }); + }); - try - { - // Assert - var configuration = provider.GetRequiredService(); - configuration.ShouldNotBeNull(); - configuration.Host.ShouldBe(options.StatsDHost); - configuration.Prefix.ShouldBeEmpty(); - - var source = provider.GetRequiredService(); - source.ShouldNotBeNull(); - - var transport = provider.GetRequiredService(); - transport.ShouldNotBeNull(); - transport.ShouldBeOfType(); - - var publisher = provider.GetRequiredService(); - publisher.ShouldNotBeNull(); - publisher.ShouldBeOfType(); - } - finally - { - if (provider is IDisposable disposable) - { - disposable.Dispose(); - } - } + // Assert + var configuration = provider.GetRequiredService(); + configuration.ShouldNotBeNull(); + configuration.Host.ShouldBe(options.StatsDHost); + configuration.Prefix.ShouldBeEmpty(); + + var source = provider.GetRequiredService(); + source.ShouldNotBeNull(); + + var transport = provider.GetRequiredService(); + transport.ShouldNotBeNull(); + transport.ShouldBeOfType(); + + var publisher = provider.GetRequiredService(); + publisher.ShouldNotBeNull(); + publisher.ShouldBeOfType(); } [Fact] @@ -192,17 +158,19 @@ public static void RegisteringServicesDoesNotOverwriteExistingRegistrations() var existingSource = Substitute.For(); var existingTransport = Substitute.For(); var existingPublisher = Substitute.For(); + var existingPublisherWithTags = Substitute.For(); - var provider = Configure(services => - { - services.AddSingleton(existingConfig); - services.AddSingleton(existingSource); - services.AddSingleton(existingTransport); - services.AddSingleton(existingPublisher); + using var provider = Configure(services => + { + services.AddSingleton(existingConfig); + services.AddSingleton(existingSource); + services.AddSingleton(existingTransport); + services.AddSingleton(existingPublisher); + services.AddSingleton(existingPublisherWithTags); - // Act - services.AddStatsD(); - }); + // Act + services.AddStatsD(); + }); // Assert var configuration = provider.GetRequiredService(); @@ -216,6 +184,9 @@ public static void RegisteringServicesDoesNotOverwriteExistingRegistrations() var publisher = provider.GetRequiredService(); publisher.ShouldBe(existingPublisher); + + var publisherWithTags = provider.GetRequiredService(); + publisherWithTags.ShouldBe(existingPublisherWithTags); } [Fact] @@ -251,13 +222,13 @@ public static void CanRegisterServicesWithCustomTransport() // Arrange string host = "localhost"; - var provider = Configure(services => - { - services.AddSingleton(); + using var provider = Configure(services => + { + services.AddSingleton(); - // Act - services.AddStatsD(host); - }); + // Act + services.AddStatsD(host); + }); // Assert var configuration = provider.GetRequiredService(); @@ -283,13 +254,13 @@ public static void CanRegisterServicesWithIPTransport() // Arrange string host = "127.0.0.1"; - var provider = Configure(services => - { - // Act - services.AddSingleton( - ctx => new SocketTransport(ctx.GetRequiredService(), SocketProtocol.IP)); - services.AddStatsD(host); - }); + using var provider = Configure(services => + { + // Act + services.AddSingleton( + ctx => new SocketTransport(ctx.GetRequiredService(), SocketProtocol.IP)); + services.AddStatsD(host); + }); // Assert var configuration = provider.GetRequiredService();