Skip to content

Commit dedf36c

Browse files
committed
fix: Propagate framework scopes
Our logging providers need to implement ISupportExternalScope in order to retain scopes from Kestrel. Fixes #509.
1 parent e0fa018 commit dedf36c

File tree

10 files changed

+93
-27
lines changed

10 files changed

+93
-27
lines changed

src/Google.Cloud.Functions.Hosting.Tests/Logging/JsonConsoleLoggerTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public void Scopes()
9494
private static JObject GetLogEntry(string category, Action<ILogger> action)
9595
{
9696
var builder = new StringBuilder();
97-
ILogger logger = new JsonConsoleLogger(category, new StringWriter(builder));
97+
ILogger logger = new JsonConsoleLogger(category, scopeProvider: null, new StringWriter(builder));
9898
action(logger);
9999
return JObject.Parse(builder.ToString());
100100
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright 2024, Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
using Google.Cloud.Functions.Framework;
16+
using Google.Cloud.Functions.Testing;
17+
using Microsoft.AspNetCore.Http;
18+
using Microsoft.Extensions.Logging;
19+
using System.Threading.Tasks;
20+
using Xunit;
21+
22+
namespace Google.Cloud.Functions.Hosting.Tests.Logging;
23+
24+
public class LoggingFunction : IHttpFunction
25+
{
26+
private readonly ILogger _logger;
27+
28+
public LoggingFunction(ILogger<LoggingFunction> logger) =>
29+
_logger = logger;
30+
31+
public Task HandleAsync(HttpContext context)
32+
{
33+
_logger.LogInformation("Test log entry");
34+
return Task.CompletedTask;
35+
}
36+
}
37+
38+
public class LoggingIntegrationTest : FunctionTestBase<LoggingFunction>
39+
{
40+
// Note: this test doesn't test JsonConsoleLogger and SimpleConsoleLogger directly;
41+
// it's testing MemoryLoggerProvider's handling of scope providers - but that's basically
42+
// the same code as in FactoryLoggerProvider, so it helps to build confidence.
43+
// (The others have been tested manually.)
44+
[Fact]
45+
public async Task LogEntryHasScopes()
46+
{
47+
await ExecuteHttpGetRequestAsync();
48+
var logEntries = GetFunctionLogEntries();
49+
var entry = Assert.Single(logEntries);
50+
Assert.Equal("Test log entry", entry.Message);
51+
// Kestrel adds two scopes: one of SpanId/TraceId/ParentId, one of RequestPath/RequestId.
52+
Assert.Equal(2, entry.Scopes.Count);
53+
}
54+
}

src/Google.Cloud.Functions.Hosting.Tests/Logging/SimpleConsoleLoggerTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ private static string SkipTimestamp(string line)
116116
private static List<string> GetLogLines(string category, Action<ILogger> action)
117117
{
118118
var builder = new StringBuilder();
119-
ILogger logger = new SimpleConsoleLogger(category, new StringWriter(builder));
119+
ILogger logger = new SimpleConsoleLogger(category, scopeProvider: null, new StringWriter(builder));
120120
action(logger);
121121
var reader = new StringReader(builder.ToString());
122122
List<string> ret = new List<string>();

src/Google.Cloud.Functions.Hosting/Extensions/FunctionsFrameworkLoggingExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public static ILoggingBuilder AddFunctionsConsoleLogging(this ILoggingBuilder bu
3636
{
3737
var options = FunctionsFrameworkOptions.FromConfiguration(context.Configuration);
3838
ILoggerProvider provider = options.JsonLogging
39-
? new FactoryLoggerProvider(category => new JsonConsoleLogger(category, System.Console.Out))
40-
: new FactoryLoggerProvider(category => new SimpleConsoleLogger(category, System.Console.Out));
39+
? new FactoryLoggerProvider((category, scopeProvider) => new JsonConsoleLogger(category, scopeProvider, System.Console.Out))
40+
: new FactoryLoggerProvider((category, scopeProvider) => new SimpleConsoleLogger(category, scopeProvider, System.Console.Out));
4141
builder.AddProvider(provider);
4242
return builder;
4343
}

src/Google.Cloud.Functions.Hosting/Logging/FactoryLoggerProvider.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,18 @@ namespace Google.Cloud.Functions.Hosting.Logging
2020
/// <summary>
2121
/// Simple logger provider that just calls a factory method each time it's asked for logger.
2222
/// </summary>
23-
internal class FactoryLoggerProvider : ILoggerProvider
23+
internal class FactoryLoggerProvider : ILoggerProvider, ISupportExternalScope
2424
{
25-
private readonly Func<string, ILogger> _factory;
25+
private readonly Func<string, IExternalScopeProvider?, ILogger> _factory;
26+
private IExternalScopeProvider? _externalScopeProvider;
2627

27-
internal FactoryLoggerProvider(Func<string, ILogger> factory) =>
28+
void ISupportExternalScope.SetScopeProvider(IExternalScopeProvider externalScopeProvider) =>
29+
_externalScopeProvider = externalScopeProvider;
30+
31+
internal FactoryLoggerProvider(Func<string, IExternalScopeProvider?, ILogger> factory) =>
2832
_factory = factory;
2933

30-
public ILogger CreateLogger(string categoryName) => _factory(categoryName);
34+
public ILogger CreateLogger(string categoryName) => _factory(categoryName, _externalScopeProvider);
3135

3236
public void Dispose()
3337
{

src/Google.Cloud.Functions.Hosting/Logging/JsonConsoleLogger.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ internal class JsonConsoleLogger : LoggerBase
3434

3535
private readonly TextWriter _console;
3636

37-
internal JsonConsoleLogger(string category, TextWriter console)
38-
: base(category) => _console = console;
37+
internal JsonConsoleLogger(string category, IExternalScopeProvider? scopeProvider, TextWriter console)
38+
: base(category, scopeProvider) => _console = console;
3939

4040
protected override void LogImpl<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, string formattedMessage)
4141
{

src/Google.Cloud.Functions.Hosting/Logging/LoggerBase.cs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,11 @@ internal abstract class LoggerBase : ILogger
3333

3434
private readonly bool _isKestrelCategory;
3535
protected string Category { get; }
36-
protected IExternalScopeProvider ScopeProvider { get; } = new LoggerExternalScopeProvider();
36+
protected IExternalScopeProvider ScopeProvider { get; }
3737

38-
protected LoggerBase(string category) =>
39-
(Category, _isKestrelCategory) = (category, category == KestrelCategory);
38+
protected LoggerBase(string category, IExternalScopeProvider? scopeProvider) =>
39+
(Category, _isKestrelCategory, ScopeProvider) =
40+
(category, category == KestrelCategory, scopeProvider ?? new LoggerExternalScopeProvider());
4041

4142
public IDisposable BeginScope<TState>(TState state) => ScopeProvider.Push(state);
4243

@@ -68,14 +69,6 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
6869
/// </summary>
6970
protected abstract void LogImpl<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, string formattedMessage);
7071

71-
// Used for scope handling.
72-
private class SingletonDisposable : IDisposable
73-
{
74-
internal static readonly SingletonDisposable Instance = new SingletonDisposable();
75-
private SingletonDisposable() { }
76-
public void Dispose() { }
77-
}
78-
7972
/// <summary>
8073
/// Convenience method to convert a value into a string using the invariant
8174
/// culture for formatting. Null input is converted into an empty string.

src/Google.Cloud.Functions.Hosting/Logging/SimpleConsoleLogger.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ internal class SimpleConsoleLogger : LoggerBase
2525
{
2626
private readonly TextWriter _console;
2727

28-
internal SimpleConsoleLogger(string category, TextWriter console)
29-
: base(category) => _console = console;
28+
internal SimpleConsoleLogger(string category, IExternalScopeProvider? scopeProvider, TextWriter console)
29+
: base(category, scopeProvider) => _console = console;
3030

3131
protected override void LogImpl<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, string formattedMessage)
3232
{

src/Google.Cloud.Functions.Testing/MemoryLogger.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,23 @@ public class MemoryLogger : ILogger
2727
{
2828
private readonly string _categoryName;
2929
private readonly ConcurrentQueue<TestLogEntry> _logEntries = new ConcurrentQueue<TestLogEntry>();
30-
private readonly IExternalScopeProvider _scopeProvider = new LoggerExternalScopeProvider();
30+
private readonly IExternalScopeProvider _scopeProvider;
3131

3232
/// <summary>
3333
/// Creates a logger with the given category name.
3434
/// </summary>
3535
/// <param name="categoryName">The category name of the logger.</param>
36-
public MemoryLogger(string categoryName) => _categoryName = categoryName;
36+
public MemoryLogger(string categoryName) : this(categoryName, null)
37+
{
38+
}
39+
40+
/// <summary>
41+
/// Creates a logger with the given category name and scope provider
42+
/// </summary>
43+
/// <param name="categoryName">The category name of the logger.</param>
44+
/// <param name="scopeProvider">The scope provider to use, or null to create a new provider.</param>
45+
public MemoryLogger(string categoryName, IExternalScopeProvider? scopeProvider) =>
46+
(_categoryName, _scopeProvider) = (categoryName, scopeProvider ?? new LoggerExternalScopeProvider());
3747

3848
/// <summary>
3949
/// Clears the log entries in this logger.

src/Google.Cloud.Functions.Testing/MemoryLoggerProvider.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,18 @@ namespace Google.Cloud.Functions.Testing
2121
/// <summary>
2222
/// A logger provider that creates instances of <see cref="MemoryLogger"/>.
2323
/// </summary>
24-
internal sealed class MemoryLoggerProvider : ILoggerProvider
24+
internal sealed class MemoryLoggerProvider : ILoggerProvider, ISupportExternalScope
2525
{
2626
private readonly ConcurrentDictionary<string, MemoryLogger> _loggersByCategory =
2727
new ConcurrentDictionary<string, MemoryLogger>();
2828

29+
private IExternalScopeProvider? _scopeProvider;
30+
31+
void ISupportExternalScope.SetScopeProvider(IExternalScopeProvider scopeProvider) =>
32+
_scopeProvider = scopeProvider;
33+
2934
public ILogger CreateLogger(string categoryName) =>
30-
_loggersByCategory.GetOrAdd(categoryName, name => new MemoryLogger(name));
35+
_loggersByCategory.GetOrAdd(categoryName, name => new MemoryLogger(name, _scopeProvider));
3136

3237
internal void Clear() => _loggersByCategory.Clear();
3338

0 commit comments

Comments
 (0)