Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

EventID's should be unique per-category #6062

Closed
ryanbrandenburg opened this issue Apr 3, 2017 · 9 comments
Closed

EventID's should be unique per-category #6062

ryanbrandenburg opened this issue Apr 3, 2017 · 9 comments
Assignees

Comments

@ryanbrandenburg
Copy link
Contributor

ryanbrandenburg commented Apr 3, 2017

According to @BrennanConroy eventId's of LoggerMessage.Define should be unique per category (usually the T in ILogger<T>). There's some spots where that's not true and we should fix it. Example of non-uniqueness here.

@rynowak
Copy link
Member

rynowak commented Apr 3, 2017

Our eventIds are unique per-logger. Where is this new requirement coming from?

@Eilon
Copy link
Member

Eilon commented Apr 11, 2017

Yeah this is news to me. The uniqueness is (category + eventid), not just (eventid).

@ryanbrandenburg
Copy link
Contributor Author

@BrennanConroy did I misunderstand the requirements here? And @Eilon what "category" are you talking about?

@Eilon
Copy link
Member

Eilon commented Apr 11, 2017

The category is usually the type name we pass into ILogger<T>. The category names are arbitrary strings, but we pretty much always pass in the type name of the type doing the logging.

@ryanbrandenburg
Copy link
Contributor Author

In that case isn't this and this as called here and here respectively an example of us not following (category + eventid) uniqueness since they'll use the same _logger?

@rynowak
Copy link
Member

rynowak commented Apr 11, 2017

If we have a bug we should fix it.

But this statement has never been a practice we have followed:

EventId's of LoggerMessage.Define should be unique within thier package. This is certainly not the case right now, so we should go through and make them all so.

@Eilon
Copy link
Member

Eilon commented Apr 11, 2017

@ryanbrandenburg that does appear to be a specific bug in that specific case.

@ryanbrandenburg ryanbrandenburg changed the title EventID's should be unique per-package EventID's should be unique per-category Apr 11, 2017
@ryanbrandenburg
Copy link
Contributor Author

Updated to reflect category vs package.

@Eilon
Copy link
Member

Eilon commented Apr 12, 2017

Ok let's fix this one offending one. I can't think of a good way to easily detect this because there's no way to tell from the various LoggerExtension methods where the log will be used, and thus what the category is.

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

No branches or pull requests

4 participants