Add logging buffering #5635

evgenyfedorov2 · 2024-11-13T15:09:37Z

Related to the #5123 proposal, this PR is focused on the logging buffering only. See also the sampling part #5574.

Microsoft Reviewers: Open in CodeFlow

src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLogger.cs

...Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferProvider.cs

src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferOptions.cs

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBufferOptions.cs

...Core.Diagnostics.Middleware.Tests/Buffering/HttpRequestBufferLoggerBuilderExtensionsTests.cs

test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs

...s/Microsoft.Extensions.Telemetry.Tests/Buffering/GlobalBufferLoggerBuilderExtensionsTests.cs

test/Libraries/Microsoft.Extensions.Telemetry.Tests/Logging/ExtendedLoggerTests.cs

...Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/IHttpRequestBufferManager.cs

geeknoid · 2024-12-15T20:58:53Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferFilterRule.cs

+/// Defines a rule used to filter log messages for purposes of futher buffering.
+/// </summary>
+[Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)]
+public class BufferFilterRule : ILoggerFilterRule


I think we need to also support filtering per name/value pair. So that you can control buffering based on things like cluster name or region.

Would cluster name and region be key-value pairs that are varying within the same process? Based on the names those sound like properties that would be fixed at process startup and remain constant throughout. This means all the decision making could occur at startup time when defining the filter rules rather than a decision that needs to be made per-log message later on. For example could you satisfy the scenario with some startup code like this?

logging.AddBuffering(buffering => { string region = GetRegionFromConfig(); // in the test region we want to buffer some log messages that won't be buffered // in other regions if(region == "TestRegion") { buffer.AddFilter("ExperimentalFeatures"); } ... });

I'm not opposed to other things, I'm just trying to better understand what the scenario is.

...soft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferLoggerBuilderExtensions.cs

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBufferOptions.cs

src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferManager.cs

Address PR comments Add .NET 8 support Add ExceptionJsonConverter

dotnet-comment-bot · 2024-12-16T15:24:35Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.03 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.51 🔻
Microsoft.Extensions.Telemetry	Line	93	86.2 🔻
Microsoft.Extensions.Telemetry	Branch	93	84.59 🔻
Microsoft.Extensions.Diagnostics.Testing	Line	99	85.78 🔻
Microsoft.Extensions.Diagnostics.Testing	Branch	99	79.57 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.Diagnostics.Probes	70	76
Microsoft.Extensions.Caching.Hybrid	75	86
Microsoft.Extensions.AI.OpenAI	72	77

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=896358&view=codecoverage-tab

src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLogger.cs

KalleOlaviNiemitalo · 2024-12-17T11:40:59Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBuffer.cs

+        {
+            case ModernTagJoiner modernTagJoiner:
+                _buffer.Enqueue(new SerializedLogRecord(logLevel, eventId, _timeProvider.GetUtcNow(), modernTagJoiner, exception,
+                    ((Func<ModernTagJoiner, Exception?, string>)(object)formatter)(modernTagJoiner, exception)));


Is this casting more performant than just formatter(attributes, exception)?

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/ILoggingBuffer.cs

KalleOlaviNiemitalo · 2024-12-17T11:48:49Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBuffer.cs

+    public void Flush()
+    {
+        var result = _buffer.ToArray();
+
+#if NETFRAMEWORK
+        lock (_netfxBufferLocker)
+        {
+            while (_buffer.TryDequeue(out _))
+            {
+                // Clear the buffer
+            }
+        }
+#else
+        _buffer.Clear();
+#endif


This loses log entries if another thread enqueues more of them between _buffer.ToArray() and _buffer.Clear(). That is more likely to happen on .NET Framework, where clearing the queue takes more time.

Moved the _lastFlushTimestamp update to the top, so that when a flush operation is initiated at one thread, another thread will skip buffering for some period of time because of the non-zero value of SuspendAfterFlushDuration option. Don't expect someone to set it to zero because it does not make sense to emit some buffered logs and buffer other at almost the same time.

KalleOlaviNiemitalo · 2024-12-17T12:01:22Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBufferManager.cs

+    {
+        while (!cancellationToken.IsCancellationRequested)
+        {
+            await _timeProvider.Delay(_options.CurrentValue.Duration, cancellationToken).ConfigureAwait(false);


IIRC, IOptionsMonitor<TOptions>.CurrentValue throws if the option is bound to configuration and the binding fails (parsing fails or a validator rejects the value); this could even happen when an operator modifies a configuration file after the application has already started. The BackgroundService would then fail and, depending on HostOptions.BackgroundServiceExceptionBehavior, either log buffer truncation would stop working or the whole application would terminate. Should this instead keep using the previous value of the options in that case?

This makes sense, but I will take another look at it later on, possibly after finalizing public API. Keeping this open until then

src/Libraries/Microsoft.Extensions.Telemetry/ILoggerFilterRule.cs

KalleOlaviNiemitalo · 2024-12-17T16:13:44Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferSink.cs

+                        logger.Log(
+                            serializedRecord.LogLevel,
+                            serializedRecord.EventId,
+                            serializedRecord.Attributes,
+                            exception,
+                            (_, _) => serializedRecord.FormattedMessage ?? string.Empty);


This allocates a closure and a delegate for each ILogger.Log<TState> call. I can think of two ways to minimize the allocations:

Instead of serializedRecord.Attributes, pass serializedRecord itself as the state to ILogger.Log<TState>. Implement IReadOnlyList<KeyValuePair<string, object?>> in SerializedLogRecord by calling the corresponding methods of public IReadOnlyList<KeyValuePair<string, string>> Attributes.

Alternatively, keep using the same delegate across all iterations of both foreach loops; declare the serializedRecord variable and a delegate variable before the outer loop. That way, it would allocate only one closure and one delegate per BufferSink.LogRecords call, no matter how many log records are in IEnumerable<T> serializedRecords. This is safe because ILogger.Log<TState> is not allowed to save the formatter delegate for later calling.

makes sense for optimizations later, thank you

This code moved from BufferSink to BufferedLoggerProxy in d7661a6. The BufferedLogRecord instances are not pooled, so each BufferedLogRecord call costs at least one allocation per BufferedLogRecord; that may make the delegate and closure allocations less important.

src/Shared/ExceptionJsonConverter/ExceptionJsonConverter.cs

KalleOlaviNiemitalo · 2024-12-18T10:14:54Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/PooledLogRecord.cs

+using Microsoft.Extensions.ObjectPool;
+
+namespace Microsoft.Extensions.Diagnostics.Buffering;
+internal sealed class PooledLogRecord : BufferedLogRecord, IResettable


Please add DebuggerDisplayAttribute so that the list of log records in a buffer can be viewed more easily.

Or add the DebuggerDisplayAttribute to SerializedLogRecord, instead.

Remove pooling for now

dotnet-comment-bot · 2025-01-14T11:38:52Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Caching.Hybrid	Line	86	84.76 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.44 🔻
Microsoft.Extensions.Telemetry	Line	93	90.62 🔻
Microsoft.Extensions.Telemetry	Branch	93	91.16 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=916781&view=codecoverage-tab

KalleOlaviNiemitalo · 2025-01-15T14:55:39Z

src/Libraries/Microsoft.Extensions.Telemetry/LoggerFilterRuleSelector.cs

+        {
+            foreach (KeyValuePair<string, object?> ruleAttribute in rule.Attributes)
+            {
+                if (!attributes.Contains(ruleAttribute))


Can the object? in the KeyValuePair<string, object?> be a list; should this have some kind of structural equality comparison then? That seems complex.

Microsoft.Extensions.Logging.LoggerFilterRule has Func<string?, string?, LogLevel, bool>? Filter, which lets the application developer implement the logic as desired. A similar callback could work here.

Updated with introducing the Filter delegate

Microsoft.Extensions.Logging.LoggerFilterRule has Func<string?, string?, LogLevel, bool>? Filter, which lets the application developer implement the logic as desired. A similar callback could work here.

Actually, even though the Func<> delegate is more flexible, we need to stick to IReadOnlyList<KeyValuePair<string, object?>> because it can be configured and bound from appsettings.json, while the Func<> delegate cannot because it is not serializable (at least, not to a human-readable format). The ability to bind from JSON config and refresh the config in runtime is the key feature of this component. Potentially we can add Func<> later on if users ask for it.

Can the object? in the KeyValuePair<string, object?> be a list; should this have some kind of structural equality comparison then? That seems complex.

Let's stick to the default equality comparer for now and consider accepting custom comparers if users ask for it.

The rules configuration of the standard LoggerFactory does not use the standard configuration binder. Instead, it explicitly reads values from IConfiguration:

https://github.com/dotnet/runtime/blob/af25787dac9b2cba8a2d4a830f280e59782878a8/src/libraries/Microsoft.Extensions.Logging.Configuration/src/LoggerFilterConfigureOptions.cs

I think it would be possible to construct the filter delegate from configuration values in a similar fashion, while allowing the application developer to replace or override this conversion.

Maybe you'll need some custom logic there anyway, if attribute values written to ILogger can be numbers but configuration provides only strings.

Although possibly it can later be extended to:

"Attributes": [ { "Key": "Duration", "Type": "System.Double", "Comparison": ">", "Value": "2.1" } ]

where the library would map type names like "System.Double" to objects that know how to parse "2.1" to (double)2.1 and how to implement the ">" comparison. This would use neither Type.GetType(string) nor System.ComponentModel.TypeConverter.

8319dc8 changed tests:

AddGlobalBuffer_WithConfiguration_RegistersInDI asserts that new KeyValuePair<string, object?>("priority", 1) is included in the attributes of the rule. I expect that this test will fail because values in IConfiguration are always strings and Microsoft.Extensions.Configuration.Binder cannot know that "1" should be converted to Int32.

CanWorkWithValueTypeAttributes asserts that BufferFilterRuleSelector correctly compares the Int32 value of an attribute that is already in a rule.

AFAICT, these tests do not verify that, if ILogger.Log is given a state object that includes an Int32 attribute value, then the value is forwarded to BufferFilterRuleSelector as Int32 rather than first converted to String.

I expect that this test will fail because values in IConfiguration are always strings and Microsoft.Extensions.Configuration.Binder cannot know that "1" should be converted to Int32.

It is not failing. Unless you are talking about some different situation?

Because this PR only supports equality comparisons, rather than < and >, I think floating point is out of scope.

Supporting float numbers and their comparison is not in scope of this feature, at least not in the initial release. For now, for comparison purposes we treat both config and log state as key/value pairs of strings

I see: GlobalBufferLoggerBuilderExtensionsTests.AddGlobalBuffer_WithConfiguration_RegistersInDI calls Assert.Equivalent(expectedData, options.CurrentValue.Rules), which tries to convert the actual value to the type of the expected value or vice versa. So the assertion succeeds and the test passes even though expectedData[0].Attributes[1].Value is the integer 1 and options.CurrentValue.Rules[0].Attributes[1].Value is the string "1". But BufferFilterRuleSelector is not so lenient.

Added a custom equality comparer

dotnet-comment-bot · 2025-01-15T15:15:44Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Telemetry	Line	93	91.66 🔻
Microsoft.Extensions.Telemetry	Branch	93	92.95 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.04 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.51 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI	88	89
Microsoft.Extensions.AI.OpenAI	77	78

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=918352&view=codecoverage-tab

dotnet-comment-bot · 2025-01-16T15:38:13Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Caching.Hybrid	Line	86	77.82 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.04 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.51 🔻
Microsoft.Extensions.Telemetry	Line	93	91.85 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI	88	89
Microsoft.Extensions.AI.OpenAI	77	78

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=919881&view=codecoverage-tab

KalleOlaviNiemitalo · 2025-01-16T15:49:13Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferFilterRule.cs

+        Category = categoryName;
+        LogLevel = logLevel;
+        EventId = eventId;
+        Filter = filter ?? ((_, _, _, _) => true);


It might be more efficient to leave Filter as null and not call the filter in that case. I expect that most rules won't have a filter specified and the null check will be well predicted by processors. It would also make rules easier to understand in a debugger as actual filters would stand out better.

src/Libraries/Microsoft.Extensions.Telemetry/LoggerFilterRuleSelector.cs

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferFilterRule.cs

noahfalk · 2025-01-21T03:45:03Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferFilterRule.cs

+    public int? EventId { get; set; }
+
+    /// <inheritdoc/>
+    public Func<string?, LogLevel?, EventId?, IReadOnlyList<KeyValuePair<string, object?>>, bool> Filter { get; }


Are there any callsites where Filter is invoked with a null category string, null log level, or null EventId? If yes thats a little surprising but fine as long as we define what that would mean. If no then those parameter types don't need to be nullable.

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/ILoggingBuffer.cs

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBufferManager.cs

src/Libraries/Microsoft.Extensions.Telemetry/Logging/LoggingEnrichmentExtensions.cs

noahfalk · 2025-01-21T04:21:45Z

test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/AcceptanceTests.cs

+                {
+                    await context.Request.Body.DrainAsync(default);
+
+                    // normally, this would be a Middleware and IHttpRequestBufferManager would be injected via constructor


await next(context).ConfigureAwait(false); }

This is the spot in the middleware where I would expect the majority of logging would occur. Right now it looks like the example code would not buffer that common case logging.

src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBuffer.cs

src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferManager.cs

PR comments

dotnet-comment-bot · 2025-01-21T14:45:38Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.22 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.48 🔻
Microsoft.Extensions.Caching.Hybrid	Line	86	77.82 🔻
Microsoft.Extensions.Telemetry	Line	93	91.91 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.OpenAI	77	78
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=923995&view=codecoverage-tab

tarekgh · 2025-01-21T15:58:36Z

I asked this in one of comments but didn't see answer. Are we going to take these APIs to the design review?

evgenyfedorov2 · 2025-01-21T16:02:22Z

I asked this in one of comments but didn't see answer. Are we going to take these APIs to the design review?

We are going to take it to API review https://apireview.net. I am not aware of design review, is it different than the API review in .NET?

tarekgh · 2025-01-21T17:33:20Z

I am not aware of design review, is it different than the API review in .NET?

No, should be same. You may consult the doc https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md for the details. Thanks!

dotnet-comment-bot · 2025-01-22T09:08:25Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Telemetry	Line	93	91.91 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.36 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.48 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.OpenAI	77	78
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=925013&view=codecoverage-tab

dotnet-comment-bot · 2025-01-22T15:03:10Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Telemetry	Line	93	92 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.04 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.44 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.OpenAI	77	78
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=925328&view=codecoverage-tab

KalleOlaviNiemitalo · 2025-01-22T15:52:32Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/StringifyComprarer.cs

+
+    public int GetHashCode(KeyValuePair<string, object?> obj)
+    {
+        return HashCode.Combine(obj.Key, obj.Value);


This would have to be obj.Value?.ToString(), in order to be consistent with Equals.
Alternatively just throw NotImplementedException, if this is designed to be used only with Enumerable.Contains, which doesn't call GetHashCode anyway.

dotnet-comment-bot · 2025-01-22T16:37:09Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Telemetry	Line	93	91.87 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	96.04 🔻
Microsoft.AspNetCore.Diagnostics.Middleware	Branch	100	98.44 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.OpenAI	77	78
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=925376&view=codecoverage-tab

KalleOlaviNiemitalo · 2025-01-23T12:20:50Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferFilterRule.cs

+namespace Microsoft.Extensions.Diagnostics.Buffering;
+
+/// <summary>
+/// Defines a rule used to filter log messages for purposes of futher buffering.


KalleOlaviNiemitalo · 2025-01-23T12:28:02Z

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/BufferFilterRule.cs

+/// <summary>
+/// Defines a rule used to filter log messages for purposes of futher buffering.
+/// </summary>
+[Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)]
+public class BufferFilterRule


This should have docs about what happens if a log entry matches a rule, and what happens if the log entry does not match any rule.

Is the log entry discarded, buffered, or written directly to underlying loggers?

Does it cause other log entries to be flushed from the buffer to underlying loggers? If so, does that apply to all log categories or only to the same category?

Alternatively, if this type is used with different semantics in different contexts, then document the behaviour in HttpRequestBufferOptions.Rules and GlobalBufferOptions.Rules, and link to those docs from here.

Buffering

988c709

dotnet-policy-service bot assigned evgenyfedorov2 Nov 13, 2024

noahfalk reviewed Nov 23, 2024

View reviewed changes

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch 2 times, most recently from e40b7b6 to 9d61d12 Compare December 12, 2024 16:30

Major update

2f1a335

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from 9d61d12 to 2f1a335 Compare December 12, 2024 17:36

Remove Json exception converter

2d2412e

geeknoid reviewed Dec 15, 2024

View reviewed changes

...Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/IHttpRequestBufferManager.cs Show resolved Hide resolved

geeknoid reviewed Dec 15, 2024

View reviewed changes

...soft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferLoggerBuilderExtensions.cs Outdated Show resolved Hide resolved

geeknoid reviewed Dec 15, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBufferOptions.cs Outdated Show resolved Hide resolved

geeknoid reviewed Dec 15, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBufferOptions.cs Outdated Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Dec 16, 2024

View reviewed changes

src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/HttpRequestBufferManager.cs Outdated Show resolved Hide resolved

evgenyfedorov2 added 2 commits December 16, 2024 14:32

Fix namespaces

f7eaab1

Address PR comments Add .NET 8 support Add ExceptionJsonConverter

Merge branch 'main' into evgenyfedorov2/log_buffering

1f464df

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from 883334d to 022f00c Compare December 16, 2024 14:23

Fix build

a371d9c

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from 022f00c to a371d9c Compare December 16, 2024 14:51

KalleOlaviNiemitalo reviewed Dec 17, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Telemetry/Logging/ExtendedLogger.cs Outdated Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Dec 17, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Telemetry/Buffering/ILoggingBuffer.cs Outdated Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Dec 17, 2024

View reviewed changes

src/Libraries/Microsoft.Extensions.Telemetry/ILoggerFilterRule.cs Outdated Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Dec 17, 2024

View reviewed changes

src/Shared/ExceptionJsonConverter/ExceptionJsonConverter.cs Outdated Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Dec 17, 2024

View reviewed changes

src/Shared/ExceptionJsonConverter/ExceptionJsonConverter.cs Outdated Show resolved Hide resolved

KalleOlaviNiemitalo reviewed Dec 18, 2024

View reviewed changes

Add log record size estimation and limit buffer size in bytes

fe00658

Remove pooling for now

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from fa52316 to fe00658 Compare January 14, 2025 11:03

KalleOlaviNiemitalo reviewed Jan 15, 2025

View reviewed changes

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from 433ede2 to e5905b1 Compare January 16, 2025 09:58

Add filtering by attributes

5fc421c

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from e5905b1 to 5fc421c Compare January 16, 2025 15:05

KalleOlaviNiemitalo reviewed Jan 16, 2025

View reviewed changes

evgenyfedorov2 added the area-telemetry label Jan 17, 2025

noahfalk reviewed Jan 21, 2025

View reviewed changes

evgenyfedorov2 added 2 commits January 21, 2025 11:26

Use attributes directly instead of Func delegate

70cfc7c

Add http buffer holder

e96277f

PR comments

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from 8b8558a to e96277f Compare January 21, 2025 14:11

Make ILoggingBuffer and DeserializedLogRecord types internal

a79fcbf

Move shared files to Shared project and add more tests

8a91c15

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from 8319dc8 to 8a91c15 Compare January 22, 2025 14:28

KalleOlaviNiemitalo reviewed Jan 22, 2025

View reviewed changes

Add custom equality comparer

4f524eb

evgenyfedorov2 force-pushed the evgenyfedorov2/log_buffering branch from d387358 to 4f524eb Compare January 22, 2025 16:03

KalleOlaviNiemitalo mentioned this pull request Jan 23, 2025

Commiting batch of log messages conditionally. dotnet/runtime#38045

Open

KalleOlaviNiemitalo reviewed Jan 23, 2025

View reviewed changes

Add logging buffering #5635

Are you sure you want to change the base?

Add logging buffering #5635

Conversation

evgenyfedorov2 commented Nov 13, 2024 • edited by dotnet-policy-service bot Loading

Microsoft Reviewers: Open in CodeFlow

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnet-comment-bot commented Dec 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenyfedorov2 Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnet-comment-bot commented Jan 14, 2025

Choose a reason for hiding this comment

evgenyfedorov2 Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KalleOlaviNiemitalo Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnet-comment-bot commented Jan 15, 2025

dotnet-comment-bot commented Jan 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnet-comment-bot commented Jan 21, 2025

tarekgh commented Jan 21, 2025

evgenyfedorov2 commented Jan 21, 2025

tarekgh commented Jan 21, 2025

dotnet-comment-bot commented Jan 22, 2025

dotnet-comment-bot commented Jan 22, 2025

KalleOlaviNiemitalo Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

dotnet-comment-bot commented Jan 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenyfedorov2 commented Nov 13, 2024 •

edited by dotnet-policy-service bot

Loading

evgenyfedorov2 Jan 9, 2025 •

edited

Loading

evgenyfedorov2 Jan 16, 2025 •

edited

Loading

KalleOlaviNiemitalo Jan 22, 2025 •

edited

Loading

KalleOlaviNiemitalo Jan 22, 2025 •

edited

Loading