Closed dozer75 closed 6 hours ago
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
Why is the addition of EventID
a problem for you? It is a good observability feature as it gives you a way to identify multiple log events and link them back to the same event in code.
And why does it create insane numbers is not specified which spams various logging systems?
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.
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.
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.
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?
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.
@julealgon
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:
Why is it autogenerated if not specified?
Why is there a analyzer warning for this when working with the codegen loggers, but not when working with the ILogger
methods?
Event ID values must be unique within the scope of each assembly.
.. Well, that is incorrect, there is no requirement for that in any way anywhere.. As mentioned in initial post, just look at event ids for e.g. ASP.NET core Hosting.Lifetime loggings. There are the same for several log messages within the same library and this is quite normal, you may want to use event ids to group logging. It's all depends on how you implement logging rules with the environment available.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.
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.
@dozer75
- Why is it autogenerated if not specified?
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 EventID
without the maintenance of manually keeping them and ensuring they are unique.
- The ILogger.LogXXX extensions uses 0 as default if not specified, here it is "randomly" generated with a number that changes whenever the code changes, you can't set up observability for randomly generated event ids.
This is incorrect. The ID will remain consistent after it is defined initially.
- A "random" event id will in fact clutter up the system if you have observability rules to detect missing event ids (it's easier to monitor for one single value (0) rather than "all values that doesn't correspond to your rules".
I don't understand what this means. Why would you "monitor for a value 0 everywhere"?
- Why is there a analyzer warning for this when working with the codegen loggers, but not when working with the
ILogger
methods?
No idea on this one.
- The same analyzer rules should apply no matter how you're doing logging if that is that important...
This is arguable, I guess. I don't have that strong of a position on it.
- In addition, the warning states
Event ID values must be unique within the scope of each assembly.
.. Well, that is incorrect, there is no requirement for that in any way anywhere.. As mentioned in initial post, just look at event ids for e.g. ASP.NET core Hosting.Lifetime loggings. There are the same for several log messages within the same library and this is quite normal, you may want to use event ids to group logging. It's all depends on how you implement logging rules with the environment available.- The warning is actually enforced only when eventid is set to 0, not for duplicate event ids, even though it states something else in the documentation.
You might be right here. Maybe someone on the team will address 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.
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.
I've already pointed out that the underlying value for EventID
does not in fact change every time there are code changes. So the basis for your disagreement does not stand.
To me, having a unique, consistent EventID
for every different log "template" is a pretty solid standard that the generator guarantees by default. I guess the definition of what a "solid standard" means is not super objective, so to each their own.
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".
Again, you may be correct on this one. I'm also curious to hear what the team has to say on it.
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.
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 EventID
generation, as long as it was opt-in. Perhaps that would be enough for your scenario.
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 SYSLIB1006
is a specific message generated by the source generator, not a generic logging analyzer. While we can consider creating an analyzer for non-generated code if there's sufficient demand or if it would benefit a wide range of users, we are open to suggestions that can improve the user experience.
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.
Codegen logging should work the same way as not code-generated logging for consistency. The only thing they should aid is to create an efficient logging alternative to default logging.
Autogenerating EventIDs does in my opinion not offer any real value because those that uses Event IDs will set them explicit anyway to create a good structure around events. Those who don't set it doesn't really care anyway and doesn't use the EventID at all.
It's better to have code analyzers for this to give developers the possibility to get warned instead of autogenerate values and then create their own rules around these code analyzers (having them as errors, warnings, suggestion or suppressing them as other code analyzers). In addition, code analyzers can be applied to both codegen and ordinary logging.
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.
Codegen logging should work the same way as not code-generated logging for consistency. The only thing they should aid is to create an efficient logging alternative to default logging.
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.
Autogenerating EventIDs does in my opinion not offer any real value because those that uses Event IDs will set them explicit anyway to create a good structure around events.
I respect your opinion, but we have already got some positive feedback. IIRC came from OpenTelemetry.
It's better to have code analyzers for this to give developers the possibility to get warned instead of autogenerate values
I have started before the benefit of having the EventId auto generation. You are the first one we see complaining about.
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.
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.
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.
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?