merge release-1.1 to master

Makefile

@@ -20,6 +20,9 @@ ARTIFACTS_DIR ?= $(LOCAL_ARTIFACTS_DIR)
 BAZEL_STARTUP_ARGS ?=
 BAZEL_BUILD_ARGS ?=
 BAZEL_TEST_ARGS ?=
+BAZEL_TARGETS ?= //...
+# Some tests run so slowly under the santizers that they always timeout.
+SANITIZER_EXCLUSIONS ?= -test/integration:mixer_fault_test
 HUB ?=
 TAG ?=
 ifeq "$(origin CC)" "default"
@@ -31,7 +34,7 @@ endif
 PATH := /usr/lib/llvm-7/bin:$(PATH)
 
 build:
-	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) //...
+	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) build $(BAZEL_BUILD_ARGS) $(BAZEL_TARGETS)
 	@bazel shutdown
 
 # Build only envoy - fast
@@ -44,15 +47,15 @@ clean:
 	@bazel shutdown
 
 test:
-	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) //...
+	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) $(BAZEL_TARGETS)
 	@bazel shutdown
 
 test_asan:
-	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-asan //...
+	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-asan -- $(BAZEL_TARGETS) $(SANITIZER_EXCLUSIONS)
 	@bazel shutdown
 
 test_tsan:
-	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-tsan //...
+	PATH=$(PATH) CC=$(CC) CXX=$(CXX) bazel $(BAZEL_STARTUP_ARGS) test $(BAZEL_TEST_ARGS) --config=clang-tsan -- $(BAZEL_TARGETS) $(SANITIZER_EXCLUSIONS)
 	@bazel shutdown
 
 check:

OWNERS

@@ -7,6 +7,7 @@ reviewers:
   - JimmyCYJ
   - venilnoronha
   - kyessenov
+  - duderino
 approvers:
   - qiwzhang
   - lizan
@@ -16,3 +17,4 @@ approvers:
   - JimmyCYJ
   - venilnoronha
   - kyessenov
+  - duderino

include/istio/control/http/request_handler.h

