Skip to content

Update log scope to be more compatible with JSON console logger.#1905

Merged
jander-msft merged 1 commit into
dotnet:masterfrom
jander-msft:dev/jander/jsonlogscope
Jan 14, 2021
Merged

Update log scope to be more compatible with JSON console logger.#1905
jander-msft merged 1 commit into
dotnet:masterfrom
jander-msft:dev/jander/jsonlogscope

Conversation

@jander-msft

Copy link
Copy Markdown
Contributor

The JSON console logger checks if the log scope implements IReadOnlyCollection<KeyValuePair<string, object>> and then falls back to calling ToString. Add the IReadOnlyCollection<KeyValuePair<string, object>> implementation to better suit the formatted output.

@jander-msft jander-msft added enhancement New feature or request dotnet-monitor labels Jan 14, 2021
@jander-msft jander-msft self-assigned this Jan 14, 2021
@jander-msft jander-msft requested a review from a team as a code owner January 14, 2021 05:26
@shirhatti

Copy link
Copy Markdown
Contributor

@maryamariyan Do you know why the JSON logger requires IReadOnlyCollection<KeyValuePair<string, object>> for scope data? Looking at other loggers, they are perfectly content with IEnumerable<KeyValuePair<string, object>>? If this wasn't intentional, we should fix this in the logger.

@jander-msft

Copy link
Copy Markdown
Contributor Author

@maryamariyan Do you know why the JSON logger requires IReadOnlyCollection<KeyValuePair<string, object>> for scope data? Looking at other loggers, they are perfectly content with IEnumerable<KeyValuePair<string, object>>? If this wasn't intentional, we should fix this in the logger.

FYI, here's the code checking for IReadOnlyCollection<>: https://github.com/dotnet/runtime/blob/f9a8abc8a5d9981e25f898181e734dc77c7e15e4/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatter.cs#L112

@jander-msft jander-msft merged commit 5295d9c into dotnet:master Jan 14, 2021
@jander-msft jander-msft deleted the dev/jander/jsonlogscope branch January 14, 2021 20:49
@maryamariyan

Copy link
Copy Markdown

@maryamariyan Do you know why the JSON logger requires IReadOnlyCollection<KeyValuePair<string, object>> for scope data? Looking at other loggers, they are perfectly content with IEnumerable<KeyValuePair<string, object>>? If this wasn't intentional, we should fix this in the logger.

I don't think there was a specific reason for having IReadOnlyCollection as opposed to IEnumerable. I'll investigate to find out. @shirhatti could you please open an issue on dotnet/runtime to track this?

@shirhatti

Copy link
Copy Markdown
Contributor

Thanks! I just filed an issue

@github-actions github-actions Bot locked and limited conversation to collaborators Jan 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

dotnet-monitor enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants