-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
LoggerMessageAttribute and EventID inconsistency with non-code generated logging. #110021
Comments
Why is the addition of
This confuses me. What do you mean by "insane" here? They are just some number that is automatically generated based on the text of the log message template, so that each generated log method has a distinct value that can be strongly linked back to the code later. What does "spams various logging systems" mean exactly? This is a good attribute to have, particularly if you have an observability platform which allows you to query based on its values. If you really don't want to persist this value at all, you should be able to configure this in your tooling. For instance, if you were using an OpenTelemetry collector, you can strip the attribute using a transform at the collector level.
Strongly disagree. The fact that the generator creates a unique number per generated method is awesome behavior that allows you to have a strong identifier linked to each "class" of log with zero effort.
No idea why anyone would want to do that. For people reading the issue, I would again strongly advise not following this suggestion/workaround as you would just be fighting against a solid standard. |
Just to be clear: It is NOT the event id logic that I'm questioning and the need of that, that has been in the logging functionality since it's beginning and it has it uses in many circumstances, like e.g. grouping messages and sending them to various destinations depending on the event id number. BUT, what I am questioning here is:
The enforced event id rules that the code-generated library forces with text-based generated event id's is not a "solid" standard, that is as useless as 0 as event id when it comes to observability since code may change. And the analyzer recommendation around unique event ids is neither a standard around event ids either. What I think is important is to have the same rules no matter how you're doing something that in the end gives the same result, and again enforcing something using randomly values isn't really a "solid standard". IMO, it is better to have the standard as it has been before when not using source-gen code, and rather create code analyzers that gives suggestion to avoid unique (or 0) event ids. |
Because that's a feature of the generator. Usually, you are supposed to keep track of these unique IDs yourself (if you want to use EventIDs). The fact that the generator creates them behind the scenes for you is a benefit of the generator that allows you to get the benefits of the
This is incorrect. The ID will remain consistent after it is defined initially.
I don't understand what this means. Why would you "monitor for a value 0 everywhere"?
No idea on this one.
This is arguable, I guess. I don't have that strong of a position on it.
You might be right here. Maybe someone on the team will address that.
I've already pointed out that the underlying value for To me, having a unique, consistent
Again, you may be correct on this one. I'm also curious to hear what the team has to say on it.
You are advocating for removing a pretty good feature of the generator then. I would not like to lose that feature myself, since keeping track of unique event ids on the application side is a nightmare. I would not be opposed, however, if there was some sort of toggle that would disable |
The auto-generation of EventId in the Logger source generator is by design. This feature was implemented to improve log aggregation by automatically generating unique identifiers. Using EventId helps distinguish different logging events and facilitates aggregation when needed. The auto-generated EventId eliminates the need for users to manually manage potential duplication or missing IDs. The warning Assigning the same EventId to all log messages undermines the purpose of having EventIds in the first place. If you wish to control the EventId, set it to values that make sense for your context, ensuring the uniqueness of each EventId for each log message (for the sake of the aggregator platforms). If EventId is not important to you, feel free to ignore it, as it won’t matter whether it's auto-generated or not. Please, let us know what the exact ask and we'll be happy to look at that. Is introduce analyzer for non-generated code? or you have other asks? |
I don't have any real question except those I initially asked. But my opinion in this matter around the codegen autogenerated event ids is the following.
It's quite interesting that you states that it undermines the purpose of using same event ids across log messages, since .NET libraries does exactly this using same event ids across , so they break what you are saying here... But anyway, I don't feel to try to argue for having it autogenerated anymore. I have stated my opinion a few times now, but it is a decision made that I feel you are reluctant to change, so we have to accept it obviously. |
In theory you are correct, but this is not the case with most code generators. Code generators are kind of new features and always come with some restriction and change in behavior in some cases.
I respect your opinion, but we have already got some positive feedback. IIRC came from OpenTelemetry.
I have started before the benefit of having the EventId auto generation. You are the first one we see complaining about.
Good point but we always do even harder breaking change in the platform when we see something was done wrong in the first place. It is not good to stick with undesired behavior when you have a chance to improve it. That is what happened here.
We appreciate your feedback. Decisions made around this area were not made lightly. It was done after detailed discussions. |
I was working with logging today and using code generated log methods using the
LoggerMessageAttribute
. After starting using that I realized that the Event ID is set randomly if it isn't set explicit, so I set all to 0 since we're not using Event ID, but then I get the [SYSLIB1006] (https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/syslib1006) message which states that Event IDs should be unique within the scope of each assembly.So to the questions:
Why is Event ID required to be unique across the assembly when using code generated logging? It isn't any requirements for that when not using code generated logging (
ILogger.Log
)? And why does it create insane numbers is not specified which spams various logging systems?Using non-code generated logging does not behave like this, and according to the logging in various Microsoft provided packages it's not set according to the
SYSLIB1006
either (just look at the startup of a ASP.NET Core or Aspire project types).I think that code generated logging should behave the same way as non-code generated logging; namely having the Event ID set to 0 if not specified to have consistency in the logging logic.
Of course we can disable the warning and add 0 to all
LoggerMessageAttribute
but it feels kind of unnecessary taken in consideration that standard logging doesn't have any requirement for this?The text was updated successfully, but these errors were encountered: