-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Events in DocumentedObservation #3367
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that this can be null here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I mean look at line 60, we literally null the name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get that a DocumentedObservation's getName() can return null for filtering purposes but if I take a step back, that might be valid for the DocumentedObservation but I think SimpleObservation should not accept null here. What should the meter handler or the tracing handler do in that case?
Also, DocumentedObservation has a null-check so that nullability behavior can be isolated there I guess.
micrometer-commons/src/main/java/io/micrometer/common/Event.java
Outdated
Show resolved
Hide resolved
micrometer-commons/src/main/java/io/micrometer/common/Event.java
Outdated
Show resolved
Hide resolved
micrometer-commons/src/main/java/io/micrometer/common/Event.java
Outdated
Show resolved
Hide resolved
bc9caf5 to
e358cd5
Compare
|
@cppwfs this might break task cause afair you're using events already. You'll need to change |
No description provided.