-
Notifications
You must be signed in to change notification settings - Fork 419
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
Add log.origin fields #563
Changes from 2 commits
89c1ac8
3d0c917
60b1ee1
ea47d83
75c32b4
f2a19d3
1ec53dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,3 +43,28 @@ | |
short: Name of the logger. | ||
description: > | ||
The name of the logger inside an application. This is usually the name of the class which initialized the logger, or can be a custom name. | ||
|
||
- name: source.file | ||
level: core | ||
felixbarny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
type: keyword | ||
example: Bootstrap.java | ||
short: The file which originated the log event. | ||
description: > | ||
The name of the source file which originated the log event. | ||
|
||
- name: source.function | ||
level: core | ||
type: keyword | ||
example: init | ||
short: The function which originated the log event. | ||
description: > | ||
The name of the function or method which originated the log event. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity: If it's a method, would we expect it to also indicate what class it belongs to? Something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it would just be the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, SGTM. Lets only add it if we already have a use case for it. But There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We actually do have a use case for logging. For java logging libraries, we usually get both the file name and the class name of the origin. See also https://docs.oracle.com/javase/7/docs/api/java/lang/StackTraceElement.html#getClassName(). I didn't add it at first because not all languages have the concept of a class. But when it's optional, why not? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I could name quite a few that have a similar concept ;-) In the case of Golang I would also use +1 on adding class directly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :) I'm fine with class as a name, but I would have made it a qualified name like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a reason why the class name should be included We can definitely have an additional field for the class ( I'd like to defer these potentially controversial discussions and get the lowest common denominator in which, I think, is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the description it mentions, it can be a function or method. So if it is a method, would it be wrong to put here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would seem a bit weird to me as the method name is foo, not Bootstrap.foo. https://docs.oracle.com/javase/7/docs/api/java/lang/StackTraceElement.html#getMethodName() would also just return the method name. What would be the benefit of concatenating the simple class name and the method name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @felixbarny I'm not too fussed if this is deferred. I don't like to sound like a broken record, but the use case I have in mind is once again profiling, and I was hoping we could find a common definition of fields. I'd say the benefit of having them combined is having a simple display name, so that UIs don't need to know how to recombine the class and method name. e.g. it's "Class::Method" in C++. |
||
|
||
- name: source.line | ||
level: core | ||
type: integer | ||
example: 42 | ||
short: The line number of the file which originated the log event. | ||
description: > | ||
The line number of the file which originated the log event. | ||
|
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 worry if what we call source here will conflict with what we have in source under https://github.com/elastic/ecs/blob/master/schemas/source.yml At the same time I think some argument could be it is similar?
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'd say it's actually not really similar to
source
. We could also rename tolog.origin.*
to avoid ambiguities.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.
+1 on origin.