Log whether the localization resource is found or not

[hishamco]

Fixes #43109

[mkArtakMSFT]

@halter73 can you please review this? Thanks!

[halter73]

Should we provide a different log for a missing manifest vs a non-localized resource where the name == value?

@DamianEdwards do you have an opinion on this?

[DamianEdwards]

No real opinion, I'd defer to folks who've been using it for real to inform us to how useful that would be.

[hishamco]

IMHO we should log for both

[halter73]

I agree we should log in both cases. I was asking if there should be a different log message for the two places this PR currently calls Log.ResourceNotFound since the resource was not found for slightly different reasons, but I'm fine with using the same message whenever a resource is not found for any reason like the PR does currently.

[hishamco]

Or we could change the log level for missing manifest to trace so in debug the missing resources will be logged which are useful in almost cases

[halter73]

I'd just leave them exactly the same for now. If we were going to change the log level, we'd want to give it a unique event id and log message. Changing the level of a log but nothing else would be confusing.

[halter73]

Given that the SearchedLocation log exists, I wonder if we need a ResourceFound log. A lack of a ResourceNotFound log should imply that it was found. This would also mean less debug log noise for a fully localized application which seems like a good thing.

[halter73]

Suggested change

[LoggerMessage(3, LogLevel.Debug, $"The resource with key '{{Key}}' is found.", EventName = "ResourceFound")]

Edit: I'd rather we remove this log altogether.

[hishamco]

So, you will the non found resources?

[halter73]

I'm not sure I understand your question, but I'm asking that we only add the "ResourceNotFound" log not the "ResourceFound" log.

[hishamco]

Forgot my above comment seems I miss some words during the writing :)

[dotnet-policy-service]

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

[hishamco]

@halter73 what's the status PR? is there anything to add to merge it ASAP

[halter73]

Yes. I would like to remove the "ResourceFound" logs and pass in the "Culture" parameter to Log.ResourceNotFound(_logger, name, keyCulture);. Not doing so is currently causing build errors now that the log message includes {{Culture}} in it.

[halter73]

@hishamco Do you think you can fix the build and test failures? We should also add a test case that checks the logs for $"A resource for '{{Key}}' with culture '{{Culture}}' was not found.".