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

Use a different identifier for the Android tag instead of SourceContextPropertyName #32

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

busec0
Copy link

@busec0 busec0 commented Aug 29, 2023

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bug fix, docs update

What is the current behavior? (You can also link to an open issue here)

The SourceContextPropertyName can be overwritten in some situations so the defined Tag, as described in the docs, won't work.

  1. Add the Android Logging as designed
.WriteTo.AndroidLog() 
.Enrich.WithProperty(Constants.SourceContextPropertyName, "MyAndroidTag")
  1. Use in combination with Microsoft's Dependency Injection
  2. Register an Microsoft.Extensions.Logging.ILogger implementation
services.AddSingleton<Microsoft.Extensions.Logging.ILogger>
    (provider => provider.GetRequiredService<Microsoft.Extensions.Logging.ILogger<object>>());
  1. Notice that the SourceContextPropertyName will be "overwritten" and it'll hold "object" as value.

Snippet for my registration style to repro this:

IServiceCollection services = new ServiceCollection();
services.AddLogging(r =>
{
    r.AddSerilog(logger);
});
services.AddSingleton<Microsoft.Extensions.Logging.ILogger>(provider => provider.GetRequiredService<Microsoft.Extensions.Logging.ILogger<object>>());

What is the new behavior (if this is a feature change)?

The constant used to identify the Android TAG is changed. It is introduced as part of Serilog.Sinks.Xamarin.Constants

Does this PR introduce a breaking change?

YES!

Users updating will need to add/update the enrich property for tag to be Constants.AndroidTagPropertyName

Please check if the PR fulfills these requirements

Other information:

@nblumhardt
Copy link
Member

Thanks for sending this; unfortunately there's limited maintenance bandwidth for this repository, and in light of Xamarin being EOL'd no further releases are planned here. Sorry about the slow response!

@nblumhardt nblumhardt closed this Jun 6, 2024
@busec0
Copy link
Author

busec0 commented Jun 6, 2024

Xamarin is EOL, but this package targets .NET 6 as well, meaning it can be used with .NET MAUI, Xamarin's successor -> which is actively maintained.

<TargetFrameworks>Xamarin.iOS10;Xamarin.Mac20;MonoAndroid90;net6.0-ios;net6.0-macos;net6.0-android;net6.0-maccatalyst</TargetFrameworks>

@nblumhardt
Copy link
Member

Thanks for your reply. I think I need some help making sense of the Xamarin/MAUI/.NET landscape to figure out what needs to be done here.

For example, when I look up Android.Util.Log (which the Android version of the sink appears to use) I see "Mock Log implementation for testing on non android (sic) host":

https://learn.microsoft.com/en-us/dotnet/api/android.util.log?view=net-android-34.0#remarks

Guessing I've found the wrong docs?

In any case, "Xamarin" naming isn't going to be the best thing for this code in the future; is the right course of action to split this repository into Serilog.Sinks.Android and Serilog.Sinks.iOS, or something else along those lines? (@nickrandolph?)

Thanks for bearing with me :-)

@nblumhardt nblumhardt reopened this Jun 6, 2024
This was referenced Jun 6, 2024
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.

2 participants