Skip to content

Option to disable logging of log producer (2nd try) #1400

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

Closed
wants to merge 4 commits into from
Closed

Option to disable logging of log producer (2nd try) #1400

wants to merge 4 commits into from

Conversation

marcstern
Copy link

[Issue #1069 - Marc Stern]

@zimmerle zimmerle self-assigned this May 2, 2017
CHANGES Outdated
@@ -1,6 +1,9 @@
DD MMM YYYY - 2.9.2 - To be released
------------------------------------

* {dis|en}able-dechunk-logging: Option to disable logging of
dechunking in audit log when log level < 9.
[Issue #1068 - Marc Stern]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue number is not correct.

CHANGES Outdated
@@ -1,6 +1,9 @@
DD MMM YYYY - 2.9.2 - To be released
------------------------------------

* {dis|en}able-dechunk-logging: Option to disable logging of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is about the producer, not the dechunking :P

@@ -1174,6 +1174,9 @@ void sec_audit_logger_json(modsec_rec *msr) {
yajl_kv_bool(g, "response_body_dechunked", 1);
}

#ifdef LOG_NO_PRODUCER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not more than the single producer entry ?

@@ -1323,6 +1326,9 @@ void sec_audit_logger_json(modsec_rec *msr) {
}
}

#ifdef LOG_NO_PRODUCER
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey wait. This is only about JSON version?

Added conditional logging of multi-line logging on top of JSON
Didn't include "Web application info" in the conditional logging
@marcstern
Copy link
Author

Fixed

zimmerle pushed a commit that referenced this pull request May 3, 2017
@@ -1271,6 +1271,9 @@ void sec_audit_logger_json(modsec_rec *msr) {
if (been_opened == 1) {
yajl_gen_map_close(g); // sanitized args map is finished
}
#ifdef LOG_NO_PRODUCER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is way more than the simple producer tag.

@@ -2013,6 +2013,9 @@ void sec_audit_logger_native(modsec_rec *msr) {
sec_auditlog_write(msr, text, strlen(text));
}

#ifdef LOG_NO_PRODUCER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - too many things ignored that are not the producer.

@marcstern
Copy link
Author

The goal was to remove all the "contextual" environment, which is indeed more than the producer. Would it be OK with an explicit name (also in configure) like "LOG_NO_SERVER_CONTEXT"?

@zimmerle
Copy link
Contributor

zimmerle commented May 4, 2017

If that is the goal, it should be clear to whom may read the code and to the user who may enabling it during the configure. So we have to change the help message and the if/defs.

…ver info (log producer, sanitized objects, ...) in audit log.

[Issue #1069 - Marc Stern]
@marcstern
Copy link
Author

Done: {dis|en}able-server-context-logging

@zimmerle zimmerle closed this in d6bd0ba May 8, 2017
@marcstern marcstern deleted the v2/log/producer-try2 branch May 10, 2017 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants