Skip to content
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 #3147 - added httpd logs into foreman-debug #907

Closed
wants to merge 1 commit into from

Conversation

lzap
Copy link
Member

@lzap lzap commented Sep 25, 2013

SSIA I would like to get this into 1.3

@@ -238,6 +238,7 @@ add_files /etc/puppet/ssl/ca/inventory.txt /var/lib/puppet/ssl/ca/inventory.txt
add_cmd "find /etc/puppet/modules -exec ls -ld {} +" "puppet_manifests_tree"
add_files /etc/{httpd,apache2}/conf/*
add_files /etc/{httpd,apache2}/conf.d/*
add_files /var/log/httpd/*.log

Choose a reason for hiding this comment

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

I'm concerned that this will collect too much sensitive data. If users have stuff other than their foreman instance (or even just foreman), they'll be giving up a lot of data about who and what is using Foreman and I'm not sure I'd be comfortable with that as a user.

Copy link
Member

Choose a reason for hiding this comment

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

+1 - I don't think is very useful, as it'll only contain passenger errors. We can always ask for that is we suspect a passenger problem.

Copy link
Member

Choose a reason for hiding this comment

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

Why not split the logs in our vhost?
On Sep 25, 2013 1:36 PM, "Greg Sutcliffe" notifications@github.com wrote:

In script/foreman-debug:

@@ -238,6 +238,7 @@ add_files /etc/puppet/ssl/ca/inventory.txt /var/lib/puppet/ssl/ca/inventory.txt
add_cmd "find /etc/puppet/modules -exec ls -ld {} +" "puppet_manifests_tree"
add_files /etc/{httpd,apache2}/conf/*
add_files /etc/{httpd,apache2}/conf.d/*
+add_files /var/log/httpd/*.log

+1 - I don't think is very useful, as it'll only contain passenger errors.
We can always ask for that is we suspect a passenger problem.


Reply to this email directly or view it on GitHubhttps://github.com//pull/907/files#r6568916
.

Copy link
Contributor

Choose a reason for hiding this comment

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

@GregSutcliffe er, that's precisely what we're trying to fix so we can quickly fix passenger errors. The point of this tool is to collect data useful for diagnosing common error conditions, and Passenger is a part of that.

I don't know if changing the logs will help with some Passenger initialisation errors (particularly on Passenger 3), it'd need to be tested. I'd restrict it to the error logs though, no need for access logs, which could contain more sensitive info.

Choose a reason for hiding this comment

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

Moving the logs into their own files won't help with early initialization issues, but I think those are less common than problems with the rack configuration or other higher-level things. 👍 to moving foreman logs into their own files and then sending only those along with debug.

Another issue is that the log files may end up being really large in some cases - the debugging output should probably truncate to the last 1k lines or something like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

The debug script already truncate all the files and also filters out all
things like "password xyz".

How about taking only *error_log files?

I am fine with creating log files with foreman_ prefix.

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happy with *error_log, I'm unsure if a Foreman vhost-specific one will catch all Passenger breakage.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, please add /var/log/apache2 also for Debian.

Copy link
Member Author

Choose a reason for hiding this comment

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

So how about this...

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

@domcleal
Copy link
Contributor

👍

@domcleal
Copy link
Contributor

Merged as 8092f73, thanks @lzap!

@domcleal domcleal closed this Sep 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants