Skip to content

Commit

Permalink
feat: Copies GoogleLogger to Common.
Browse files Browse the repository at this point in the history 
This allows easier use of GoogleLogger in non ASP.NET Core applications.
Towards #6367

- Replicate LoggerOptions in Common.
  - And have AspNetCore*.LoggerOptions be just a wrapper of Common.LoggerOptions.
- Copies ILogEntryLabelProvider to Common.
  - And marks the one in AspNetCore* Obsolete.
- Makes AspNetCore*.IExternalTraceProvider obsolete.
  - It can now be replaced by Common.ITraceContext.
  • Loading branch information
@amanda-tarafa @jskeet
amanda-tarafa authored and jskeet committed Jun 24, 2021
1 parent 7fbe862 commit 60e8cd8
Show file tree
Hide file tree
Showing 26 changed files with 922 additions and 445 deletions.
75 changes: 63 additions & 12 deletions ...cs.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.IntegrationTests/Logging/LoggingTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ public static object[][] ExternalTraceBuilders
{
return new object[][]
{
new object[] { GetTestServer<ObsoleteNoBufferWarningLoggerExternalTraceTestApplication>() },
new object[] { GetTestServer<ObsoleteExternalTracingTestApplication>() },
new object[] { GetTestServer<NoBufferWarningLoggerExternalTraceTestApplication>() },
new object[] { GetTestServer<ExternalTracingTestApplication>() }
};
Expand Down Expand Up @@ -546,10 +548,10 @@ public async Task Logging_Trace_External_OneEntry(TestServer server)
// And the resource name of the trace associated to it contains the external trace id.
Assert.Contains(TestEnvironment.GetTestProjectId(), entry.Trace);
Assert.Contains("external_trace_id", entry.Trace);
Assert.Contains("105445aa7843bc8bf206b12000100f00", entry.Trace);
// The span associated to our entry is the external span.
Assert.Equal("external_span_number1", entry.SpanId);
Assert.Equal("0000000000000001", entry.SpanId);
});
}

Expand Down Expand Up @@ -578,12 +580,14 @@ public async Task Logging_Trace_External_MultipleEntries(TestServer server)
Assert.All(results, entry =>
{
Assert.Contains(projectId, entry.Trace);
Assert.Contains("external_trace_id", entry.Trace);
Assert.Contains("105445aa7843bc8bf206b12000100f00", entry.Trace);
});
// The span associated to our entry is the external span, and is the same span number
// as the log entry.
Assert.All(results, entry => Assert.Equal($"external_span_number{entry.JsonPayload.Fields["message"].StringValue.Last()}", entry.SpanId));
Assert.All(results, entry => Assert.Equal($"{ExpectedSpanId(entry):x16}", entry.SpanId));
static ulong ExpectedSpanId(LogEntry entry) =>
Convert.ToUInt64(int.Parse($"{entry.JsonPayload.Fields["message"].StringValue.Last()}"));
});
}

Expand Down Expand Up @@ -668,9 +672,10 @@ public async Task Logging_Labels()
_fixture.AddValidator(testId, results =>
{
var entry = results.Single();
Assert.Equal(3, entry.Labels.Count);
Assert.Equal(4, entry.Labels.Count);
Assert.Equal("some-value", entry.Labels["some-key"]);
Assert.Equal("Hello, World!", entry.Labels["Foo"]);
Assert.Equal("Hello, World!", entry.Labels["Bar"]);
Assert.NotEmpty(entry.Labels["trace_identifier"]);
});
}
Expand Down Expand Up @@ -828,16 +833,28 @@ public abstract class LoggerNoTracingActivatedTestApplication : BaseStartup
app.ApplicationServices, _projectId, LoggerOptions.Create(logLevel: LogLevel.Warning, bufferOptions: BufferOptions.NoBuffer())));
}

public class ExternalTracingTestApplication : LoggerNoTracingActivatedTestApplication
public class ObsoleteExternalTracingTestApplication : LoggerNoTracingActivatedTestApplication
{
public override void ConfigureServices(IServiceCollection services) =>
#pragma warning disable CS0618 // Type or member is obsolete
base.ConfigureServices(services.AddSingleton<IExternalTraceProvider, SpanCountingExternalTraceProvider>());
#pragma warning restore CS0618 // Type or member is obsolete
}

public class ExternalTracingTestApplication : LoggerNoTracingActivatedTestApplication
{
public override void ConfigureServices(IServiceCollection services) =>
base.ConfigureServices(services
// We start counting at -1 here because of the call we make on the middleware to
// try and obtain the context. We don't care about that one.
.AddSingleton(new SpanCountingITraceContextFactory(0))
.AddTransient(sp => sp.GetRequiredService<SpanCountingITraceContextFactory>().GetCurrentTraceContext()));
}

public class GoogleTraceAsExternalTracingTestApplication : LoggerNoTracingActivatedTestApplication
{
public override void ConfigureServices(IServiceCollection services) =>
base.ConfigureServices(services.AddSingleton<IExternalTraceProvider, GoogleTraceProvider>());
base.ConfigureServices(services.TryAddGoogleTraceContextProvider());
}

/// <summary>
Expand Down Expand Up @@ -894,10 +911,22 @@ public NoBufferWarningLoggerTracesAllTestApplication()
{ }
}

public class NoBufferWarningLoggerExternalTraceTestApplication : NoBufferWarningLoggerTestApplication
public class ObsoleteNoBufferWarningLoggerExternalTraceTestApplication : NoBufferWarningLoggerTestApplication
{
public override void ConfigureServices(IServiceCollection services) =>
#pragma warning disable CS0618 // Type or member is obsolete
base.ConfigureServices(services.AddSingleton<IExternalTraceProvider>(new SpanCountingExternalTraceProvider()));
#pragma warning restore CS0618 // Type or member is obsolete
}

public class NoBufferWarningLoggerExternalTraceTestApplication : NoBufferWarningLoggerTestApplication
{
public override void ConfigureServices(IServiceCollection services) =>
base.ConfigureServices(services
// We start counting at -1 here because of the call we make on the middleware to
// try and obtain the context. We don't care about that one.
.AddSingleton(new SpanCountingITraceContextFactory(-1))
.AddTransient(sp => sp.GetRequiredService<SpanCountingITraceContextFactory>().GetCurrentTraceContext()));
}

/// <summary>
Expand All @@ -924,15 +953,18 @@ public class NoBufferResourceLoggerTestApplication : LoggerTestApplication
}

/// <summary>
/// An application that has a <see cref="GoogleLogger"/> and a <see cref="ILogEntryLabelProvider"/>,
/// An application that has a <see cref="GoogleLogger"/> and a <see cref="Common.ILogEntryLabelProvider"/>,
/// that accept all logs of level warning or above.
/// </summary>
public class WarningWithLabelsLoggerTestApplication : LoggerTestApplication
{
public override void ConfigureServices(IServiceCollection services) =>
base.ConfigureServices(services
.AddLogEntryLabelProvider<FooLogEntryLabelProvider>()
.AddLogEntryLabelProvider<TraceIdLogEntryLabelProvider>());
.AddLogEntryLabelProvider<TraceIdLogEntryLabelProvider>()
#pragma warning disable CS0618 // Type or member is obsolete
.AddSingleton<ILogEntryLabelProvider, BarLogEntryLabelProvider>());
#pragma warning restore CS0618 // Type or member is obsolete

public override void Configure(IApplicationBuilder app, ILoggerFactory loggerFactory) =>
base.Configure(app, loggerFactory.AddGoogle(
Expand Down Expand Up @@ -1141,18 +1173,37 @@ public string LogsThreeEntries(string id)
}
}

internal class FooLogEntryLabelProvider : ILogEntryLabelProvider
internal class FooLogEntryLabelProvider : Common.ILogEntryLabelProvider
{
public void Invoke(Dictionary<string, string> labels)
{
labels["Foo"] = "Hello, World!";
}
}

[Obsolete("Added to test that we still support the obsolete ILogEntryLabelProvider.")]
internal class BarLogEntryLabelProvider : ILogEntryLabelProvider
{
public void Invoke(Dictionary<string, string> labels)
{
labels["Bar"] = "Hello, World!";
}
}

internal class SpanCountingITraceContextFactory
{
public SpanCountingITraceContextFactory(int startAt) => _callCount = startAt;
private int _callCount = 0;
internal ITraceContext GetCurrentTraceContext() =>
new SimpleTraceContext("105445aa7843bc8bf206b12000100f00", Convert.ToUInt64(Interlocked.Increment(ref _callCount)), false);
}

#pragma warning disable CS0618 // Type or member is obsolete
internal class SpanCountingExternalTraceProvider : IExternalTraceProvider
{
internal int _callCount = 0;
public TraceContextForLogEntry GetCurrentTraceContext(IServiceProvider serviceProvider) =>
new TraceContextForLogEntry("external_trace_id", $"external_span_number{Interlocked.Increment(ref _callCount)}");
new TraceContextForLogEntry("105445aa7843bc8bf206b12000100f00", $"{Convert.ToUInt64(Interlocked.Increment(ref _callCount)):x16}");
}
#pragma warning restore CS0618 // Type or member is obsolete
}
47 changes: 25 additions & 22 deletions ...gnostics.AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Logging/GoogleLoggerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,15 @@ private string Formatter(string message, Exception ex)
consumer ??= new Mock<IConsumer<LogEntry>>(MockBehavior.Strict).Object;
monitoredResource ??= MonitoredResourceBuilder.GlobalResource;
logTarget ??= s_defaultLogTarget;
LoggerOptions options = LoggerOptions.CreateWithServiceContext(
Common.LoggerOptions options = Common.LoggerOptions.CreateWithServiceContext(
logLevel, logName, labels, monitoredResource, retryOptions: retryOptions, serviceName: serviceName, version: version);
return new GoogleLogger(consumer, logTarget, options, LogName, s_clock, serviceProvider);
#pragma warning disable CS0618 // Type or member is obsolete
Common.GoogleLogger _innerLogger = new Common.GoogleLogger(
consumer, logTarget, options, LogName,
GoogleLoggerProvider.ObsoleteLabelsGetter, GoogleLoggerProvider.ObsoleteTraceContextGetter,
s_clock, serviceProvider);
#pragma warning restore CS0618 // Type or member is obsolete
return new GoogleLogger(_innerLogger);
}

[Fact]
Expand Down Expand Up @@ -439,23 +445,20 @@ public void Log_NoFormatParams()
public void Log_Trace()
{
string traceId = "external_trace_id";
string spanId = "external_span_id";
ulong spanId = 1;
string fullTraceName = TraceTarget.ForProject(ProjectId).GetFullTraceName(traceId);

Predicate<IEnumerable<LogEntry>> matcher = logEntries =>
{
LogEntry entry = logEntries.Single();
return entry.LogName == new LogName(ProjectId, BaseLogName).ToString() &&
entry.Trace == fullTraceName &&
entry.SpanId == spanId;
entry.SpanId == $"{spanId:x16}";
};

var mockServiceProvider = new Mock<IServiceProvider>();
var mockExternalTraceProvider = new Mock<IExternalTraceProvider>();
mockServiceProvider.Setup(sp => sp.GetService(typeof(IExternalTraceProvider))).Returns(mockExternalTraceProvider.Object);
mockExternalTraceProvider.Setup(
sp => sp.GetCurrentTraceContext(It.IsAny<IServiceProvider>()))
.Returns(new TraceContextForLogEntry(traceId, spanId));
var traceContext = new SimpleTraceContext(traceId, spanId, true);
mockServiceProvider.Setup(sp => sp.GetService(typeof(ITraceContext))).Returns(traceContext);

var mockConsumer = new Mock<IConsumer<LogEntry>>();
mockConsumer.Setup(c => c.Receive(Match.Create(matcher)));
Expand All @@ -481,8 +484,8 @@ public void Log_Labels()
};

var mockServiceProvider = new Mock<IServiceProvider>();
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<ILogEntryLabelProvider>)))
.Returns(new ILogEntryLabelProvider[] { new FooLogEntryLabelProvider(), new BarLogEntryLabelProvider() });
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<Common.ILogEntryLabelProvider>)))
.Returns(new Common.ILogEntryLabelProvider[] { new FooLogEntryLabelProvider(), new BarLogEntryLabelProvider() });

var mockConsumer = new Mock<IConsumer<LogEntry>>();
mockConsumer.Setup(c => c.Receive(Match.Create(matcher)));
Expand All @@ -505,8 +508,8 @@ public void Log_Exception()
};

var mockServiceProvider = new Mock<IServiceProvider>();
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<ILogEntryLabelProvider>)))
.Returns(new ILogEntryLabelProvider[] { new FooLogEntryLabelProvider(), new BarLogEntryLabelProvider() });
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<Common.ILogEntryLabelProvider>)))
.Returns(new Common.ILogEntryLabelProvider[] { new FooLogEntryLabelProvider(), new BarLogEntryLabelProvider() });

var mockConsumer = new Mock<IConsumer<LogEntry>>();
mockConsumer.Setup(c => c.Receive(Match.Create(matcher)));
Expand All @@ -519,8 +522,8 @@ public void Log_Exception()
public void Log_DoesNotLogIfNullLabels()
{
var mockServiceProvider = new Mock<IServiceProvider>();
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<ILogEntryLabelProvider>)))
.Returns(new ILogEntryLabelProvider[] { new EmptyLogEntryLabelProvider() });
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<Common.ILogEntryLabelProvider>)))
.Returns(new Common.ILogEntryLabelProvider[] { new EmptyLogEntryLabelProvider() });

var mockConsumer = new Mock<IConsumer<LogEntry>>();
var logger = GetLogger(mockConsumer.Object, LogLevel.Information, serviceProvider: mockServiceProvider.Object, logName: BaseLogName);
Expand All @@ -532,8 +535,8 @@ public void Log_DoesNotLogIfNullLabels()
public void Log_ThrowsIfNullLabels_RetryOptionsPropagateExceptions()
{
var mockServiceProvider = new Mock<IServiceProvider>();
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<ILogEntryLabelProvider>)))
.Returns(new ILogEntryLabelProvider[] { new EmptyLogEntryLabelProvider() });
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<Common.ILogEntryLabelProvider>)))
.Returns(new Common.ILogEntryLabelProvider[] { new EmptyLogEntryLabelProvider() });

var mockConsumer = new Mock<IConsumer<LogEntry>>();
var logger = GetLogger(mockConsumer.Object, LogLevel.Information, serviceProvider: mockServiceProvider.Object, logName: BaseLogName, retryOptions: RetryOptions.NoRetry(ExceptionHandling.Propagate));
Expand Down Expand Up @@ -564,8 +567,8 @@ public void Log_Labels_DefaultLabelsFirst()
};

var mockServiceProvider = new Mock<IServiceProvider>();
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<ILogEntryLabelProvider>)))
.Returns(new ILogEntryLabelProvider[] { new FooLogEntryLabelProvider(), new BarLogEntryLabelProvider() });
mockServiceProvider.Setup(sp => sp.GetService(typeof(IEnumerable<Common.ILogEntryLabelProvider>)))
.Returns(new Common.ILogEntryLabelProvider[] { new FooLogEntryLabelProvider(), new BarLogEntryLabelProvider() });

var mockConsumer = new Mock<IConsumer<LogEntry>>();
mockConsumer.Setup(c => c.Receive(Match.Create(matcher)));
Expand Down Expand Up @@ -666,23 +669,23 @@ public void Log_NoServiceContext()
}
}

