[bugreport] --minimal to restrict the attachments to inspect.json
* if run from the target shell, the output of `bugreport` can be quite large
(logs, inspect, etc.). `bugreport --minimal` preserves the annotations and
the Inspect data, but skips the logs and build snapshot, which can be quite large.
* we add support for command-line option parsing and its own unit test
DX-1678 #done #comment
TESTED=`fx run-test bugreport_tests`
TESTED=`fx bugreport`
TESTED=`bugreport --minimal`
TESTED=`bugreport --help`
TESTED=`bugreport help`
Change-Id: Ibbdfd57fd6149286f218de0cdda87dc2c271369c
diff --git a/src/developer/bugreport/BUILD.gn b/src/developer/bugreport/BUILD.gn
index 5e41232..c6f6908d 100644
--- a/src/developer/bugreport/BUILD.gn
+++ b/src/developer/bugreport/BUILD.gn
@@ -34,11 +34,29 @@
]
deps = [
+ ":command_line_options",
":src",
"//sdk/lib/sys/cpp",
]
}
+source_set("bug_report_schema") {
+ sources = [
+ "bug_report_schema.h",
+ ]
+}
+
+source_set("command_line_options") {
+ sources = [
+ "command_line_options.cc",
+ "command_line_options.h",
+ ]
+
+ deps = [
+ "//src/lib/fxl",
+ ]
+}
+
source_set("src") {
sources = [
"bug_reporter.cc",
@@ -58,12 +76,6 @@
]
}
-source_set("bug_report_schema") {
- sources = [
- "bug_report_schema.h",
- ]
-}
-
# Host side client -------------------------------------------------------------
if (current_toolchain == host_toolchain) {
diff --git a/src/developer/bugreport/bug_reporter.cc b/src/developer/bugreport/bug_reporter.cc
index c907ea1..1c05d2b 100644
--- a/src/developer/bugreport/bug_reporter.cc
+++ b/src/developer/bugreport/bug_reporter.cc
@@ -10,6 +10,8 @@
#include <zircon/errors.h>
#include <zircon/status.h>
+#include <vector>
+
#include "src/developer/bugreport/bug_report_schema.h"
#include "third_party/rapidjson/include/rapidjson/document.h"
#include "third_party/rapidjson/include/rapidjson/filewritestream.h"
@@ -33,11 +35,16 @@
void AddAttachments(
const std::vector<fuchsia::feedback::Attachment>& attachments,
+ const std::set<std::string>& attachment_allowlist,
rapidjson::Document* document) {
rapidjson::Value json_attachments(rapidjson::kObjectType);
for (const auto& attachment : attachments) {
- // TODO(DX-1551): interpret the attachment value as a JSON object for
- // the "inspect" key.
+ if (!attachment_allowlist.empty() &&
+ attachment_allowlist.find(attachment.key) ==
+ attachment_allowlist.end()) {
+ continue;
+ }
+
std::string value;
if (!fsl::StringFromVmo(attachment.value, &value)) {
fprintf(stderr, "Failed to parse attachment VMO as string for key %s\n",
@@ -52,11 +59,12 @@
}
bool MakeAndWriteJson(const fuchsia::feedback::Data& feedback_data,
+ const std::set<std::string>& attachment_allowlist,
FILE* out_file) {
rapidjson::Document document;
document.SetObject();
AddAnnotations(feedback_data.annotations(), &document);
- AddAttachments(feedback_data.attachments(), &document);
+ AddAttachments(feedback_data.attachments(), attachment_allowlist, &document);
char buffer[65536];
rapidjson::FileWriteStream output_stream(out_file, buffer, sizeof(buffer));
@@ -73,6 +81,7 @@
} // namespace
bool MakeBugReport(std::shared_ptr<::sys::ServiceDirectory> services,
+ const std::set<std::string>& attachment_allowlist,
const char* out_filename) {
fuchsia::feedback::DataProviderSyncPtr feedback_data_provider;
services->Connect(feedback_data_provider.NewRequest());
@@ -99,11 +108,13 @@
fprintf(stderr, "Failed to open output file %s\n", out_filename);
return false;
}
- const bool success = MakeAndWriteJson(result.response().data, out_file);
+ const bool success = MakeAndWriteJson(result.response().data,
+ attachment_allowlist, out_file);
fclose(out_file);
return success;
} else {
- return MakeAndWriteJson(result.response().data, stdout);
+ return MakeAndWriteJson(result.response().data, attachment_allowlist,
+ stdout);
}
}
diff --git a/src/developer/bugreport/bug_reporter.h b/src/developer/bugreport/bug_reporter.h
index 7e8505f..46aad0b 100644
--- a/src/developer/bugreport/bug_reporter.h
+++ b/src/developer/bugreport/bug_reporter.h
@@ -8,16 +8,23 @@
#include <lib/sys/cpp/service_directory.h>
#include <memory>
+#include <set>
+#include <string>
namespace fuchsia {
namespace bugreport {
-// Makes a JSON file representing a bug report containing all the feedback data
-// by connecting to fuchsia.feedback.DataProvider from |services|.
+// Makes a JSON file representing a bug report containing the feedback data
+// (annotations and attachments) collected from fuchsia.feedback.DataProvider
+// from |services|.
//
-// By default, the JSON file is streamed to stdout. Use |out_filename| to output
-// it to a file.
+// By default:
+// - all the attachments are reported. Use |attachment_allowlist| to restrict
+// the attachments to specific keys, e.g., to minimize the output.
+// - the JSON file is streamed to stdout. Use |out_filename| to output it to
+// a file.
bool MakeBugReport(std::shared_ptr<::sys::ServiceDirectory> services,
+ const std::set<std::string>& attachment_allowlist = {},
const char* out_filename = nullptr);
} // namespace bugreport
diff --git a/src/developer/bugreport/command_line_options.cc b/src/developer/bugreport/command_line_options.cc
new file mode 100644
index 0000000..8019b36
--- /dev/null
+++ b/src/developer/bugreport/command_line_options.cc
@@ -0,0 +1,53 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/developer/bugreport/command_line_options.h"
+
+#include "src/lib/fxl/command_line.h"
+#include "src/lib/fxl/strings/substitute.h"
+
+namespace fuchsia {
+namespace bugreport {
+namespace {
+
+constexpr char kUsage[] = R"($0
+
+ Dumps in stdout a JSON file containing the feedback data (annotations and
+ attachments) collected from fuchsia.feedback.DataProvider.
+
+Usage:
+
+ $0 [--minimal]
+
+Arguments:
+
+ --minimal Restricts the attachments to the Inspect data only (no logs,
+ no build snapshot, etc.). Annotations are preserved.
+
+)";
+
+} // namespace
+
+Mode ParseModeFromArgcArgv(int argc, const char* const* argv) {
+ auto command_line = fxl::CommandLineFromArgcArgv(argc, argv);
+ auto& pos_args = command_line.positional_args();
+ if (command_line.HasOption("help") ||
+ (pos_args.size() == 1u && pos_args[0] == "help")) {
+ printf("%s\n", fxl::Substitute(kUsage, command_line.argv0()).c_str());
+ return Mode::HELP;
+ }
+
+ if (command_line.HasOption("minimal")) {
+ return Mode::MINIMAL;
+ } else if (!command_line.options().empty() || !pos_args.empty()) {
+ fprintf(stderr, "Unexpected option. Usage: %s\n",
+ fxl::Substitute(kUsage, command_line.argv0()).c_str());
+ return Mode::FAILURE;
+ }
+
+ return Mode::DEFAULT;
+}
+
+} // namespace bugreport
+} // namespace fuchsia
diff --git a/src/developer/bugreport/command_line_options.h b/src/developer/bugreport/command_line_options.h
new file mode 100644
index 0000000..700a099
--- /dev/null
+++ b/src/developer/bugreport/command_line_options.h
@@ -0,0 +1,23 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SRC_DEVELOPER_BUGREPORT_COMMAND_LINE_OPTIONS_H_
+#define SRC_DEVELOPER_BUGREPORT_COMMAND_LINE_OPTIONS_H_
+
+namespace fuchsia {
+namespace bugreport {
+
+enum class Mode {
+ FAILURE,
+ HELP,
+ DEFAULT,
+ MINIMAL,
+};
+
+Mode ParseModeFromArgcArgv(int argc, const char* const* argv);
+
+} // namespace bugreport
+} // namespace fuchsia
+
+#endif // SRC_DEVELOPER_BUGREPORT_COMMAND_LINE_OPTIONS_H_
diff --git a/src/developer/bugreport/main.cc b/src/developer/bugreport/main.cc
index f4eb10a..574e153 100644
--- a/src/developer/bugreport/main.cc
+++ b/src/developer/bugreport/main.cc
@@ -3,13 +3,38 @@
// found in the LICENSE file.
#include <lib/sys/cpp/component_context.h>
+#include <stdio.h>
#include <stdlib.h>
+#include <set>
+#include <string>
+
#include "src/developer/bugreport/bug_reporter.h"
+#include "src/developer/bugreport/command_line_options.h"
+
+namespace {
+
+using fuchsia::bugreport::Mode;
+
+} // namespace
int main(int argc, char** argv) {
- auto environment_services = ::sys::ServiceDirectory::CreateFromNamespace();
+ const Mode mode = fuchsia::bugreport::ParseModeFromArgcArgv(argc, argv);
+ std::set<std::string> attachment_allowlist;
+ switch (mode) {
+ case Mode::FAILURE:
+ return EXIT_FAILURE;
+ case Mode::HELP:
+ return EXIT_SUCCESS;
+ case Mode::MINIMAL:
+ attachment_allowlist = {"inspect.json"};
+ case Mode::DEFAULT:
+ auto environment_services =
+ ::sys::ServiceDirectory::CreateFromNamespace();
- return fuchsia::bugreport::MakeBugReport(environment_services) ? EXIT_SUCCESS
- : EXIT_FAILURE;
+ return fuchsia::bugreport::MakeBugReport(environment_services,
+ attachment_allowlist)
+ ? EXIT_SUCCESS
+ : EXIT_FAILURE;
+ }
}
diff --git a/src/developer/bugreport/tests/BUILD.gn b/src/developer/bugreport/tests/BUILD.gn
index c291ad7..8c3dbd4 100644
--- a/src/developer/bugreport/tests/BUILD.gn
+++ b/src/developer/bugreport/tests/BUILD.gn
@@ -13,11 +13,15 @@
{
name = "bugreport_integration_test"
},
+ {
+ name = "command_line_options_unittest"
+ },
]
deps = [
":bugreport_integration_test",
":bugreport_unittest",
+ ":command_line_options_unittest",
]
}
@@ -65,6 +69,20 @@
]
}
+executable("command_line_options_unittest") {
+ testonly = true
+
+ sources = [
+ "command_line_options_unittest.cc",
+ ]
+
+ deps = [
+ "//src/developer/bugreport:command_line_options",
+ "//src/lib/fxl/test:gtest_main",
+ "//third_party/googletest:gtest",
+ ]
+}
+
if (current_toolchain == host_toolchain) {
group("bugreport_client_tests") {
testonly = true
diff --git a/src/developer/bugreport/tests/bug_reporter_integration_test.cc b/src/developer/bugreport/tests/bug_reporter_integration_test.cc
index 4647d1d..66ba8e8 100644
--- a/src/developer/bugreport/tests/bug_reporter_integration_test.cc
+++ b/src/developer/bugreport/tests/bug_reporter_integration_test.cc
@@ -32,7 +32,8 @@
};
TEST_F(BugReporterIntegrationTest, SmokeTest) {
- ASSERT_TRUE(MakeBugReport(environment_services_, json_path_.data()));
+ ASSERT_TRUE(MakeBugReport(environment_services_,
+ /*attachment_allowlist=*/{}, json_path_.data()));
std::string output;
ASSERT_TRUE(files::ReadFileToString(json_path_, &output));
diff --git a/src/developer/bugreport/tests/bug_reporter_unittest.cc b/src/developer/bugreport/tests/bug_reporter_unittest.cc
index 45e7c52..0bbb808 100644
--- a/src/developer/bugreport/tests/bug_reporter_unittest.cc
+++ b/src/developer/bugreport/tests/bug_reporter_unittest.cc
@@ -63,7 +63,7 @@
files::ScopedTempDir tmp_dir_;
};
-TEST_F(BugReporterTest, SmokeTest) {
+TEST_F(BugReporterTest, Basic) {
ResetFeedbackDataProvider(/*annotations=*/
{
{"annotation.1.key", "annotation.1.value"},
@@ -76,7 +76,7 @@
});
ASSERT_TRUE(MakeBugReport(service_directory_provider_.service_directory(),
- json_path_.data()));
+ /*attachment_allowlist=*/{}, json_path_.data()));
std::string output;
ASSERT_TRUE(files::ReadFileToString(json_path_, &output));
@@ -117,6 +117,34 @@
"attachment.2.value");
}
+TEST_F(BugReporterTest, RestrictedAttachments) {
+ ResetFeedbackDataProvider(/*annotations=*/
+ {
+ {"annotation.key", "unused"},
+ },
+ /*attachments=*/{
+ {"attachment.key.filtered", "unused"},
+ {"attachment.key.kept", "unused"},
+ });
+
+ ASSERT_TRUE(MakeBugReport(service_directory_provider_.service_directory(),
+ /*attachment_allowlist=*/
+ {
+ "attachment.key.kept",
+ "attachment.key.ignored",
+ },
+ json_path_.data()));
+
+ std::string output;
+ ASSERT_TRUE(files::ReadFileToString(json_path_, &output));
+ rapidjson::Document document;
+ ASSERT_FALSE(document.Parse(output.c_str()).HasParseError());
+ ASSERT_TRUE(document.HasMember("attachments"));
+ ASSERT_FALSE(document["attachments"].HasMember("attachment.key.filtered"));
+ ASSERT_TRUE(document["attachments"].HasMember("attachment.key.kept"));
+ ASSERT_FALSE(document["attachments"].HasMember("attachment.key.ignored"));
+}
+
} // namespace
} // namespace bugreport
} // namespace fuchsia
diff --git a/src/developer/bugreport/tests/command_line_options_unittest.cc b/src/developer/bugreport/tests/command_line_options_unittest.cc
new file mode 100644
index 0000000..93f569b
--- /dev/null
+++ b/src/developer/bugreport/tests/command_line_options_unittest.cc
@@ -0,0 +1,75 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/developer/bugreport/command_line_options.h"
+
+#include "third_party/googletest/googletest/include/gtest/gtest.h"
+
+namespace fuchsia {
+namespace bugreport {
+namespace {
+
+TEST(CommandLineOptionsTest, Default) {
+ const char* argv[] = {"bugreport"};
+ EXPECT_EQ(ParseModeFromArgcArgv(1, argv), Mode::DEFAULT);
+}
+
+TEST(CommandLineOptionsTest, Minimal) {
+ const char* argv[] = {"bugreport", "--minimal"};
+ EXPECT_EQ(ParseModeFromArgcArgv(2, argv), Mode::MINIMAL);
+}
+
+TEST(CommandLineOptionsTest, Help) {
+ const char* argv[] = {"bugreport", "--help"};
+ EXPECT_EQ(ParseModeFromArgcArgv(2, argv), Mode::HELP);
+}
+
+TEST(CommandLineOptionsTest, HelpAnywhere) {
+ const char* argv[] = {"bugreport", "--irrelevant", "--help"};
+ EXPECT_EQ(ParseModeFromArgcArgv(3, argv), Mode::HELP);
+}
+
+TEST(CommandLineOptionsTest, HelpAsPositionalArgument) {
+ const char* argv[] = {"bugreport", "help"};
+ EXPECT_EQ(ParseModeFromArgcArgv(2, argv), Mode::HELP);
+}
+
+TEST(CommandLineOptionsTest, HelpAsPositionalArgumentAnywhere) {
+ const char* argv[] = {"bugreport", "--irrelevant", "help"};
+ EXPECT_EQ(ParseModeFromArgcArgv(3, argv), Mode::HELP);
+}
+
+TEST(CommandLineOptionsTest, FailureUnknownOption) {
+ const char* argv[] = {"bugreport", "--unknown"};
+ EXPECT_EQ(ParseModeFromArgcArgv(2, argv), Mode::FAILURE);
+}
+
+TEST(CommandLineOptionsTest, FailureUnknownPositionalArgument) {
+ const char* argv[] = {"bugreport", "unknown"};
+ EXPECT_EQ(ParseModeFromArgcArgv(2, argv), Mode::FAILURE);
+}
+
+} // namespace
+
+// Pretty-prints Mode in gTest matchers instead of the default byte string in
+// case of failed expectations.
+void PrintTo(const Mode& mode, std::ostream* os) {
+ switch (mode) {
+ case Mode::FAILURE:
+ *os << "FAILURE";
+ return;
+ case Mode::HELP:
+ *os << "HELP";
+ return;
+ case Mode::MINIMAL:
+ *os << "MINIMAL";
+ return;
+ case Mode::DEFAULT:
+ *os << "DEFAULT";
+ return;
+ }
+}
+
+} // namespace bugreport
+} // namespace fuchsia
diff --git a/src/developer/bugreport/tests/meta/command_line_options_unittest.cmx b/src/developer/bugreport/tests/meta/command_line_options_unittest.cmx
new file mode 100644
index 0000000..b879612
--- /dev/null
+++ b/src/developer/bugreport/tests/meta/command_line_options_unittest.cmx
@@ -0,0 +1,5 @@
+{
+ "program": {
+ "binary": "test/command_line_options_unittest"
+ }
+}
