> On March 14, 2014, 10:23 a.m., Vinod Kone wrote: > > 3rdparty/libprocess/src/process.cpp, lines 3207-3208 > > <https://reviews.apache.org/r/19208/diff/1/?file=519368#file519368line3207> > > > > We avoid doing LOG(INFO) in our 3rd party libraries. Convert this to > > VLOG instead. > > > > More importantly, at Twitter we have observed that these LOGs are > > actually useful during debugging and hence we wanted these to be always > > present (i.e., LOG(INFO) instead of VLOG). That's likely the reason these > > are in http.cpp files. > > > > Dominic Hamon wrote: > This doesn't add any more log-spam than we already had as we were logging > this anyway, albeit without the IP address. We can't easily expose the IP > address down to the http files (and probably shouldn't) whereas this is > really useful debugging information. > > This should be a LOG(INFO), or at least a log to a separate HTTP access > log file if the main log is too sensitive to it. > > Vinod Kone wrote: > Rebase? > > More importantly, this is not an issue of log spam. We have deliberately > avoided adding LOG(INFO) statements in stout and libprocess (the only > violation i see is in the profiler but that should be fixed). > > ? ack "LOG\(INFO" ../3rdparty/libprocess/ > ../3rdparty/libprocess/include/process/profiler.hpp > 58: LOG(INFO) << "Starting Profiler"; > 94: LOG(INFO) << "Stopping Profiler"; > ? ack "LOG\(INFO" ../3rdparty/libprocess/3rdparty/stout > ? ? > > > How about adding the sender ip/hostname as a member in "http::Request"? > Not sure if thats the right place. BenM? >
Adding sender IP/hostname to http::Request sounds reasonable, though it does mean that every endpoint is responsible for logging the info. For something so important to debugging, that seems fragile. Is the concern about having LOG(INFO) in libprocess that it will get out of hand? Every HTTP server I know of has some kind of default access logging, precisely because it is vital information. Moving it to a custom log is doable, but requires infrastructure around log rotation that may be a pain to deal with. - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19208/#review37220 ----------------------------------------------------------- On March 19, 2014, 3:49 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19208/ > ----------------------------------------------------------- > > (Updated March 19, 2014, 3:49 p.m.) > > > Review request for mesos, Benjamin Hindman and Ian Downes. > > > Bugs: MESOS-1096 > https://issues.apache.org/jira/browse/MESOS-1096 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/Makefile.am > df149ddc8669b0c541361922631f4f3958bd096d > 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp > 47fe92c380de3e2abc625dc936afbd034280b76a > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 901e5550c82c12934a6b9c3154f030c677e41a38 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp > ba8463cc152701d6eb1deba8d1561688ecf5b7b5 > 3rdparty/libprocess/include/process/future.hpp > 5e124760ca304082da506c91b6341e784f103d93 > 3rdparty/libprocess/include/process/socket.hpp > dbcb4f4c2eb12663158057a844b4511d6dde0508 > 3rdparty/libprocess/include/process/subprocess.hpp > 75b2a96b3ca62a4a59e64aba046f1d797bd8ebf6 > 3rdparty/libprocess/src/process.cpp > 6c6acc03ad78779e8f25b1a4584069fd377f647b > bin/mesos-slave-flags.sh.in 8c936aa06e994a87a8b09b31c907868bf9be38c7 > configure.ac 9a6de87fa6523bf8137f5d74ea0b7c6cbd947d3a > docs/configuration.md 2dbe0f81aea0e0db41f3eb4b0554ae5e9f13da2e > docs/getting-started.md d7ced76125c6aba0d7e1a0f4ca8165652b17af0d > src/Makefile.am 0775a0df293e945d41c7ba90fd1bbb503ae22f9e > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in > 668647fdfc0e203fcde59263256659ba14e29960 > src/linux/cgroups.hpp 6056e3ae59ed813d75b1bb46b276872530155ec7 > src/linux/cgroups.cpp eff15af417f3827b4a1f73770b98dc9fccbc397e > src/linux/fs.hpp ac8b5f416dae0a9388a3839b6f078abdcd42edae > src/linux/fs.cpp b01d14c3a1b296566b2b17a7f540097bd7cc53dd > src/local/local.cpp 2cfdf49c9eb92302502eb50c623f9606977b88b6 > src/log/log.cpp d9b26871bfa1b3c36f5e7d879eb7b67132af1b33 > src/master/detector.cpp 8b10061de12a35ad6624db594dc4ec36502b2420 > src/master/flags.hpp 024f86d93824a20ce42c28b8264576f1cb715d0e > src/master/http.cpp 72d8e91013cd6a9c52a6d1ffdf517a52c4567bb7 > src/master/main.cpp f12f20a1eabd163c4f35056bf01f28f3edd408a9 > src/master/master.hpp 0c7c5204c31087f12c4e98028f90c1b941eab7c7 > src/master/master.cpp 6da776699beb6f449e8160dcb6a125d94c1ab437 > src/master/registrar.hpp 98bfa1e83b7e6d28e011444a665155e31922446c > src/master/registrar.cpp cbb67bd17402692d3bb81fe58baea952897c56d1 > src/sched/sched.cpp 3684cfe9d153cab5f4ea86094fffd814ce74baa1 > src/slave/containerizer/containerizer.cpp > faf3d0cf97aafd88eab82ae39e2255b1eb4ce294 > src/slave/containerizer/isolators/cgroups/cpushare.cpp > 11665dbe88e3920fd9d8a2259987611d49e85462 > src/slave/containerizer/mesos_containerizer.cpp > c819c97d96d232a1de3c1ed2fc848bebf66981f7 > src/slave/slave.cpp d8d3e0fa54972201d72b2650ec0ba922a4912d54 > src/slave/status_update_manager.cpp > 5d5cf234ef2dd47fa4b1f67be761dbca31659451 > src/tests/cluster.hpp 40d9f8c18307aead2374396710f9a82466e3a716 > src/tests/environment.cpp feeca042265eede40e87db3ee5ab89be04509a90 > src/tests/registrar_tests.cpp 41836aeaf94e1b21e040b9e8ecf71e0b5a351f8e > src/tests/state_tests.cpp d0e084070c566ee7d751a8e1279772e05b966145 > > Diff: https://reviews.apache.org/r/19208/diff/ > > > Testing > ------- > > make check > > ran locally and connected to :5050 endpoint and watched log > > > Thanks, > > Dominic Hamon > >