@@ -35,10 +35,13 @@ class RequestHandler {
   // * extract attributes from the config.
   // * if necessary, forward some attributes to downstream
   // * make a Check call.
-  virtual ::istio::mixerclient::CancelFunc Check(
-      CheckData* check_data, HeaderUpdate* header_update,
-      ::istio::mixerclient::TransportCheckFunc transport,
-      ::istio::mixerclient::CheckDoneFunc on_done) = 0;
+  virtual void Check(CheckData* check_data, HeaderUpdate* header_update,
+                     const ::istio::mixerclient::TransportCheckFunc& transport,
+                     const ::istio::mixerclient::CheckDoneFunc& on_done) = 0;
+
+  virtual void ResetCancel() = 0;
+
+  virtual void CancelCheck() = 0;
 
   // Make a Report call. It will:
   // * check service config to see if Report is required

include/istio/control/tcp/request_handler.h

@@ -32,8 +32,12 @@ class RequestHandler {
   // Perform a Check call. It will:
   // * extract downstream tcp connection attributes
   // * check config, make a Check call if necessary.
-  virtual ::istio::mixerclient::CancelFunc Check(
-      CheckData* check_data, ::istio::mixerclient::CheckDoneFunc on_done) = 0;
+  virtual void Check(CheckData* check_data,
+                     const ::istio::mixerclient::CheckDoneFunc& on_done) = 0;
+
+  virtual void ResetCancel() = 0;
+
+  virtual void CancelCheck() = 0;
 
   // Make report call.
   virtual void Report(ReportData* report_data,

include/istio/mixerclient/check_response.h

@@ -22,21 +22,15 @@
 namespace istio {
 namespace mixerclient {
 
-// The CheckResponseInfo holds response information in detail.
-struct CheckResponseInfo {
-  // Whether this check response is from cache.
-  bool is_check_cache_hit{false};
+// The CheckResponseInfo exposes policy and quota check details to the check
+// callbacks.
+class CheckResponseInfo {
+ public:
+  virtual ~CheckResponseInfo(){};
 
-  // Whether this quota response is from cache.
-  bool is_quota_cache_hit{false};
+  virtual const ::google::protobuf::util::Status& status() const = 0;
 
-  // The check and quota response status.
-  ::google::protobuf::util::Status response_status{
-      ::google::protobuf::util::Status::UNKNOWN};
-
-  // Routing directive (applicable if the status is OK)
-  ::istio::mixer::v1::RouteDirective route_directive{
-      ::istio::mixer::v1::RouteDirective::default_instance()};
+  virtual const ::istio::mixer::v1::RouteDirective& routeDirective() const = 0;
 };
 
 }  // namespace mixerclient

include/istio/mixerclient/client.h

@@ -19,6 +19,8 @@
 #include "environment.h"
 #include "include/istio/quota_config/requirement.h"
 #include "options.h"
+#include "src/istio/mixerclient/check_context.h"
+#include "src/istio/mixerclient/shared_attributes.h"
 
 #include <vector>
 
@@ -50,24 +52,79 @@ struct MixerClientOptions {
 
 // The statistics recorded by mixerclient library.
 struct Statistics {
-  // Total number of check calls.
-  uint64_t total_check_calls;
-  // Total number of remote check calls.
-  uint64_t total_remote_check_calls;
-  // Total number of remote check calls that blocking origin requests.
-  uint64_t total_blocking_remote_check_calls;
-
-  // Total number of quota calls.
-  uint64_t total_quota_calls;
-  // Total number of remote quota calls.
-  uint64_t total_remote_quota_calls;
-  // Total number of remote quota calls that blocking origin requests.
-  uint64_t total_blocking_remote_quota_calls;
+  //
+  // Policy check counters.
+  //
+  // total_check_calls = total_check_hits + total_check_misses
+  // total_check_hits = total_check_hit_accepts + total_check_hit_denies
+  // total_remote_check_calls = total_check_misses
+  // total_remote_check_calls >= total_remote_check_accepts +
+  // total_remote_check_denies
+  //    ^ Transport errors are responsible for the >=
+  //
+
+  uint64_t total_check_calls_{0};              // 1.0
+  uint64_t total_check_cache_hits_{0};         // 1.1
+  uint64_t total_check_cache_misses_{0};       // 1.1
+  uint64_t total_check_cache_hit_accepts_{0};  // 1.1
+  uint64_t total_check_cache_hit_denies_{0};   // 1.1
+  uint64_t total_remote_check_calls_{0};       // 1.0
+  uint64_t total_remote_check_accepts_{0};     // 1.1
+  uint64_t total_remote_check_denies_{0};      // 1.1
+
+  //
+  // Quota check counters
+  //
+  // total_quota_calls = total_quota_hits + total_quota_misses
+  // total_quota_hits = total_quota_hit_accepts + total_quota_hit_denies
+  // total_remote_quota_calls = total_quota_misses +
+  // total_remote_quota_prefetch_calls total_remote_quota_calls >=
+  // total_remote_quota_accepts + total_remote_quota_denies
+  //    ^ Transport errors are responsible for the >=
+  //
+
+  uint64_t total_quota_calls_{0};                  // 1.0
+  uint64_t total_quota_cache_hits_{0};             // 1.1
+  uint64_t total_quota_cache_misses_{0};           // 1.1
+  uint64_t total_quota_cache_hit_accepts_{0};      // 1.1
+  uint64_t total_quota_cache_hit_denies_{0};       // 1.1
+  uint64_t total_remote_quota_calls_{0};           // 1.0
+  uint64_t total_remote_quota_accepts_{0};         // 1.1
+  uint64_t total_remote_quota_denies_{0};          // 1.1
+  uint64_t total_remote_quota_prefetch_calls_{0};  // 1.1
+
+  //
+  // Counters for upstream requests to Mixer.
+  //
+  // total_remote_calls = SUM(total_remote_call_successes, ...,
+  // total_remote_call_other_errors) Total transport errors would be
+  // (total_remote_calls - total_remote_call_successes).
+  //
+
+  uint64_t total_remote_calls_{0};               // 1.1
+  uint64_t total_remote_call_successes_{0};      // 1.1
+  uint64_t total_remote_call_timeouts_{0};       // 1.1
+  uint64_t total_remote_call_send_errors_{0};    // 1.1
+  uint64_t total_remote_call_other_errors_{0};   // 1.1
+  uint64_t total_remote_call_retries_{0};        // 1.1
+  uint64_t total_remote_call_cancellations_{0};  // 1.1
+
+  //
+  // Telemetry report counters
+  //
 
   // Total number of report calls.
-  uint64_t total_report_calls;
+  uint64_t total_report_calls_{0};  // 1.0
   // Total number of remote report calls.
-  uint64_t total_remote_report_calls;
+  uint64_t total_remote_report_calls_{0};  // 1.0
+  // Remote report calls that succeeed
+  uint64_t total_remote_report_successes_{0};  // 1.1
+  // Remote report calls that fail due to timeout waiting for the response
+  uint64_t total_remote_report_timeouts_{0};  // 1.1
+  // Remote report calls that fail sending the request (socket connect or write)
+  uint64_t total_remote_report_send_errors_{0};  // 1.1
+  // Remote report calls that fail do to some other error
+  uint64_t total_remote_report_other_errors_{0};  // 1.1
 };
 
 class MixerClient {
@@ -84,13 +141,13 @@ class MixerClient {
   // The response data from mixer will be consumed by mixer client.
 
   // A check call.
-  virtual CancelFunc Check(
-      const ::istio::mixer::v1::Attributes& attributes,
-      const std::vector<::istio::quota_config::Requirement>& quotas,
-      TransportCheckFunc transport, CheckDoneFunc on_done) = 0;
+  virtual void Check(istio::mixerclient::CheckContextSharedPtr& context,
+                     const TransportCheckFunc& transport,
+                     const CheckDoneFunc& on_done) = 0;
 
   // A report call.
-  virtual void Report(const ::istio::mixer::v1::Attributes& attributes) = 0;
+  virtual void Report(
+      const istio::mixerclient::SharedAttributesSharedPtr& attributes) = 0;
 
   // Get statistics.
   virtual void GetStatistics(Statistics* stat) const = 0;

include/istio/mixerclient/options.h

@@ -39,7 +39,17 @@ struct CheckOptions {
   const int num_entries;
 
   // If true, Check is passed for any network failures.
-  bool network_fail_open = true;
+  bool network_fail_open{true};
+
+  // Number of retries on transport error
+  uint32_t retries{0};
+
+  // Base milliseconds to sleep between retries.  Will be adjusted by
+  // exponential backoff and jitter.
+  uint32_t base_retry_ms{80};
+
+  // Max milliseconds to sleep between retries.
+  uint32_t max_retry_ms{1000};
 };
 
 // Options controlling report batch.

include/istio/utils/attribute_names.h

@@ -31,6 +31,11 @@ struct AttributeName {
   static const char kSourceUID[];
   static const char kDestinationPrincipal[];
 
+  static const char kDestinationServiceName[];
+  static const char kDestinationServiceUID[];
+  static const char kDestinationServiceHost[];
+  static const char kDestinationServiceNamespace[];
+
   static const char kRequestHeaders[];
   static const char kRequestHost[];
   static const char kRequestMethod[];

include/istio/utils/attributes_builder.h

@@ -26,6 +26,8 @@
 namespace istio {
 namespace utils {
 
+const char kMixerMetadataKey[] = "istio.mixer";
+
 // Builder class to add attribute to protobuf Attributes.
 // Its usage:
 //    builder(attribute).Add("key", value)
@@ -142,7 +144,9 @@ class AttributesBuilder {
     }
 
     for (const auto &filter : filter_state) {
-      AddProtoStructStringMap(filter.first, filter.second);
+      if (FiltersToIgnore().find(filter.first) == FiltersToIgnore().end()) {
+        AddProtoStructStringMap(filter.first, filter.second);
+      }
     }
   }
 
@@ -152,6 +156,14 @@ class AttributesBuilder {
   }
 
  private:
+  const std::unordered_set<std::string> &FiltersToIgnore() {
+    static const auto *filters =
+        new std::unordered_set<std::string>{kMixerMetadataKey};
+    return *filters;
+  }
+
+  // TODO(jblatt) audit all uses of raw pointers and replace as many as possible
+  // with unique/shared pointers.
   ::istio::mixer::v1::Attributes *attributes_;
 };
 

include/istio/utils/simple_lru_cache_inl.h

@@ -497,7 +497,7 @@ class SimpleLRUCacheBase {
   // so we implement it in the derived SimpleLRUCache.
   virtual void RemoveElement(const Key& k, Value* value) = 0;
 
-  virtual void DebugIterator(const Key& k, const Value* value, int pin_count,
+  virtual void DebugIterator(const Key&, const Value* value, int pin_count,
                              int64_t last_timestamp, bool is_deferred,
                              std::string* output) const {
     std::stringstream ss;
@@ -1054,7 +1054,7 @@ class SimpleLRUCache
             EQ>(total_units) {}
 
  protected:
-  virtual void RemoveElement(const Key& k, Value* value) { delete value; }
+  virtual void RemoveElement(const Key&, Value* value) { delete value; }
 
  private:
   GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(SimpleLRUCache);
@@ -1077,7 +1077,7 @@ class SimpleLRUCacheWithDeleter
       : Base(total_units), deleter_(deleter) {}
 
  protected:
-  virtual void RemoveElement(const Key& k, Value* value) { deleter_(value); }
+  virtual void RemoveElement(const Key&, Value* value) { deleter_(value); }
 
  private:
   Deleter deleter_;

istio.deps

@@ -4,7 +4,7 @@
 		"name": "ISTIO_API",
 		"repoName": "api",
 		"file": "repositories.bzl",
-		"lastStableSHA": "825044c7e15f6723d558b7b878855670663c2e1e"
+		"lastStableSHA": "8463cba039d858e8a849847b872ecea50b0994df"
 	},
 	{
 		"_comment": "",

repositories.bzl

@@ -99,8 +99,14 @@ cc_library(
             actual = "@googletest_git//:googletest_prod",
         )
 
-ISTIO_API = "056eb85d96f09441775d79283c149d93fcbd0982"
-ISTIO_API_SHA256 = "df491c399f0a06bb2b85f43f5328c880c8e5cb5b3ce972efbd1ce137f83ebc52"
+#
+# To update these...
+# 1) find the ISTIO_API SHA you want in git
+# 2) wget https://github.com/istio/api/archive/ISTIO_API_SHA.tar.gz
+# 3) sha256sum ISTIO_API_SHA.tar.gz
+#
+ISTIO_API = "8463cba039d858e8a849847b872ecea50b0994df"
+ISTIO_API_SHA256 = "ae0fecec9bd316ec811833fff72ba191cf8c97348b33995585a1baa79fb26bf9"
 
 def mixerapi_repositories(bind = True):
     BUILD = """