-
Notifications
You must be signed in to change notification settings - Fork 74
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
Fixes #21784: add a default value for syslog facility output for agent and server compontent #4486
Conversation
@@ -8,7 +8,8 @@ | |||
}, | |||
"server": { | |||
"cf_serverd_bind_address": "::" | |||
} | |||
}, | |||
"syslog_facility": "LOG_LOCAL3" |
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.
"syslog_facility": "LOG_LOCAL3" | |
"agent_syslog_facility": "LOG_LOCAL3" |
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.
IIRC it is only a setting for the agent
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.
if it's for agent only, it should be "agent": { "syslog_facility" : "LOG_LOCAL3" }
If it's for both, it seems worthy to have a "log" category, it seems common enough to be useful. I'm not sure why but I would prefer to minimize number of top level props.
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.
it's for both cf-agent and cf-serverd
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.
so, for now we will put it in log, and we will try to split / make it more granular if we want/can in further releases:
{ ...
"log": { "syslog_facility" : "LOG_LOCAL3" }
...
}
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.
Pr updated
PR updated with a new commit |
This PR is not mergeable to upper versions. |
OK, squash merging this PR |
…t and server compontent
47585c5
to
f3196fd
Compare
https://issues.rudder.io/issues/21784