We need to move common request headers to be first class attributes in order to improve cache efficiency. If the Mixer references any request headers during a Check call, the cache entry ends up being invalidated whenever any request header changes, which is the normal case. This effect completely neutralizes the existing cache.

Introduce a new Mixer attribute "connection.mtls", which indicates whether connection is mutual TLS enabled.
Send this attribute in both Check() and Report() calls.
This is to support monitoring mTLS connections per service.

Add Check/Report latency stats as histograms.

Add configuration control over which attributes should be sent to Mixer for both Check and Report. This will optimize the RPC exchanges between proxy and Mixer.

Although this configuration will set initially set by hand, eventually we can generate this config state automatically based on what the Mixer is configured to use.

There should be different configuration state for Check and Report.

We need to reliably prefetch quota tokens within the proxy.

For the distributed tracing features of Istio, we want to have mixerclient spans in the generated trace data. This would help with a number of debug-ability related items:

• better understanding caching
• better understanding perf of client library
• better understanding impact of network

cc: @rshriram

Need to put in place a framework to track unit test coverage for this repo.

We want to express caching in the proxy in terms of attributes. If a specific set of attributes remains constant for a given time and given # of uses, we can use a cached result for Check.

The attributes we have today often do not play well with the above goal because they aren't specific enough, leading to potentially completely ineffective caching. More specifically, there are cases where several different attribute values should be treated similarly from a caching perspective and are not. Some examples of this:

• The quintessential example of this is request.size, where the semantics we'd like is "all request.size values < XXX".

• Another example is request.time where we'd like to say things like "all requests between 12:00AM and 6:00AM".

• Finally, request.url contains method names which we want to operate on specifically, having different cached answers for different methods.

The solution to these problems involves introducing an attribute specialization model within the client such that it can produce derived attributes based on its existing set. These derived attributes would be sufficiently fine-grained to allow effective caching. This can be driven by relatively simple code and simple configuration within the client. For example:

• request.size. Generate the request.size.scale attribute which is a string assigned via config based on the range of the size. For example, the config can specify small:0..20, medium:21..200, large:201

• request.time. Generate a variety of attributes automatically such as request.time.hour, request.time.dayOfWeek, etc. Also, generate the config-driven request.time.scheduledEvent attribute with stuff like offtimes: 0:00-6:00,18:00-23:59

• request.url is broken down per Swagger
  In other words, we have specialized attribute processing algebra for a variety of attributes which is intended to produce new finer-grained attributes against which policies are applied in Mixer. To improve caching, we will encourage Mixer config to be authored using these derived attributes when possible.

We need to look at the individual attributes we produce and figure out what kind of specialization makes sense for each, and what kind of config syntax we want to control this. Finally, we need to decide which of these derived attributes are the most useful in the short term in order to prioritize implementation work.

Now the library is using GOOGLE_LOG. It is better to use the callers (Envoy) to provide logging function.

We need to enable the proxy to cache the result of calls to the mixer's Check API.

For all cached items, check and quota.

For quota, it is tricky, Some callback functions attached to on-progress requests may use the expired items. Have to be careful when removing them.

Now the check cache keys are a list of attribute names. e.g.

[
"request.path",
request.host"
]

A StringMap attribute such as "request.headers" has to use all its values.

It will be useful if we could choose some of keys out of a string map. such as ":method".

One idea is to use

"request.headers/:method" If there is a "/" in the cache key, the second portion is for the key within the map.

We should make the set of request headers sent by the proxy to Mixer to be configurable such as to reduce the payload size of proxy RPCs to Mixer.

For now, the specific set will need to be configured by hand. But in the future, this can be handled automatically by observing what Mixer actually consumes and configuring the proxy accordingly.

1. it is not in the attribute list, should add it
2. It is only used for tcp now. HTTP is using response.code. Should change code to use check.status if Check fail. Not to use response.code

for
number of Check calls,
number of cache hits
number of quota calls for each metric
number of quota cache hits for each metric
number of report calls
number of remote report calls

In order to improve caching efficiency, we need to add support for regex attribute value matching. This will enable increase cache lookup matches.

Expected to receive attribute destination.ip as a []byte. Instead, received string.

Example from attribute bag dump in latest Mixer:

destination.ip                : 10.56.2.185
source.ip                     : [10 150 0 5]

Note the key difference between the way in which source.ip and destination.ip are received.

@esnible commented on Tue May 30 2017

To reproduce, start up bookinfo.

cat << EOF |
type: route-rule
name: ratings-fail-xpercent
spec:
  destination: ratings.default.svc.cluster.local
  precedence: 2
  httpFault:
    abort:
      percent: 95
      httpStatus: 500
EOF
istioctl replace

Drive some traffic. Then look at the output in Prometheus or ServiceGraph. The traffic is reduced by the %, but there are no response_code entries for the injected 500s.

@ZackButcher commented on Tue May 30 2017

This is likely a problem in either Envoy's Mixer filter or the mixerclient itself. Mixer just blindly pushes the data we're provided to the metric backend, these missing stats seem to indicate that Report calls aren't being made to the mixer by Envoy when it sees a 500.

@douglas-reid commented on Tue May 30 2017

@qiwzhang any ideas what might cause this related to the filter chain?

2 Jobs needs to be created
mixerclient-presubmit
mixerclient-proxy-regression

You might need to rreproduce this issue on a xenial VM.

bazel test --config=tsan_nopie //:grpc_transport_test

tsan.txt

We see quite a lot of :

W0908 06:04:25.765] [libprotobuf ERROR external/mixerclient_git/src/report_batch.cc:78] Mixer Report failed with: UNAVAILABLE

eg https://k8s-gubernator.appspot.com/build/istio-prow/pull/istio_istio/699/e2e-suite-rbac-no_auth/360/

it would be great upon error to print which ip etc. we were connecting to and more details and granularity about the failure (did we fail to connect, did we timeout, did we not get a response, did we get a weird response, read error etc... (did we read anything, if so print that)

cc @mandarjog

See details in: istio/istio#365.

We should have support in mixerclient to recognize that the Mixer is using an older dictionary. Whenever this happens, the mixerclient would switch to using the V1 dictionary from that point forward (until the process is restarted that is). So a single Mixer instance not having been upgraded to the latest dictionary will lead to using an older dictionary, which would reduce overall compression, but would avoid a dictionary-related outage.

Doing this effectively means we can upgrade the dictionary in the client and mixer independently, they don't need to be in sync.

Please update all copyright messages in the repo to

Copyright XXX Istio Authors

it looks that mixerclient added a dependency on libuuid from
e2fsprogs, which isn't part of base OS (i.e. Goobuntu doesn't have it
installed by default).

However, this dependency wasn't added via Bazel, so the build is
broken and not hermetic at all.

I wanted to make a PR adding libuuid, but it looks that it's licensed
under GPLv2, which means that we cannot statically link against it,
AFAIK.

I think it is from util-linux, yes it is still LGPL, so I think we should remove the dependency.

Envoy has its own uuid impl:
https://github.com/lyft/envoy/blob/master/source/common/runtime/runtime_impl.cc#L28

Mixer client needs retry semantics around all calls to Mixer.

Currently, it use fixed expiration time (1s), passed in from check_options. Need to use the the expiration time from check_response.