Better diagnostics for unrecognized trace format

fdbclient/NativeAPI.actor.cpp

@@ -799,6 +799,10 @@ void setNetworkOption(FDBNetworkOptions::Option option, Optional<StringRef> valu
 		case FDBNetworkOptions::TRACE_FORMAT:
 			validateOptionValue(value, true);
 			networkOptions.traceFormat = value.get().toString();
+			if (!validateTraceFormat(networkOptions.traceFormat)) {
+				fprintf(stderr, "Unrecognized trace format: `%s'\n", networkOptions.traceFormat.c_str());
+				throw invalid_option_value();
+			}
 			break;
 		case FDBNetworkOptions::KNOB: {
 			validateOptionValue(value, true);

flow/Trace.cpp

@@ -43,18 +43,6 @@
 #undef min
 #endif
 
-namespace {
-	Reference<ITraceLogFormatter> createLogFormatter(const std::string& f) {
-		if (f == "json") {
-			return Reference<ITraceLogFormatter>(new JsonTraceLogFormatter());
-		} else if (f == "xml") {
-			return Reference<ITraceLogFormatter>(new XmlTraceLogFormatter());
-		} else {
-			UNREACHABLE();
-		}
-	}
-} // namespace
-
 class DummyThreadPool : public IThreadPool, ReferenceCounted<DummyThreadPool> {
 public:
 	~DummyThreadPool() {}
@@ -576,16 +564,41 @@ TraceEventFields LatestEventCache::getLatestError() {
 
 static TraceLog g_traceLog;
 
-bool selectTraceFormatter(std::string format) {
-	ASSERT(!g_traceLog.isOpen());
+namespace {
+template <bool validate>
+bool traceFormatImpl(std::string& format) {
 	std::transform(format.begin(), format.end(), format.begin(), ::tolower);
-	if (format == "xml" || format == "json") {
-		g_traceLog.formatter = createLogFormatter(format);
+	if (format == "xml") {
+		if (!validate) {
+			g_traceLog.formatter = Reference<ITraceLogFormatter>(new XmlTraceLogFormatter());
+		}
+		return true;
+	} else if (format == "json") {
+		if (!validate) {
+			g_traceLog.formatter = Reference<ITraceLogFormatter>(new JsonTraceLogFormatter());
+		}
 		return true;
 	} else {
+		if (!validate) {
+			g_traceLog.formatter = Reference<ITraceLogFormatter>(new XmlTraceLogFormatter());
+		}
 		return false;
 	}
 }
+} // namespace
+
+bool selectTraceFormatter(std::string format) {
+	ASSERT(!g_traceLog.isOpen());
+	bool recognized = traceFormatImpl</*validate*/ false>(format);
+	if (!recognized) {
+		TraceEvent(SevWarnAlways, "UnrecognizedTraceFormat").detail("format", format);
+	}
+	return recognized;
+}
+
+bool validateTraceFormat(std::string format) {
+	return traceFormatImpl</*validate*/ true>(format);
+}
 
 ThreadFuture<Void> flushTraceFile() {
 	if (!g_traceLog.isOpen())

flow/Trace.h

@@ -265,6 +265,8 @@ bool traceFileIsOpen();
 // Changes the format of trace files. Returns false if the format is unrecognized. No longer safe to call after a call
 // to openTraceFile.
 bool selectTraceFormatter(std::string format);
+// Returns true iff format is recognized.
+bool validateTraceFormat(std::string format);
 
 void addTraceRole(std::string role);
 void removeTraceRole(std::string role);