Skip to content

Logging middleware#87550

Closed
noahfalk wants to merge 27 commits into
dotnet:mainfrom
noahfalk:logging-middleware
Closed

Logging middleware#87550
noahfalk wants to merge 27 commits into
dotnet:mainfrom
noahfalk:logging-middleware

Conversation

@noahfalk
Copy link
Copy Markdown
Member

@noahfalk noahfalk commented Jun 14, 2023

Work in progress, taking back over from JamesNK who was previously working here.

Tasks:

  • API - [CUT] We need a proposed API design for using enrichment. This includes a producer side (the developers who are populating the enrichment properties) and a consumer side (the developers writing ILoggerProviders that log the properties)
  • API - We need a proposed API design for configuring the middleware pipeline
  • API - ProcessorFactory. Replace with a func?
  • API - BufferWriter. Use IBufferWriter instead? Add to dotnet/runtime in neutral location? Rename to logger specific API?
  • API - Pipeline, LogEntryPipeline, ScopePipeline. Make internal. This is difficult because they're used in Extensions.Logging and Extensions.Logging.Abstractions.
  • API - ILogEntryPipelineFactory. Make internal.
  • API - PropertyCustomFormatter, IPropertyFormatterFactory - Delegates vs virtual methods for property factory
  • API - ProcessorContext. Is cancellation needed here?
  • API - IProcessorFactory, ILogEntryProcessorFactory. Merge two processor factories?
  • API - ILogMetadata. Members on this interface need review.
  • API - handle dynamic properties
  • API - create default metadata for unknown TStates
  • Perf - [CUT] Add enrichment perf tests to https://github.com/dotnet/performance
  • Perf - Add redaction perf tests to https://github.com/dotnet/performance
  • Functionality - Processors are registered via type to DI container. This means it isn't possible to have multiple processors of the same type in a pipeline. Is that an acceptable limitation?

cc @noahfalk @tarekgh

JamesNK and others added 24 commits May 30, 2023 14:51
- Refactored to remove BufferWriter
- Extracted all formatting work outside of ILogMetadata to extension methods.
- Added a new CreatePropertyListVisitor as the primitive all high performance
serialization can be layered over.
- Removed all the virtual override based formatting logic and converged
on IPropertyVisitorFactory to produce delegates.
- Continued refactoring away from LogValuesFormatter towards ILogMetadata.
- Replace all usage of the formatter with ILogMetadata
@ghost
Copy link
Copy Markdown

ghost commented Jun 14, 2023

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link
Copy Markdown

ghost commented Jun 14, 2023

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

Work in progress, taking back over from JamesNK who was previously working here.

Tasks:

  • API - We need a proposed API design for using enrichment. This includes a producer side (the developers who are populating the enrichment properties) and a consumer side (the developers writing ILoggerProviders that log the properties)
  • API - We need a proposed API design for configuring the middleware pipeline
  • API - BufferWriter. Use IBufferWriter instead? Add to dotnet/runtime in neutral location? Rename to logger specific API?
  • API (Noah working on it) - Pipeline, LogEntryPipeline, ScopePipeline. Make internal. This is difficult because they're used in Extensions.Logging and Extensions.Logging.Abstractions.
  • API (Noah working on it) - ILogEntryPipelineFactory. Make internal.
    https://github.com/JamesNK/runtime/pull/1/files
  • API - PropertyCustomFormatter, IPropertyFormatterFactory - Delegates vs virtual methods for property factory
  • API - ProcessorContext. Is cancellation needed here?
  • API - ProcessorFactory. Replace with a func?
  • API - IProcessorFactory, ILogEntryProcessorFactory. Merge two processor factories?
  • API - ILogMetadata. Members on this interface need review.
  • Perf - Add enrichment perf tests to https://github.com/dotnet/performance
  • Perf - Add redaction perf tests to https://github.com/dotnet/performance
  • Functionality - Processors are registered via type to DI container. This means it isn't possible to have multiple processors of the same type in a pipeline. Is that an acceptable limitation?

cc @noahfalk @tarekgh

Author: noahfalk
Assignees: noahfalk
Labels:

new-api-needs-documentation, area-Extensions-Logging

Milestone: -

@tarekgh
Copy link
Copy Markdown
Member

tarekgh commented Jun 14, 2023

can we close #86995 now?

@noahfalk
Copy link
Copy Markdown
Member Author

can we close #86995 now?

done

@noahfalk
Copy link
Copy Markdown
Member Author

Decided not to pursue this in .NET 8

@noahfalk noahfalk closed this Jul 10, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants