Add check_format for instantiating time-sources in a new source file …

…(tests excluded).

Signed-off-by: Joshua Marantz <jmarantz@google.com>

tools/check_format.py

@@ -23,6 +23,15 @@
 GOOGLE_PROTOBUF_WHITELIST = ("ci/prebuilt", "source/common/protobuf", "api/test")
 REPOSITORIES_BZL = "bazel/repositories.bzl"
 
+# Files matching these exact names can reference prod time. These include the class
+# definitions for prod time, the construction of them in main(), and perf annotation.
+# For now it includes the validation server but that really should be injected too.
+PROD_TIME_WHITELIST = ('./source/common/common/utility.h',
+                       './source/exe/main_common.cc',
+                       './source/exe/main_common.h',
+                       './source/server/config_validation/server.cc',
+                       './source/common/common/perf_annotation.h')
+
 CLANG_FORMAT_PATH = os.getenv("CLANG_FORMAT", "clang-format-6.0")
 BUILDIFIER_PATH = os.getenv("BUILDIFIER_BIN", "$GOPATH/bin/buildifier")
 ENVOY_BUILD_FIXER_PATH = os.path.join(
@@ -65,6 +74,12 @@ def whitelistedForProtobufDeps(file_path):
   return (file_path.endswith(PROTO_SUFFIX) or file_path.endswith(REPOSITORIES_BZL) or \
           any(path_segment in file_path for path_segment in GOOGLE_PROTOBUF_WHITELIST))
 
+# Production time sources should not be instantiated in the source, except for a few
+# specific cases. They should be passed down from where they are instantied to where
+# they need to be used, e.g. through the ServerInstance, Dispatcher, or ClusterManager.
+def whitelistedForProdTime(file_path):
+  return file_path in PROD_TIME_WHITELIST or file_path.startswith('./test/')
+
 def findSubstringAndReturnError(pattern, file_path, error_message):
   with open(file_path) as f:
     text = f.read()
@@ -145,6 +160,9 @@ def checkSourceLine(line, file_path, reportError):
     # comments, for example this one.
     reportError("Don't use <mutex> or <condition_variable*>, switch to "
                 "Thread::MutexBasicLockable in source/common/common/thread.h")
+  if not whitelistedForProdTime(file_path):
+    if 'ProdSystemTimeSource' in line or 'ProdMonotonicTimeSource' in line:
+      reportError("Don't reference real-time sources from production code; use injection")
 
 def checkBuildLine(line, file_path, reportError):
   if not whitelistedForProtobufDeps(file_path) and '"protobuf"' in line:

tools/check_format_test_helper.py

@@ -143,6 +143,10 @@ def checkFileExpectingOK(filename):
                                     "Don't use <mutex> or <condition_variable*>")
   errors += fixFileExpectingFailure("condition_variable_any.cc",
                                     "Don't use <mutex> or <condition_variable*>")
+  errors += fixFileExpectingFailure(
+      "prod_system_time.cc", "Don't reference real-time sources from production code; use injection")
+  errors += fixFileExpectingFailure(
+      "prod_monotonic_time.cc", "Don't reference real-time sources from production code; use injection")
 
   errors += fixFileExpectingNoChange("ok_file.cc")
 
@@ -163,6 +167,10 @@ def checkFileExpectingOK(filename):
   errors += checkFileExpectingError("bad_envoy_build_sys_ref.BUILD",
                                     "Superfluous '@envoy//' prefix")
   errors += checkFileExpectingError("proto_format.proto", "clang-format check failed")
+  errors += checkFileExpectingError(
+      "prod_system_time.cc", "Don't reference real-time sources from production code; use injection")
+  errors += checkFileExpectingError(
+      "prod_monotonic_time.cc", "Don't reference real-time sources from production code; use injection")
 
   errors += checkFileExpectingOK("ok_file.cc")
 

tools/testdata/check_format/prod_monotonic_time.cc

@@ -0,0 +1,4 @@
+int foo() {
+  ProdSystemTimeSource system_time;
+  ProdMonotonicTimeSource monotonic_time;
+}

tools/testdata/check_format/prod_system_time.cc

@@ -0,0 +1,3 @@
+int foo() {
+  ProdMonotonicTimeSource monotonic_time;
+}