shill: diagnostics: Stash away net.log on connectivity event.
The stashed log can be included in user-initiated feedback report at a
later point.
BUG=chromium-os:36923
TEST=unit tests; run net-diags-upload on device
Change-Id: I679f559e5eff2e38ed6a38d47b284da8b06b1f51
Reviewed-on: https://gerrit.chromium.org/gerrit/39310
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew%chromium.org@gtempaccount.com>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Darin Petkov <petkov@chromium.org>
diff --git a/Makefile b/Makefile
index 8457b69..cbe268c 100644
--- a/Makefile
+++ b/Makefile
@@ -36,6 +36,7 @@
ModemManager \
libnl-3.0 \
libnl-genl-3.0
+NET_DIAGS_UPLOAD_PC_DEPS = $(COMMON_PC_DEPS)
NSS_GET_CERT_PC_DEPS = $(COMMON_PC_DEPS) nss
OPENVPN_SCRIPT_PC_DEPS = $(COMMON_PC_DEPS) dbus-c++-1
PPPD_PLUGIN_PC_DEPS = $(COMMON_PC_DEPS) dbus-c++-1
@@ -44,6 +45,7 @@
-iquote.. \
-iquote $(BUILDDIR) \
$(shell $(PKG_CONFIG) --cflags \
+ $(NET_DIAGS_UPLOAD_PC_DEPS) \
$(NSS_GET_CERT_PC_DEPS) \
$(OPENVPN_SCRIPT_PC_DEPS) \
$(PPPD_PLUGIN_PC_DEPS) \
@@ -56,6 +58,8 @@
-lmetrics \
-lminijail \
$(shell $(PKG_CONFIG) --libs $(SHILL_PC_DEPS))
+NET_DIAGS_UPLOAD_LIBS = \
+ $(shell $(PKG_CONFIG) --libs $(NET_DIAGS_UPLOAD_PC_DEPS))
NSS_GET_CERT_LIBS = $(shell $(PKG_CONFIG) --libs $(NSS_GET_CERT_PC_DEPS))
OPENVPN_SCRIPT_LIBS = $(shell $(PKG_CONFIG) --libs $(OPENVPN_SCRIPT_PC_DEPS))
PPPD_PLUGIN_LIBS = $(shell $(PKG_CONFIG) --libs $(PPPD_PLUGIN_PC_DEPS))
@@ -537,7 +541,7 @@
$(CXX) $(CXXFLAGS) $(LDFLAGS) $^ $(SHILL_LIBS) -o $@
$(NET_DIAGS_UPLOAD_BIN): $(NET_DIAGS_UPLOAD_MAIN_OBJ)
- $(CXX) $(CXXFLAGS) $(LDFLAGS) $^ -o $@
+ $(CXX) $(CXXFLAGS) $(LDFLAGS) $^ $(NET_DIAGS_UPLOAD_LIBS) -o $@
$(NSS_GET_CERT_BIN): $(NSS_GET_CERT_MAIN_OBJ) $(NSS_GET_CERT_OBJS) $(SHILL_LIB)
$(CXX) $(CXXFLAGS) $(LDFLAGS) $^ $(NSS_GET_CERT_LIBS) -o $@
diff --git a/diagnostics_reporter.cc b/diagnostics_reporter.cc
index 8f730be..f464c79 100644
--- a/diagnostics_reporter.cc
+++ b/diagnostics_reporter.cc
@@ -35,31 +35,6 @@
glib_ = glib;
}
-void DiagnosticsReporter::Report() {
- if (!IsReportingEnabled()) {
- return;
- }
- LOG(INFO) << "Spawning " << kNetDiagsUpload;
- CHECK(glib_);
- char *argv[] = { const_cast<char *>(kNetDiagsUpload), NULL };
- char *envp[] = { NULL };
- int status = 0;
- GError *error = NULL;
- if (!glib_->SpawnSync(NULL,
- argv,
- envp,
- static_cast<GSpawnFlags>(0),
- NULL,
- NULL,
- NULL,
- NULL,
- &status,
- &error)) {
- LOG(ERROR) << "net-diags-upload failed: "
- << glib_->ConvertErrorToMessage(error);
- }
-}
-
void DiagnosticsReporter::OnConnectivityEvent() {
LOG(INFO) << "Diagnostics event triggered.";
@@ -73,8 +48,26 @@
last_log_stash_ = now.tv_sec;
- // TODO(petkov): Stash away logs for potential inclusion in feedback
- // (crosbug.com/36923).
+ LOG(INFO) << "Spawning " << kNetDiagsUpload << " @ " << last_log_stash_;
+ CHECK(glib_);
+ char *argv[] = {
+ const_cast<char *>(kNetDiagsUpload),
+ IsReportingEnabled() ? const_cast<char *>("--upload") : NULL,
+ NULL
+ };
+ char *envp[] = { NULL };
+ GError *error = NULL;
+ // TODO(petkov): Run the subprocess through minijail (crosbug.com/37099).
+ if (!glib_->SpawnAsync(NULL,
+ argv,
+ envp,
+ static_cast<GSpawnFlags>(0),
+ NULL,
+ NULL,
+ NULL,
+ &error)) {
+ LOG(ERROR) << "Spawn failed: " << glib_->ConvertErrorToMessage(error);
+ }
}
bool DiagnosticsReporter::IsReportingEnabled() {
diff --git a/diagnostics_reporter.h b/diagnostics_reporter.h
index cb0b1bc..2d4a4bc 100644
--- a/diagnostics_reporter.h
+++ b/diagnostics_reporter.h
@@ -30,9 +30,6 @@
virtual bool IsReportingEnabled();
- // Uploads diagnostics data for analysis.
- void Report();
-
private:
friend struct base::DefaultLazyInstanceTraits<DiagnosticsReporter>;
friend class DiagnosticsReporterTest;
diff --git a/diagnostics_reporter_unittest.cc b/diagnostics_reporter_unittest.cc
index 3332e51..028b847 100644
--- a/diagnostics_reporter_unittest.cc
+++ b/diagnostics_reporter_unittest.cc
@@ -11,6 +11,7 @@
#include "shill/mock_time.h"
using testing::_;
+using testing::InSequence;
using testing::Return;
using testing::SetArgumentPointee;
@@ -24,8 +25,6 @@
MOCK_METHOD0(IsReportingEnabled, bool());
- void Report() { DiagnosticsReporter::Report(); }
-
private:
DISALLOW_COPY_AND_ASSIGN(ReporterUnderTest);
};
@@ -55,18 +54,6 @@
ReporterUnderTest reporter_;
};
-TEST_F(DiagnosticsReporterTest, ReportEnabled) {
- EXPECT_CALL(reporter_, IsReportingEnabled()).WillOnce(Return(true));
- EXPECT_CALL(glib_, SpawnSync(_, _, _, _, _, _, _, _, _, _)).Times(1);
- reporter_.Report();
-}
-
-TEST_F(DiagnosticsReporterTest, ReportDisabled) {
- EXPECT_CALL(reporter_, IsReportingEnabled()).WillOnce(Return(false));
- EXPECT_CALL(glib_, SpawnSync(_, _, _, _, _, _, _, _, _, _)).Times(0);
- reporter_.Report();
-}
-
TEST_F(DiagnosticsReporterTest, IsReportingEnabled) {
EXPECT_FALSE(IsReportingEnabled());
}
@@ -85,18 +72,46 @@
EXPECT_EQ(kLastStash, GetLastLogStash());
}
+namespace {
+
+MATCHER(NoUpload, "") {
+ return arg && arg[0] && !arg[1];
+}
+
+MATCHER(DoUpload, "") {
+ return arg && arg[0] && arg[1] && !strcmp(arg[1], "--upload");
+}
+
+} // namespace
+
TEST_F(DiagnosticsReporterTest, OnConnectivityEvent) {
const uint64 kLastStash = 50;
- const uint64 kNow = kLastStash + GetLogStashThrottleSeconds() + 1;
SetLastLogStash(kLastStash);
- const struct timeval now = {
- .tv_sec = static_cast<long int>(kNow),
+ const uint64 kNow0 = kLastStash + GetLogStashThrottleSeconds() + 1;
+ const struct timeval now0 = {
+ .tv_sec = static_cast<long int>(kNow0),
+ .tv_usec = 0
+ };
+ const uint64 kNow1 = kNow0 + GetLogStashThrottleSeconds() + 1;
+ const struct timeval now1 = {
+ .tv_sec = static_cast<long int>(kNow1),
.tv_usec = 0
};
EXPECT_CALL(time_, GetTimeMonotonic(_))
- .WillOnce(DoAll(SetArgumentPointee<0>(now), Return(0)));
+ .WillOnce(DoAll(SetArgumentPointee<0>(now0), Return(0)))
+ .WillOnce(DoAll(SetArgumentPointee<0>(now1), Return(0)));
+ EXPECT_CALL(reporter_, IsReportingEnabled())
+ .WillOnce(Return(false))
+ .WillOnce(Return(true));
+ InSequence s;
+ EXPECT_CALL(glib_, SpawnAsync(_, NoUpload(), _, _, _, _, _, _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(glib_, SpawnAsync(_, DoUpload(), _, _, _, _, _, _))
+ .WillOnce(Return(true));
reporter_.OnConnectivityEvent();
- EXPECT_EQ(kNow, GetLastLogStash());
+ EXPECT_EQ(kNow0, GetLastLogStash());
+ reporter_.OnConnectivityEvent();
+ EXPECT_EQ(kNow1, GetLastLogStash());
}
} // namespace shill
diff --git a/shims/net_diags_upload.cc b/shims/net_diags_upload.cc
index a7cc2f2..c26dbe9 100644
--- a/shims/net_diags_upload.cc
+++ b/shims/net_diags_upload.cc
@@ -4,6 +4,60 @@
#include <stdlib.h>
+#include <base/at_exit.h>
+#include <base/command_line.h>
+#include <base/logging.h>
+#include <chromeos/syslog_logging.h>
+
+namespace switches {
+
+static const char kHelp[] = "help";
+static const char kUpload[] = "upload";
+
+// The help message shown if help flag is passed to the program.
+static const char kHelpMessage[] = "\n"
+ "Available Switches:\n"
+ " --help\n"
+ " Show this help message.\n"
+ " --upload\n"
+ " Upload the diagnostics logs.\n";
+
+} // namespace switches
+
+namespace shill {
+
+void StashLogs() {
+ // Captures the last 10000 lines in the log regardless of log rotation. I.e.,
+ // prints the log files in timestamp sorted order and gets the tail of the
+ // output.
+ if (system("/bin/cat $(/bin/ls -rt /var/log/net.*log) | /bin/tail -10000 > "
+ "/var/log/net-diags.net.log")) {
+ LOG(ERROR) << "Unable to stash net.log.";
+ } else {
+ LOG(INFO) << "net.log stashed.";
+ }
+}
+
+} // namespace shill
+
int main(int argc, char **argv) {
- abort();
+ base::AtExitManager exit_manager;
+ CommandLine::Init(argc, argv);
+ CommandLine* cl = CommandLine::ForCurrentProcess();
+
+ if (cl->HasSwitch(switches::kHelp)) {
+ LOG(INFO) << switches::kHelpMessage;
+ return 0;
+ }
+
+ chromeos::InitLog(chromeos::kLogToSyslog | chromeos::kLogHeader);
+
+ shill::StashLogs();
+
+ if (cl->HasSwitch(switches::kUpload)) {
+ // Crash so that crash_reporter can upload the logs.
+ abort();
+ }
+
+ return 0;
}