internal class FooLogEntryLabelProvider : ILogEntryLabelProvider
internal class FooLogEntryLabelProvider : Common.ILogEntryLabelProvider
{
public void Invoke(Dictionary<string, string> labels)
{
labels["Foo"] = "Hello";
}
}

internal class BarLogEntryLabelProvider : ILogEntryLabelProvider
internal class BarLogEntryLabelProvider : Common.ILogEntryLabelProvider
{
public void Invoke(Dictionary<string, string> labels)
{
labels["Bar"] = "World";
}
}

internal class EmptyLogEntryLabelProvider : ILogEntryLabelProvider
internal class EmptyLogEntryLabelProvider : Common.ILogEntryLabelProvider
{
public void Invoke(Dictionary<string, string> labels)
{
Expand Down
3 changes: 3 additions & 0 deletions ....AspNetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Logging/GoogleTraceProviderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace Google.Cloud.Diagnostics.AspNetCore.Tests
#error unknown target framework
#endif
{
#pragma warning disable CS0618 // Type or member is obsolete
public class GoogleTraceProviderTests
{
[Fact]
Expand All @@ -41,6 +42,7 @@ public void GetCurrentTraceContext()

IServiceProvider serviceProvider = MockServiceProvider(traceId, spanId, true);


GoogleTraceProvider traceProvider = new GoogleTraceProvider();
TraceContextForLogEntry traceContext = traceProvider.GetCurrentTraceContext(serviceProvider);

Expand Down Expand Up @@ -83,4 +85,5 @@ private IServiceProvider MockServiceProvider(string traceId, ulong spanId, bool
return serviceProviderMock.Object;
}
}
#pragma warning restore CS0618 // Type or member is obsolete
}
6 changes: 3 additions & 3 deletions ....Cloud.Diagnostics.AspNetCore.Tests/Logging/LabelProviders/LabelProviderExtensionsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void AddsServiceFromType()
Assert.Single(services);
var descriptor = services.Single();

Assert.Equal(typeof(ILogEntryLabelProvider), descriptor.ServiceType);
Assert.Equal(typeof(Common.ILogEntryLabelProvider), descriptor.ServiceType);
Assert.Equal(typeof(TraceIdLogEntryLabelProvider), descriptor.ImplementationType);
Assert.Equal(ServiceLifetime.Singleton, descriptor.Lifetime);
}
Expand All @@ -57,7 +57,7 @@ public void AddsServiceFromFactory()
Assert.Single(services);
var descriptor = services.Single();

Assert.Equal(typeof(ILogEntryLabelProvider), descriptor.ServiceType);
Assert.Equal(typeof(Common.ILogEntryLabelProvider), descriptor.ServiceType);
Assert.NotNull(descriptor.ImplementationFactory);
Assert.Equal(ServiceLifetime.Singleton, descriptor.Lifetime);
}
Expand All @@ -74,7 +74,7 @@ public void AddsServiceFromInstance()
Assert.Single(services);
var descriptor = services.Single();

Assert.Equal(typeof(ILogEntryLabelProvider), descriptor.ServiceType);
Assert.Equal(typeof(Common.ILogEntryLabelProvider), descriptor.ServiceType);
Assert.IsType<TraceIdLogEntryLabelProvider>(descriptor.ImplementationInstance);
Assert.Equal(ServiceLifetime.Singleton, descriptor.Lifetime);
}
Expand Down
2 changes: 2 additions & 0 deletions ...NetCore/Google.Cloud.Diagnostics.AspNetCore.Tests/Logging/TraceContextForLogEntryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace Google.Cloud.Diagnostics.AspNetCore.Tests
#error unknown target framework
#endif
{
#pragma warning disable CS0618 // Type or member is obsolete
public class TraceContextForLogEntryTests
{
[Fact]
Expand Down Expand Up @@ -98,4 +99,5 @@ private IManagedTracer MockTracer(string traceId = null, ulong? spanId = null)
return tracerMock.Object;
}
}
#pragma warning restore CS0618 // Type or member is obsolete
}
Loading

0 comments on commit 60e8cd8

Please sign in to comment.