Support GELF logging output. #497
Conversation
jbgi
force-pushed
the
slog-gelf
branch
2 times, most recently
from
bb1a33f
to
34497a6
Compare
|
I've rebased in my fork (disassembler) but have no way to push to this PR. |
jbgi
force-pushed
the
slog-gelf
branch
3 times, most recently
from
6b683d7
to
94855bb
Compare
jbgi
force-pushed
the
slog-gelf
branch
2 times, most recently
from
b6e4f33
to
d48a826
Compare
|
This UUID will be generated every time at start up. Is this what you really want? How will the user be aware of the UUID to give to get support? What about re-using the same UUID? Since this is a feature Devops wishes to include, why not have it set as parameter when starting GELF logging and have it generated by the devops script side of things? |
|
I would provide the GELF logging id as an option in the config.yaml option map for GELF. |
|
Yes, I was thinking of adding an |
jbgi
force-pushed
the
slog-gelf
branch
2 times, most recently
from
b11a512
to
b36d16a
Compare
jbgi
force-pushed
the
slog-gelf
branch
3 times, most recently
from
f04a932
to
7e5fabd
Compare
100: Add jormungandrMaster for non-released build of jormungandr r=jbgi a=jbgi currently pointing at this PR: input-output-hk/jormungandr#497 Co-authored-by: Jean-Baptiste Giraudeau <[email protected]>
|
should be mergeable now. |
|
I think this is looking good, but it's missing a configurability option to make the gelf support optional |
| @@ -24,6 +24,8 @@ pub struct ConfigLogSettings { | |||
| pub verbosity: Option<u8>, | |||
| pub format: Option<LogFormat>, | |||
| pub output: Option<LogOutput>, | |||
| pub backend: Option<String>, | |||
There was a problem hiding this comment.
that might be better as an Enum related to gelf instead of a random string.
| @@ -24,6 +24,8 @@ pub struct ConfigLogSettings { | |||
| pub verbosity: Option<u8>, | |||
| pub format: Option<LogFormat>, | |||
| pub output: Option<LogOutput>, | |||
| pub backend: Option<String>, | |||
| pub logs_id: Option<String>, | |||
| @@ -28,6 +31,7 @@ pub enum LogFormat { | |||
| /// Output of the logger. | |||
| pub enum LogOutput { | |||
| Stderr, | |||
| Gelf, | |||
There was a problem hiding this comment.
seems that if you make this Gelf(String, String) you same yourself the trouble of extending the generic structure for something specific to the backend
There was a problem hiding this comment.
The structopt/YAML thing would need to be customized for this, I believe.
This whole enum needs love, but I can improve it later.
There was a problem hiding this comment.
yeah, I was not sure how the yaml parsing would workout if I did that. I can try.
There was a problem hiding this comment.
ok nevermind, if it's too complicated we can push this through and revisit later
|
@vincenthz should we expect @jbgi to do the right |
@vincenthz so a feature like for journald support? |
|
I don't mind too much on the order, it should be easy to cfg() is out, so that it doesn't impose some transitient unseen burden |
Closes #447.