Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7.0.0 Release #223

Merged
merged 32 commits into from
May 10, 2023
Merged

7.0.0 Release #223

merged 32 commits into from
May 10, 2023

Conversation

nblumhardt
Copy link
Member

@nblumhardt nblumhardt commented May 5, 2023

nblumhardt and others added 30 commits November 2, 2021 07:38
Before this commit:
`LogLevel.None` is converted to `LogEventLevel.Fatal` and events with a `None` level are logged as fatal.

After this commit:
`LogLevel.None` is observed and logs  and events with a `None` level are ignored.

The `None` level is documented as such:
> Not used for writing log messages. Specifies that a logging category should not write any messages.

Note: this erroneous behaviour was seen in a real-world scenario:

1. `BuildOrgConnectUri CoreClass ()` is [logged as `TraceEventType.Start `][1]
2. `TraceEventType.Start` is [converted to `LogLevel.None`][2]
3. `LogLevel.None ` is [converted to `LogEventLevel.Fatal`][3]

As a result, `BuildOrgConnectUri CoreClass ()` is logged as fatal whereas it should have been ignored.

[1]: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/blob/0.6.1/src/GeneralTools/DataverseClient/Client/ConnectionService.cs#L3207
[2]: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/blob/0.6.1/src/GeneralTools/DataverseClient/Client/DataverseTraceLogger.cs#L675
[3]: https://github.com/serilog/serilog-extensions-logging/blob/dev/src/Serilog.Extensions.Logging/Extensions/Logging/LevelConvert.cs#L39
Observe Microsoft.Extensions.Logging.LogLevel.None
Eliminates boxing of Dictionary<TKey, TValue>.Enumerator for the most common use case
 Migrate to NET7 and C#11
# Conflicts:
#	src/Serilog.Extensions.Logging/Extensions/Logging/SerilogLogger.cs
#	src/Serilog.Extensions.Logging/Extensions/Logging/SerilogLoggerScope.cs
Cache trimmed keys for @ and $
Set default categoryName
Do not swallow exception from Audit
… Serilog v2.12 and apply annotations for nullable reference types
@nblumhardt nblumhardt marked this pull request as ready for review May 10, 2023 04:52
@nblumhardt
Copy link
Member Author

This one's ready to go.

@sungam3r
Copy link
Contributor

Wait for #219 ?

@nblumhardt
Copy link
Member Author

Given the difficulty of getting everything into shape and out at the same time, I don't think we should hold up the release. There's always a 7.0.1, etc., to ship another day :-)

@niemyjski
Copy link

niemyjski commented May 10, 2023

@nblumhardt any reason to pin to v7 instead of lts v6?

@nblumhardt
Copy link
Member Author

@niemyjski just practicalities, really. Hopefully most people who really need LTS versions of everything will already be managing dependency upgrades carefully and can stick with whatever they're using today. Otherwise, v7 of these packages all include .NET 6 targets anyway.

There's also a new LTS just around the corner with v8, we've already published a Serilog.AspNetCore v6, and we don't want to have to coordinate shipping two new releases just to get the versioning policy bootstrapped. Did give it all lots of thought; always trade-offs whatever we do :-)

@nblumhardt nblumhardt merged commit 710777b into main May 10, 2023
@sungam3r
Copy link
Contributor

Do you intentionally not reference serilog v3 here and in other repos with such v7 releases?

@nblumhardt
Copy link
Member Author

@sungam3r thanks for the double-check; yes, I think we still have some work to go on Serilog v3 👍

I'm not sure at which stage we'll want to flip over the dependencies here (and in the other projects), if Serilog v3 is out within a few months, the v8 packages might pull it in, perhaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants