You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexandra Sava <al...@gmail.com> on 2014/03/18 17:03:30 UTC
Review Request 19357: Add support for all levels of logging through the new
minloglevel flag
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/
-----------------------------------------------------------
Review request for mesos.
Bugs: MESOS-1067
https://issues.apache.org/jira/browse/MESOS-1067
Repository: mesos-git
Description
-------
minloglevel flag is passed as an argument to the command line and it
configures the level of logging to stderr and file. It takes values from
0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
at or above the configured log level will be printed.
If 'quiet' flag is also passed to the command line, minloglevel will
affect just the logs from file (if configured by log_dir flag)
If a log directory is present, the log file that will be available for viewing in the browser
will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
If FATAL is the minimum log level, there is an exception: no log file will
be available in the browser. This is because FATAL logs will cause the program to crash.
So if at any point the user can still access mesos page in the browser, it means no
FATAL logs have been issued.
Diffs
-----
src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
Diff: https://reviews.apache.org/r/19357/diff/
Testing
-------
1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
- expected results: - logs at and above WARNING level are logged into log_dir and stderr
- the log file for WARNING logs is accessible in the browser
2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
- expected results: - logs at FATAL level are logged into log_dir and stderr
- no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
- expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browser
4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
- expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
- no log file is accessible in the browser
5.Test5 - run master and slave with log_dir and quiet flags defined
- expected results: - no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browse
Thanks,
Alexandra Sava
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37561
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19357]
All tests passed.
- Mesos ReviewBot
On March 18, 2014, 4:03 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 4:03 p.m.)
>
>
> Review request for mesos.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It takes values from
> 0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
> On March 18, 2014, 9:40 p.m., Vinod Kone wrote:
> > src/webui/master/static/js/controllers.js, lines 343-349
> > <https://reviews.apache.org/r/19357/diff/1/?file=526261#file526261line343>
> >
> > Why did you need this special case? In other words, what is the behavior if you don't have this?
>
> Alexandra Sava wrote:
> If you try to access the log file in the browser, you get the following message: 'FAILED TO INITIALIZE...RETRYING'
I'll mark this issue as dropped because without this change, 'FAILED TO INITIALIZE...RETRYING' will appear and it does not provide any relevant information about the log file.
> On March 18, 2014, 9:40 p.m., Vinod Kone wrote:
> > src/webui/master/static/js/controllers.js, lines 415-421
> > <https://reviews.apache.org/r/19357/diff/1/?file=526261#file526261line415>
> >
> > ditto. see above.
Same comment as the previous issue.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37632
-----------------------------------------------------------
On March 19, 2014, 11:38 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 19, 2014, 11:38 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
> On March 18, 2014, 9:40 p.m., Vinod Kone wrote:
> > src/webui/master/static/js/controllers.js, lines 343-349
> > <https://reviews.apache.org/r/19357/diff/1/?file=526261#file526261line343>
> >
> > Why did you need this special case? In other words, what is the behavior if you don't have this?
If you try to access the log file in the browser, you get the following message: 'FAILED TO INITIALIZE...RETRYING'
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37632
-----------------------------------------------------------
On March 18, 2014, 5 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 5 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It takes values from
> 0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
> On March 18, 2014, 9:40 p.m., Vinod Kone wrote:
> > src/logging/logging.cpp, lines 115-117
> > <https://reviews.apache.org/r/19357/diff/1/?file=526258#file526258line115>
> >
> > This means we will crash if someone calls this function with "FATAL". Doesn't seem to be nice. Is it possible to just create an empty file for a log level?
No, you can't do that. glog does not probide such a method. It creates the log file as soon as you write the first log message. I will remove the case foar FATAL level.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37632
-----------------------------------------------------------
On March 18, 2014, 5 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 5 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It takes values from
> 0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37632
-----------------------------------------------------------
src/logging/flags.hpp
<https://reviews.apache.org/r/19357/#comment69246>
+1 to Dominic's comment.
Also make "INFO" the default.
src/logging/logging.hpp
<https://reviews.apache.org/r/19357/#comment69248>
New line.
We separate outer elements by 2 lines.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69256>
We use braces around if blocks, even when they contain one statement.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69258>
How about:
string message = "Logging level " + string(google::GetLogSeverityName(severity)) + " started";
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69257>
End with period. s/run/run./
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69259>
This means we will crash if someone calls this function with "FATAL". Doesn't seem to be nice. Is it possible to just create an empty file for a log level?
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69262>
Lets do the validation for 'minloglevel' once.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69260>
s/above/above./
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment69264>
This is a bit hard to follow.
Why not just:
FLAGS_minlogevel = flags.minloglevel at line #148.
src/master/master.cpp
<https://reviews.apache.org/r/19357/#comment69255>
End comment with a period.
s/crash/crash./
src/master/master.cpp
<https://reviews.apache.org/r/19357/#comment69254>
End comment with a period.
s/point/point./
src/slave/slave.cpp
<https://reviews.apache.org/r/19357/#comment69253>
End comment with a period.
s/crash/crash./
src/slave/slave.cpp
<https://reviews.apache.org/r/19357/#comment69266>
ditto. see above.
src/slave/slave.cpp
<https://reviews.apache.org/r/19357/#comment69252>
End comment with a period.
s/point/point./
src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/19357/#comment69250>
Why did you need this special case? In other words, what is the behavior if you don't have this?
src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/19357/#comment69249>
ditto. see above.
- Vinod Kone
On March 18, 2014, 5 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 5 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It takes values from
> 0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37707
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19357]
All tests passed.
- Mesos ReviewBot
On March 19, 2014, 11:38 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 19, 2014, 11:38 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38117
-----------------------------------------------------------
Ship it!
Ship It!
- Dominic Hamon
On March 19, 2014, 4:38 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 19, 2014, 4:38 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
> On March 21, 2014, 5:12 p.m., Vinod Kone wrote:
> > src/logging/logging.cpp, lines 120-135
> > <https://reviews.apache.org/r/19357/diff/2/?file=527984#file527984line120>
> >
> > how about a switch statement instead?
In C++ you can use switch statement just on primitives such as int, char and enum.
> On March 21, 2014, 5:12 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 524
> > <https://reviews.apache.org/r/19357/diff/2/?file=527985#file527985line524>
> >
> > This means you could end up writing "Logging ... level started" somewhere in the middle of the file if the log file is already created by this point. That seems a bit weird.
> >
> > Why not just create the log file in logging::initialize() when we are sure that we haven't written anything yet.
> >
> > That way the only change in this file is
> > s/google::INFO/flags.minloglevel/.
> >
> > Does that make sense?
> >
Yes I will create the log file in logging::initialize() to be sure "Logging ... level started" is the first message in the log file.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38121
-----------------------------------------------------------
On March 19, 2014, 11:38 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 19, 2014, 11:38 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38121
-----------------------------------------------------------
Thanks for the fixes Alexandra. This is looking pretty close to get committed.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70084>
Pull this below the comment.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70085>
new line.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70089>
Let this method just take flags.minloglevel instead of flags.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70086>
how about a switch statement instead?
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70087>
new line.
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70090>
Can we kill this?
src/master/master.cpp
<https://reviews.apache.org/r/19357/#comment70094>
This means you could end up writing "Logging ... level started" somewhere in the middle of the file if the log file is already created by this point. That seems a bit weird.
Why not just create the log file in logging::initialize() when we are sure that we haven't written anything yet.
That way the only change in this file is
s/google::INFO/flags.minloglevel/.
Does that make sense?
src/slave/slave.cpp
<https://reviews.apache.org/r/19357/#comment70095>
ditto. see above.
- Vinod Kone
On March 19, 2014, 11:38 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 19, 2014, 11:38 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review41621
-----------------------------------------------------------
Hey. Mind creating a new review with the changes, since this review has already been committed? Thanks.
- Vinod Kone
On April 28, 2014, 7:14 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 7:14 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/
-----------------------------------------------------------
(Updated April 28, 2014, 7:14 a.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Renamed 'minloglevel' flag in 'logging_level' in order to be consistent the existing flags.
Created validation for the 'logging_level' flag in order to notify the user if it entered an unsupported value.
Updated the documentation for 'logging_level' flag, with the values it can take.
Renamed 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it does.
Bugs: MESOS-1067
https://issues.apache.org/jira/browse/MESOS-1067
Repository: mesos-git
Description
-------
minloglevel flag is passed as an argument to the command line and it
configures the level of logging to stderr and file. It can take INFO,
WARNING, ERROR or FATAL values. All log messages
at or above the configured log level will be printed.
If 'quiet' flag is also passed to the command line, minloglevel will
affect just the logs from file (if configured by log_dir flag)
If a log directory is present, the log file that will be available for viewing in the browser
will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
If FATAL is the minimum log level, there is an exception: no log file will
be available in the browser. This is because FATAL logs will cause the program to crash.
So if at any point the user can still access mesos page in the browser, it means no
FATAL logs have been issued.
Diffs (updated)
-----
src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
Diff: https://reviews.apache.org/r/19357/diff/
Testing
-------
1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
- expected results: - logs at and above WARNING level are logged into log_dir and stderr
- the log file for WARNING logs is accessible in the browser
2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
- expected results: - logs at FATAL level are logged into log_dir and stderr
- no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
- expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browser
4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
- expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
- no log file is accessible in the browser
5.Test5 - run master and slave with log_dir and quiet flags defined
- expected results: - no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browse
Thanks,
Alexandra Sava
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38295
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19357]
All tests passed.
- Mesos ReviewBot
On March 24, 2014, 12:51 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 24, 2014, 12:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Vinod Kone <vi...@gmail.com>.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, line 42
> > <https://reviews.apache.org/r/19357/diff/3/?file=533901#file533901line42>
> >
> > This flag name is inconsistent with our other flag names which use snake case. It would be nice if we could call this "logging_level" :)
>
> Alexandra Sava wrote:
> The only one that has sneak case is 'log_dir'. For example, there is 'logbufsecs' as well that does not have it. Anyway, for me is not a problem changing this but the code has already been commited (commit 7c366bad61e8c241797ef5c92562e0539c0dd02a). I'm not sure how we should proceed in this case: reopen the jira ticket(which has been marked as resolved) or create another one?
FWIW, I actually prefer minloglevel because it is more accurate. And I think it is nice that our logging flag names map directly to GLOG flags. (flags.log_dir => FLAGS_log_dir, flags.logbufsecs => FLAGS_logbufsecs and flags.minloglevel => FLAGS_FLAGS_logbufsecs")
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
On March 24, 2014, 12:51 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 24, 2014, 12:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging
through the new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
Hi guys,
Thanks for the feedback. I will create a new review .
Alexandra
On 28 April 2014 22:19, Ben Mahler <be...@gmail.com> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
>
> On March 25th, 2014, 12:23 a.m. UTC, *Ben Mahler* wrote:
>
> src/master/master.cpp<https://reviews.apache.org/r/19357/diff/3/?file=533904#file533904line514> (Diff
> revision 3)
>
> void Master::initialize()
>
> 514
>
> if (flags.log_dir.isSome()) {
>
> 514
>
> // No need to access FATAL log file; if the program
>
> 515
>
> Try<string> log = logging::getLogFile(google::INFO);
>
> 515
>
> // is still running, there definitely haven't been any
>
> 516
>
> // FATAL logs yet; a FATAL log will cause the program to crash.
>
> 517
>
> google::LogSeverity minloglevel = logging::getMinLogLevel(flags.minloglevel);
>
> 518
>
> if (flags.log_dir.isSome() && minloglevel != google::FATAL) {
>
> 519
>
> Try<string> log = logging::getLogFile(minloglevel);
>
> Would it not be simpler to disallow FATAL-only logging for now so that we don't have to do this special casing in the Master / Slave / webui?
>
> Or, if we want to keep FATAL-only logging as an option, we could update the "pailer" component of the webui show the underlying error from the backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts?
>
> On March 25th, 2014, 8:04 a.m. UTC, *Alexandra Sava* wrote:
>
> When a FATAL log is been issued, the program exits(this is how glog works) so you won't be able to access the log file from the web gui. I already updated the webgui for the case when minloglevel is FATAL (in this patch), so you will not see "FAILED TO INITIALIZE...RETRYING".
>
> On March 25th, 2014, 6:31 p.m. UTC, *Ben Mahler* wrote:
>
> I understand the semantics of FATAL logging and the log file creation, I'm just not convinced we should add the special case logic in the Master, Slave, webui, just to support FATAL-only logging. So, I'm wondering why we don't just support INFO,WARNING,ERROR to keep things simple. If it turns out that people need FATAL we can add it.
>
> On April 28th, 2014, 7:16 a.m. UTC, *Alexandra Sava* wrote:
>
> If FATAL logs are used int the code, why not having the possibility of configuring the log level as FATAL? I don't see where it gets complicated if we have a few lines more for FATAL logs.
>
> We had to change the master, slave, logging library, and the webui to handle the FATAL case. In general, special cases are unfortunate because it means an "exceptional" case that we need to:
>
> 1. Remember in our mental model of the system.
> 2. Add comments to inform the reader of the code.
> 3. Handle correctly in all the appropriate places (opening room for more bugs).
>
> This is why I feel supporting FATAL introduces too much complexity for the use-case. Does this make sense?
>
> As vinod said, following up in a separate review would be great!
>
>
> - Ben
>
> On April 28th, 2014, 7:14 a.m. UTC, Alexandra Sava wrote:
> Review request for mesos and Vinod Kone.
> By Alexandra Sava.
>
> *Updated April 28, 2014, 7:14 a.m.*
> *Bugs: * MESOS-1067 <https://issues.apache.org/jira/browse/MESOS-1067>
> *Repository: * mesos-git
> Description
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
> Testing
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
> Diffs
>
> - src/logging/flags.hpp (457feeeb7ca7ececf3c4e69189800ecb370053e6)
> - src/logging/logging.hpp (39c2934f733e5c058f62b5497f31f7a7057bb4c7)
> - src/logging/logging.cpp (176e49a6a2aef13be44ff910144c7321c942345a)
> - src/master/master.cpp (f205dca43f10697862e3fd3f435f1127a9d0aecb)
> - src/slave/slave.cpp (cb80609ba421b3b9a4664e600f0e53ecab8574c4)
> - src/webui/master/static/js/controllers.js
> (4b8487e0c285f892ad352993c81637f38df1429f)
>
> View Diff <https://reviews.apache.org/r/19357/diff/>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Ben Mahler <be...@gmail.com>.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 514-519
> > <https://reviews.apache.org/r/19357/diff/3/?file=533904#file533904line514>
> >
> > Would it not be simpler to disallow FATAL-only logging for now so that we don't have to do this special casing in the Master / Slave / webui?
> >
> > Or, if we want to keep FATAL-only logging as an option, we could update the "pailer" component of the webui show the underlying error from the backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts?
>
> Alexandra Sava wrote:
> When a FATAL log is been issued, the program exits(this is how glog works) so you won't be able to access the log file from the web gui. I already updated the webgui for the case when minloglevel is FATAL (in this patch), so you will not see "FAILED TO INITIALIZE...RETRYING".
>
> Ben Mahler wrote:
> I understand the semantics of FATAL logging and the log file creation, I'm just not convinced we should add the special case logic in the Master, Slave, webui, just to support FATAL-only logging. So, I'm wondering why we don't just support INFO,WARNING,ERROR to keep things simple. If it turns out that people need FATAL we can add it.
>
> Alexandra Sava wrote:
> If FATAL logs are used int the code, why not having the possibility of configuring the log level as FATAL? I don't see where it gets complicated if we have a few lines more for FATAL logs.
We had to change the master, slave, logging library, and the webui to handle the FATAL case. In general, special cases are unfortunate because it means an "exceptional" case that we need to:
1. Remember in our mental model of the system.
2. Add comments to inform the reader of the code.
3. Handle correctly in all the appropriate places (opening room for more bugs).
This is why I feel supporting FATAL introduces too much complexity for the use-case. Does this make sense?
As vinod said, following up in a separate review would be great!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
On April 28, 2014, 7:14 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 7:14 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Ben Mahler <be...@gmail.com>.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, lines 43-45
> > <https://reviews.apache.org/r/19357/diff/3/?file=533901#file533901line43>
> >
> > Please quote the other flags, e.g. 'quiet' and 'log_dir'. Also, can we tell them what the valid values for this "logging_level" are? How are they to know that they can specify "INFO", "WARNING", "ERROR", "FATAL"?
>
> Alexandra Sava wrote:
> I think users will be able to see what flags/flags values Mesos supports through documentation. Ho were they able to find out which flags/flags values Mesos supports up until now?
This is the part of the help message that gets printed when a user enters:
$ mesos-master --help
$ mesos-slave --help
etc
So this is documentation, and we should guide users as to what flag values are acceptable :)
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 100-114
> > <https://reviews.apache.org/r/19357/diff/3/?file=533903#file533903line100>
> >
> > Can you add a comment that this is because GLOG only lazily creates the log file once the first log message occurs?
> >
> > Also, seems like this code could be cleaned up to be just:
> >
> > LOG(severity) << "Logging " << google::getLogSeverity(severity) << " level started";
>
> Alexandra Sava wrote:
> LOG(severity) does not work. You get a compilation error ( error: 'COMPACT_GOOGLE_LOG_severity' was not declared in this scope).
Can you use LOG_AT_LEVEL?
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 118-136
> > <https://reviews.apache.org/r/19357/diff/3/?file=533903#file533903line118>
> >
> > This looks more like a "getLogSeverity" function to convert from logging level string to 'LogSeverity' type?
> >
> > I'm curious why you decided to proceed with INFO if the flag has an unexpected value? Seems like an operator would want an EXIT(1) in this case to be aware of the mistake in flags..
>
> Alexandra Sava wrote:
> I decided to proceed with INFO because up until now the default log level was INFO. I didn't put an EXIT there because this is not the kind of flag without whom the program could not work.
Best practice is to inform the operator if they've made a mistake in their configuration rather than to silently change it.
If they pass FOOBAR, it would be nice to tell them 'FOOBAR is not a valid logging level' as this avoids possible confusion on the side of the operator.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 514-519
> > <https://reviews.apache.org/r/19357/diff/3/?file=533904#file533904line514>
> >
> > Would it not be simpler to disallow FATAL-only logging for now so that we don't have to do this special casing in the Master / Slave / webui?
> >
> > Or, if we want to keep FATAL-only logging as an option, we could update the "pailer" component of the webui show the underlying error from the backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts?
>
> Alexandra Sava wrote:
> When a FATAL log is been issued, the program exits(this is how glog works) so you won't be able to access the log file from the web gui. I already updated the webgui for the case when minloglevel is FATAL (in this patch), so you will not see "FAILED TO INITIALIZE...RETRYING".
I understand the semantics of FATAL logging and the log file creation, I'm just not convinced we should add the special case logic in the Master, Slave, webui, just to support FATAL-only logging. So, I'm wondering why we don't just support INFO,WARNING,ERROR to keep things simple. If it turns out that people need FATAL we can add it.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
On March 24, 2014, 12:51 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 24, 2014, 12:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Ben Mahler <be...@gmail.com>.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, line 42
> > <https://reviews.apache.org/r/19357/diff/3/?file=533901#file533901line42>
> >
> > This flag name is inconsistent with our other flag names which use snake case. It would be nice if we could call this "logging_level" :)
>
> Alexandra Sava wrote:
> The only one that has sneak case is 'log_dir'. For example, there is 'logbufsecs' as well that does not have it. Anyway, for me is not a problem changing this but the code has already been commited (commit 7c366bad61e8c241797ef5c92562e0539c0dd02a). I'm not sure how we should proceed in this case: reopen the jira ticket(which has been marked as resolved) or create another one?
>
> Vinod Kone wrote:
> FWIW, I actually prefer minloglevel because it is more accurate. And I think it is nice that our logging flag names map directly to GLOG flags. (flags.log_dir => FLAGS_log_dir, flags.logbufsecs => FLAGS_logbufsecs and flags.minloglevel => FLAGS_FLAGS_logbufsecs")
That's not quite true since we have 'quiet', right?
Do you recall when we watched an entire room of operators get confused when using the (replicated) log tooling and thinking that 'log_dir' was the directory for the replicated log?
Because logging flags are global to all of our binaries, it would be nice to make these very explicit. 'logging_level', 'logging_directory', 'logging_quiet' read more clearly to me when faced with flags coming from various components, including the replicated log.
Alexandra, in the rest of our flags across the project, we use snake case.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
On March 24, 2014, 12:51 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 24, 2014, 12:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, line 42
> > <https://reviews.apache.org/r/19357/diff/3/?file=533901#file533901line42>
> >
> > This flag name is inconsistent with our other flag names which use snake case. It would be nice if we could call this "logging_level" :)
The only one that has sneak case is 'log_dir'. For example, there is 'logbufsecs' as well that does not have it. Anyway, for me is not a problem changing this but the code has already been commited (commit 7c366bad61e8c241797ef5c92562e0539c0dd02a). I'm not sure how we should proceed in this case: reopen the jira ticket(which has been marked as resolved) or create another one?
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 100-114
> > <https://reviews.apache.org/r/19357/diff/3/?file=533903#file533903line100>
> >
> > Can you add a comment that this is because GLOG only lazily creates the log file once the first log message occurs?
> >
> > Also, seems like this code could be cleaned up to be just:
> >
> > LOG(severity) << "Logging " << google::getLogSeverity(severity) << " level started";
LOG(severity) does not work. You get a compilation error ( error: 'COMPACT_GOOGLE_LOG_severity' was not declared in this scope).
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 118-136
> > <https://reviews.apache.org/r/19357/diff/3/?file=533903#file533903line118>
> >
> > This looks more like a "getLogSeverity" function to convert from logging level string to 'LogSeverity' type?
> >
> > I'm curious why you decided to proceed with INFO if the flag has an unexpected value? Seems like an operator would want an EXIT(1) in this case to be aware of the mistake in flags..
I decided to proceed with INFO because up until now the default log level was INFO. I didn't put an EXIT there because this is not the kind of flag without whom the program could not work.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 514-519
> > <https://reviews.apache.org/r/19357/diff/3/?file=533904#file533904line514>
> >
> > Would it not be simpler to disallow FATAL-only logging for now so that we don't have to do this special casing in the Master / Slave / webui?
> >
> > Or, if we want to keep FATAL-only logging as an option, we could update the "pailer" component of the webui show the underlying error from the backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts?
When a FATAL log is been issued, the program exits(this is how glog works) so you won't be able to access the log file from the web gui. I already updated the webgui for the case when minloglevel is FATAL (in this patch), so you will not see "FAILED TO INITIALIZE...RETRYING".
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, lines 43-45
> > <https://reviews.apache.org/r/19357/diff/3/?file=533901#file533901line43>
> >
> > Please quote the other flags, e.g. 'quiet' and 'log_dir'. Also, can we tell them what the valid values for this "logging_level" are? How are they to know that they can specify "INFO", "WARNING", "ERROR", "FATAL"?
I think users will be able to see what flags/flags values Mesos supports through documentation. Ho were they able to find out which flags/flags values Mesos supports up until now?
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
On March 24, 2014, 12:51 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 24, 2014, 12:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 100-114
> > <https://reviews.apache.org/r/19357/diff/3/?file=533903#file533903line100>
> >
> > Can you add a comment that this is because GLOG only lazily creates the log file once the first log message occurs?
> >
> > Also, seems like this code could be cleaned up to be just:
> >
> > LOG(severity) << "Logging " << google::getLogSeverity(severity) << " level started";
>
> Alexandra Sava wrote:
> LOG(severity) does not work. You get a compilation error ( error: 'COMPACT_GOOGLE_LOG_severity' was not declared in this scope).
>
> Ben Mahler wrote:
> Can you use LOG_AT_LEVEL?
Yes, LOG_AT_LEVEL worked.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 514-519
> > <https://reviews.apache.org/r/19357/diff/3/?file=533904#file533904line514>
> >
> > Would it not be simpler to disallow FATAL-only logging for now so that we don't have to do this special casing in the Master / Slave / webui?
> >
> > Or, if we want to keep FATAL-only logging as an option, we could update the "pailer" component of the webui show the underlying error from the backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts?
>
> Alexandra Sava wrote:
> When a FATAL log is been issued, the program exits(this is how glog works) so you won't be able to access the log file from the web gui. I already updated the webgui for the case when minloglevel is FATAL (in this patch), so you will not see "FAILED TO INITIALIZE...RETRYING".
>
> Ben Mahler wrote:
> I understand the semantics of FATAL logging and the log file creation, I'm just not convinced we should add the special case logic in the Master, Slave, webui, just to support FATAL-only logging. So, I'm wondering why we don't just support INFO,WARNING,ERROR to keep things simple. If it turns out that people need FATAL we can add it.
If FATAL logs are used int the code, why not having the possibility of configuring the log level as FATAL? I don't see where it gets complicated if we have a few lines more for FATAL logs.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/flags.hpp, lines 43-45
> > <https://reviews.apache.org/r/19357/diff/3/?file=533901#file533901line43>
> >
> > Please quote the other flags, e.g. 'quiet' and 'log_dir'. Also, can we tell them what the valid values for this "logging_level" are? How are they to know that they can specify "INFO", "WARNING", "ERROR", "FATAL"?
>
> Alexandra Sava wrote:
> I think users will be able to see what flags/flags values Mesos supports through documentation. Ho were they able to find out which flags/flags values Mesos supports up until now?
>
> Ben Mahler wrote:
> This is the part of the help message that gets printed when a user enters:
>
> $ mesos-master --help
> $ mesos-slave --help
> etc
>
> So this is documentation, and we should guide users as to what flag values are acceptable :)
I updated the documentation with the values it can have.
> On March 25, 2014, 12:23 a.m., Ben Mahler wrote:
> > src/logging/logging.cpp, lines 118-136
> > <https://reviews.apache.org/r/19357/diff/3/?file=533903#file533903line118>
> >
> > This looks more like a "getLogSeverity" function to convert from logging level string to 'LogSeverity' type?
> >
> > I'm curious why you decided to proceed with INFO if the flag has an unexpected value? Seems like an operator would want an EXIT(1) in this case to be aware of the mistake in flags..
>
> Alexandra Sava wrote:
> I decided to proceed with INFO because up until now the default log level was INFO. I didn't put an EXIT there because this is not the kind of flag without whom the program could not work.
>
> Ben Mahler wrote:
> Best practice is to inform the operator if they've made a mistake in their configuration rather than to silently change it.
>
> If they pass FOOBAR, it would be nice to tell them 'FOOBAR is not a valid logging level' as this avoids possible confusion on the side of the operator.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
On April 28, 2014, 7:14 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 7:14 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6
> src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7
> src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a
> src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb
> src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4
> src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review38382
-----------------------------------------------------------
src/logging/flags.hpp
<https://reviews.apache.org/r/19357/#comment70472>
This flag name is inconsistent with our other flag names which use snake case. It would be nice if we could call this "logging_level" :)
src/logging/flags.hpp
<https://reviews.apache.org/r/19357/#comment70475>
Please quote the other flags, e.g. 'quiet' and 'log_dir'. Also, can we tell them what the valid values for this "logging_level" are? How are they to know that they can specify "INFO", "WARNING", "ERROR", "FATAL"?
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70522>
I would prefer to see validation of the flag to ensure these cases are not possible, that way operators know they've made a mistake! :)
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70521>
Can you add a comment that this is because GLOG only lazily creates the log file once the first log message occurs?
Also, seems like this code could be cleaned up to be just:
LOG(severity) << "Logging " << google::getLogSeverity(severity) << " level started";
src/logging/logging.cpp
<https://reviews.apache.org/r/19357/#comment70519>
This looks more like a "getLogSeverity" function to convert from logging level string to 'LogSeverity' type?
I'm curious why you decided to proceed with INFO if the flag has an unexpected value? Seems like an operator would want an EXIT(1) in this case to be aware of the mistake in flags..
src/master/master.cpp
<https://reviews.apache.org/r/19357/#comment70523>
Would it not be simpler to disallow FATAL-only logging for now so that we don't have to do this special casing in the Master / Slave / webui?
Or, if we want to keep FATAL-only logging as an option, we could update the "pailer" component of the webui show the underlying error from the backend instead of "FAILED TO INITIALIZE...RETRYING'", thoughts?
- Ben Mahler
On March 24, 2014, 12:51 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 24, 2014, 12:51 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It can take INFO,
> WARNING, ERROR or FATAL values. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/
-----------------------------------------------------------
(Updated March 24, 2014, 12:51 p.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Fix coding style issues.
Create log file in logging::initialize() to be sure "Logging ... level started" is the first message in the log file.
Bugs: MESOS-1067
https://issues.apache.org/jira/browse/MESOS-1067
Repository: mesos-git
Description
-------
minloglevel flag is passed as an argument to the command line and it
configures the level of logging to stderr and file. It can take INFO,
WARNING, ERROR or FATAL values. All log messages
at or above the configured log level will be printed.
If 'quiet' flag is also passed to the command line, minloglevel will
affect just the logs from file (if configured by log_dir flag)
If a log directory is present, the log file that will be available for viewing in the browser
will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
If FATAL is the minimum log level, there is an exception: no log file will
be available in the browser. This is because FATAL logs will cause the program to crash.
So if at any point the user can still access mesos page in the browser, it means no
FATAL logs have been issued.
Diffs (updated)
-----
src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
Diff: https://reviews.apache.org/r/19357/diff/
Testing
-------
1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
- expected results: - logs at and above WARNING level are logged into log_dir and stderr
- the log file for WARNING logs is accessible in the browser
2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
- expected results: - logs at FATAL level are logged into log_dir and stderr
- no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
- expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browser
4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
- expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
- no log file is accessible in the browser
5.Test5 - run master and slave with log_dir and quiet flags defined
- expected results: - no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browse
Thanks,
Alexandra Sava
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/
-----------------------------------------------------------
(Updated March 19, 2014, 11:38 a.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-1067
https://issues.apache.org/jira/browse/MESOS-1067
Repository: mesos-git
Description (updated)
-------
minloglevel flag is passed as an argument to the command line and it
configures the level of logging to stderr and file. It can take INFO,
WARNING, ERROR or FATAL values. All log messages
at or above the configured log level will be printed.
If 'quiet' flag is also passed to the command line, minloglevel will
affect just the logs from file (if configured by log_dir flag)
If a log directory is present, the log file that will be available for viewing in the browser
will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
If FATAL is the minimum log level, there is an exception: no log file will
be available in the browser. This is because FATAL logs will cause the program to crash.
So if at any point the user can still access mesos page in the browser, it means no
FATAL logs have been issued.
Diffs (updated)
-----
src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
Diff: https://reviews.apache.org/r/19357/diff/
Testing (updated)
-------
1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to WARNING
- expected results: - logs at and above WARNING level are logged into log_dir and stderr
- the log file for WARNING logs is accessible in the browser
2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to FATAL
- expected results: - logs at FATAL level are logged into log_dir and stderr
- no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to INFO and quiet flag defined
- expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browser
4.Test4 - run master and slave with minloglevel flag set to INFO and quiet flag defined
- expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
- no log file is accessible in the browser
5.Test5 - run master and slave with log_dir and quiet flags defined
- expected results: - no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browse
Thanks,
Alexandra Sava
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37644
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/19357/#comment69271>
My comment seem to have vanished. Here it goes again.
This piece is a bit complicated to follow.
Can we get this working by just changing
Try<string> log = logging::getLogFile(flags.minloglevel);
?
If logging.cpp always creates an empty log file at the requested level, it should work right?
- Vinod Kone
On March 18, 2014, 5 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 5 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It takes values from
> 0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/#review37594
-----------------------------------------------------------
src/logging/flags.hpp
<https://reviews.apache.org/r/19357/#comment69197>
maybe this should be a string 'INFO', 'WARNING', etc to be clearer for users.
- Dominic Hamon
On March 18, 2014, 10 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19357/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 10 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1067
> https://issues.apache.org/jira/browse/MESOS-1067
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> minloglevel flag is passed as an argument to the command line and it
> configures the level of logging to stderr and file. It takes values from
> 0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
> at or above the configured log level will be printed.
>
> If 'quiet' flag is also passed to the command line, minloglevel will
> affect just the logs from file (if configured by log_dir flag)
>
> If a log directory is present, the log file that will be available for viewing in the browser
> will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
> If FATAL is the minimum log level, there is an exception: no log file will
> be available in the browser. This is because FATAL logs will cause the program to crash.
> So if at any point the user can still access mesos page in the browser, it means no
> FATAL logs have been issued.
>
>
> Diffs
> -----
>
> src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
> src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
> src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
> src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
> src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
> src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
>
> Diff: https://reviews.apache.org/r/19357/diff/
>
>
> Testing
> -------
>
> 1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
> - expected results: - logs at and above WARNING level are logged into log_dir and stderr
> - the log file for WARNING logs is accessible in the browser
>
> 2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
> - expected results: - logs at FATAL level are logged into log_dir and stderr
> - no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
>
> 3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
> - expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browser
>
> 4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
> - expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
> - no log file is accessible in the browser
>
> 5.Test5 - run master and slave with log_dir and quiet flags defined
> - expected results: - no logs (just FATAL) are displayed at stderr
> - the log file for INFO logs is accessible in the browse
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 19357: Add support for all levels of logging through the
new minloglevel flag
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19357/
-----------------------------------------------------------
(Updated March 18, 2014, 5 p.m.)
Review request for mesos and Vinod Kone.
Changes
-------
assigning to myself to shepherd -- Vinod.
Bugs: MESOS-1067
https://issues.apache.org/jira/browse/MESOS-1067
Repository: mesos-git
Description
-------
minloglevel flag is passed as an argument to the command line and it
configures the level of logging to stderr and file. It takes values from
0 to 3 which traduces into INFO, WARNING, ERROR and FATAL. All log messages
at or above the configured log level will be printed.
If 'quiet' flag is also passed to the command line, minloglevel will
affect just the logs from file (if configured by log_dir flag)
If a log directory is present, the log file that will be available for viewing in the browser
will be the one for the minimum log level configured (with or without the presence of minloglevel flag)
If FATAL is the minimum log level, there is an exception: no log file will
be available in the browser. This is because FATAL logs will cause the program to crash.
So if at any point the user can still access mesos page in the browser, it means no
FATAL logs have been issued.
Diffs
-----
src/logging/flags.hpp e2e4afccc4d8b8cc269960f90d59aab9c1e53807
src/logging/logging.hpp 3c24211cb711a43d6f23950607f5a30e1351dd7d
src/logging/logging.cpp a46a3f133830297cbc466e761e1682fb49df09df
src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437
src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54
src/webui/master/static/js/controllers.js afb24fb9c2184772f7314162f5637dbabaa2ab94
Diff: https://reviews.apache.org/r/19357/diff/
Testing
-------
1.Test1 - run master and slave with log_dir flag defined and minloglevel flag set to 1
- expected results: - logs at and above WARNING level are logged into log_dir and stderr
- the log file for WARNING logs is accessible in the browser
2.Test2 - run master and slave with log_dir flag defined and minloglevel flag set to 3
- expected results: - logs at FATAL level are logged into log_dir and stderr
- no log file is accessible in the browser because as soon as a FATAL log will be issued, the program will crash
3.Test3 - run master and slave with log_dir flag defined, minloglevel flag set to 0 and quiet flag defined
- expected results: - logs at and above INFO level are logged into log_dir and no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browser
4.Test4 - run master and slave with minloglevel flag set to 0 and quiet flag defined
- expected results: - no logs (just FATAL) are displayed at stderr (quiet flag takes precedence)
- no log file is accessible in the browser
5.Test5 - run master and slave with log_dir and quiet flags defined
- expected results: - no logs (just FATAL) are displayed at stderr
- the log file for INFO logs is accessible in the browse
Thanks,
Alexandra Sava