Skip to content

Commit

Permalink
fix: Adds AddGoogleDiagnostics empty overload.
Browse files Browse the repository at this point in the history
To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
This is the same as what was done on #7642 for the ASP.NET Core packages, but for the Common package instead, which had the same issue.
  • Loading branch information
amanda-tarafa committed Jan 5, 2022
1 parent ba21870 commit d5c3963
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 15 deletions.
Expand Up @@ -65,7 +65,7 @@ public async Task UseGoogleDiagnostics_ConfiguresServices_Default()
using var client = server.CreateClient();
await TestTrace(testId, startTime, client);
await TestLogging(testId, startTime, client);
await TestErrorReporting(testId, client);
await TestErrorReporting(testId, client, verifyServiceAndVersion: false);
}

[Fact]
Expand Down Expand Up @@ -182,7 +182,7 @@ private static async Task TestTrace(string testId, DateTimeOffset startTime, Htt
Assert.False(response.Headers.Contains(TraceHeaderContext.TraceHeader));
}

private static async Task TestErrorReporting(string testId, HttpClient client)
private static async Task TestErrorReporting(string testId, HttpClient client, bool verifyServiceAndVersion = true)
{
await Assert.ThrowsAsync<Exception>(() => client.GetAsync($"/ErrorReporting/{nameof(ErrorReportingController.ThrowsException)}/{testId}"));
var errorEvent = ErrorEventEntryVerifiers.VerifySingle(ErrorEventEntryPolling.Default, testId);
Expand Down
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using Google.Api.Gax;
using Google.Cloud.ClientTesting;
using Google.Cloud.Logging.Type;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -40,15 +41,20 @@ public DiagnosticsTests()
_startTime = DateTimeOffset.UtcNow;
}

private static class EmptyHostBuilder
{
public static IHostBuilder CreateHostBuilder() =>
new HostBuilder()
.ConfigureLogging(builder => builder.ClearProviders())
.ConfigureServices(services => services.AddGoogleDiagnostics());
}

private static class DefaultHostBuilder
{
public static IHostBuilder CreateHostBuilder() =>
new HostBuilder()
.ConfigureLogging(builder => builder.ClearProviders())
.ConfigureServices(services =>
{
services.AddGoogleDiagnostics(ProjectId, Service, Version);
});
.ConfigureServices(services => services.AddGoogleDiagnostics(ProjectId, Service, Version));
}

private static class WithOptionsHostBuilder
Expand All @@ -57,14 +63,12 @@ private static class WithOptionsHostBuilder
new HostBuilder()
.ConfigureLogging(builder => builder.ClearProviders())
.ConfigureServices(services =>
{
// We don't care much about which options we are setting.
// These are for testing that all the extension method overloads work as expected.
services.AddGoogleDiagnostics(ProjectId, Service, Version,
TraceOptions.Create(retryOptions: RetryOptions.NoRetry(ExceptionHandling.Propagate)),
LoggingOptions.Create(retryOptions: RetryOptions.NoRetry(ExceptionHandling.Propagate)),
ErrorReportingOptions.CreateInstance(retryOptions: RetryOptions.NoRetry(ExceptionHandling.Propagate)));
});
ErrorReportingOptions.CreateInstance(retryOptions: RetryOptions.NoRetry(ExceptionHandling.Propagate))));
}

private static class WithServiceOptionsHostBuilder
Expand Down Expand Up @@ -106,6 +110,11 @@ public static IEnumerable<object[]> HostBuilderData
yield return new object[] { (Func<IHostBuilder>)DefaultHostBuilder.CreateHostBuilder };
yield return new object[] { (Func<IHostBuilder>)WithOptionsHostBuilder.CreateHostBuilder };
yield return new object[] { (Func<IHostBuilder>)WithServiceOptionsHostBuilder.CreateHostBuilder };
// Skip these tests if we are not running on GCP
if (Platform.Instance().Type != PlatformType.Unknown)
{
yield return new object[] { (Func<IHostBuilder>)EmptyHostBuilder.CreateHostBuilder };
}
}
}

Expand Down Expand Up @@ -193,7 +202,7 @@ public async Task LogsExceptionAsync(Func<IHostBuilder> createHostBuilder)

var errorEvent = ErrorEventEntryVerifiers.VerifySingle(ErrorEventEntryPolling.Default, _testId);
ErrorEventEntryVerifiers.VerifyFullErrorEventLogged(
errorEvent, _testId, nameof(ThrowsException), verifyHttpContext: false);
errorEvent, _testId, nameof(ThrowsException), verifyHttpContext: false, verifyServiceAndVersion: false);
}
finally
{
Expand Down
Expand Up @@ -50,9 +50,10 @@ public static IEnumerable<ErrorEvent> VerifyMany(ErrorEventEntryPolling polling,
/// Checks that an <see cref="ErrorEvent"/> contains valid data,
/// including HTTP Context data.
/// </summary>
public static void VerifyFullErrorEventLogged(ErrorEvent errorEvent, string testId, string functionName, bool verifyHttpContext = true)
public static void VerifyFullErrorEventLogged(ErrorEvent errorEvent, string testId, string functionName,
bool verifyHttpContext = true, bool verifyServiceAndVersion = true)
{
VerifyErrorEventLogged(errorEvent, testId, functionName);
VerifyErrorEventLogged(errorEvent, testId, functionName, verifyServiceAndVersion);
if (verifyHttpContext)
{
VerifyHttpContextLogged(errorEvent);
Expand All @@ -66,10 +67,13 @@ public static void VerifyFullErrorEventLogged(ErrorEvent errorEvent, string test
/// Google.Cloud.Diagnostics.GoogleExceptionLogger is used instead of the
/// Google.Cloud.Diagnostics.GoogleWebApiExceptionLogger.
/// </summary>
public static void VerifyErrorEventLogged(ErrorEvent errorEvent, string testId, string functionName)
public static void VerifyErrorEventLogged(ErrorEvent errorEvent, string testId, string functionName, bool verifyServiceAndVersion = true)
{
Assert.Equal(EntryData.Service, errorEvent.ServiceContext.Service);
Assert.Equal(EntryData.Version, errorEvent.ServiceContext.Version);
if (verifyServiceAndVersion)
{
Assert.Equal(EntryData.Service, errorEvent.ServiceContext.Service);
Assert.Equal(EntryData.Version, errorEvent.ServiceContext.Version);
}

Assert.Contains(functionName, errorEvent.Message);
Assert.Contains(testId, errorEvent.Message);
Expand Down
Expand Up @@ -21,6 +21,21 @@ namespace Google.Cloud.Diagnostics.Common
/// </summary>
public static class GoogleDiagnosticsExtensions
{
/// <summary>
/// Configures Google Diagnostics to be used in non ASP.NET Core applications.
/// </summary>
/// <remarks>
/// Note that the Google Cloud Project ID to use is required and it can only be
/// obtained from the environment if running on GCP. This means that this overload
/// can only be used when running on GCP. If you are not running on GCP or need to specify
/// the Google Cloud Project ID, you can use any of
/// <see cref="AddGoogleDiagnostics(IServiceCollection, TraceServiceOptions, LoggingServiceOptions, ErrorReportingServiceOptions)"/>
/// or
/// <see cref="AddGoogleDiagnostics(IServiceCollection, string, string, string, TraceOptions, LoggingOptions, ErrorReportingOptions)"/>.
/// </remarks>
public static IServiceCollection AddGoogleDiagnostics(this IServiceCollection services) =>
services.AddGoogleDiagnostics((TraceServiceOptions)null);

/// <summary>
/// Configures Google Diagnostics to be used in non ASP.NET Core applications.
/// </summary>
Expand Down

0 comments on commit d5c3963

Please sign in to comment.