Recent PRs resulted in build and test failures that had to be corrected by subsequent PRs. It would be great if these could be caught before merging (as part of the presubmit checks, perhaps even before review).

Instead of having files like adapter/denyChecker/cfg.pb.go checked in, they should be generated on the fly during a build based on the checked in .proto file.

Add centralized test where every new adapter is added.
This includes running thru

pkg/adaptertesting.TestBuilderInvariants()

Also ensure that the Impl protobuf supports standard attributes, like.
debug and trace
etc.

I'd like the adapter configuration at adapter/denyChecker/cfg.proto to use a google.rpc.Status instead of an int32. But I can't figure out how to get the dependencies setup properly to get that to compile.

Would you mind taking a look to see if you can figure it out?

The work done by the attribute package is overhead imposed on every incoming request to the mixer. This code would benefit from tracking and improving perf over time.

Add Travis based unit test and code coverage.

Tracking issue

Mixer should also provide a quota aspect.

Current proposal:

istio.io/mixer/cmd/mixer/main.go
istio.io/mixer/pkg/adapter/...
istio.io/mixer/pkg/api/... (most of what's currently in mixer/server/*.go)
istio.io/mixer/​pkg/config/...
istio.io/mixer/pkg/attribute/...
istio.io/mixer/doc/...
istio.io/mixer/example/...

This impacts the ability to build protos that depend on:

google/rpc/status.proto

which uses:

option go_package = "google.golang.org/genproto/googleapis/rpc/status;status";

Current building requires setting up imports for protoc that break sandbox.

Tracking issue

Need to add a basic log to stdio logger to mixer to provide barebones logging support.

Work is being done in #157.

go_proto_library(
    name = "istio/config/v1",
    imports = [
        "../../external/com_github_google_protobuf/src"

prior comments:

geeknoid: Definitely not the right way to do this. I think the importmap feature is what you need here.
douglas-reid: importmap isn't needed for "well-known" protos. but you do need to specify where to find the sources for imports.
geeknoid: This can't be the right way to find these files. This is assuming the structure of the blaze output directories.

Tracking issue

Mixer will soon have a (application) logger aspect. We should consider adding an access log aspect that provides a LogAccess([]accesslogger.Entry) method for adapters.

We should figure out how we want to monitor the mixer process itself. There is probably a way to do it in a slick way using the configured adapters, etc., but we should design/plan for it and start working on it. In particular, we should start defining what (beyond basic request count, latencies, go routines, mem usage, etc.) would be interesting to monitor (most likely, adapter metadata).

I would like to propose to define attributes as a first-class concept of Istio at the same level of proxy, mixer, adapter and others, and use it to connect many sub-systems together, such as billing, logging, monitoring, analytics.

The main reason to define attributes as a first-class concept instead of part of proxy or mixer is to establish a stable foundation that we can quickly build many components on top it, including proxy, chemist, logging, monitoring, billing, rate limiting and more. So we don't let one component overrule others. Istio ecosystem must interact with many other systems, including standards like HTTP, gRPC, REST, Swagger/OpenAPI. With first-class attributes, we can develop the system as N modules instead of (N * N) mesh. Per our experience with Google API Platform, I am confident this proposal is significantly better and simpler than previous metadata designs and should meet Istio's needs comfortably.

An Istio attribute is a piece of metadata describing activities with an Istio service, such as the origin.ip (the attribute) of an API request (the activity) received by an Istio API service. Being a first-class concept, the same attribute will have the same value and semantics across all sub-systems, e.g. origin.ip in the logs will mean the same thing in the rate limiting system. If you want to allow one request per IP address per second, you can define the rate limit as "1/{origin.ip}/s".

The schema of the attribute will be simple:

• name: Each attribute has a qualified name. The namespace will be organized as "organization.product.property".
  • For example, origin.ip.
• type: Each attribute has a type, which specifies the value type. The type will be defined by an enum type.
  • For example, STRING.
• value: Each attribute has a string value, which must be interpreted according to the type.
  • For example, 127.0.0.1.

To define the above scheme using proto3, it would look like this:

package istio.type;

message Attribute {
enum Type {
// The value is a valid UTF-8 string.
STRING = 0;
// The value is a 64-bit integer printed as a decimal number text. It must not have leading "+".
INT = 1;
// The value is a 64-bit floating point printed as text.
FLOAT = 2;
// The value is a timestamp with RTC 3339 format.
TIMETAMP = 3;
}
string name = 1;
// Most attributes have well known type, so this field is often omitted.
Type type = 2;
string value = 3;
};

The attribute can be used in various systems. For example:

• To log attributes about an API request, we can simply specify a list of attribute names in the logging config.
• To monitor a system against certain attributes, we can use attributes as dimensions of metrics.
• To specify rate limits, we can use attributes in the limits.
  • Example: "1/{origin.ip}/s" and "3/{origin.user}/min".
• To control access, we can use attributes in access policy.
  • Example: origin.user != null.
• To express simple resource ownership.
  • Example: origin.user == resource.owner.
• To send attributes from proxy to REST servers, we can map attribute to HTTP header.
  • Example: origin.ip => X-Origin-IP

Thoughts?

Add an endpoint to mixer to do adapter schema discovery.

• either mixer used builder.getconfig()
• the complete schema is emitted at build time

Please build server docker images in your CI/CD workflow to be utilized by Istio manager.
Common pattern is to tag your images as:
gcr.io/istio-testing/mixer-alpha:GIT_SHA

Write mixer C++ client.
This client is used by istio proxy to interface with mixer.

example:
https://github.com/cloudendpoints/service-control-client-cxx

Sorry for this nit: almost all directories in this repo use singular name except adapters. It would be nicer to use singular for all directories. :)

This should exercise the config -> aspect -> adapter story from start to finish. When complete, we should ask the prometheus team to take a look and provide feedback on the approach and implementation.

Tracking issue: Per discussions related to #22, there is a desire to have a minimal schema for a set of well-known attribute that can be leveraged in configs, etc.

The mixer should support dynamic discovery/loading of adapters via a plugin model. There are a number of possible approaches, including usage of the coming go 1.8 plugin package.

For testing, debugging, API validation, etc., it would be nice to generate a simple CLI that can be used to send requests to a mixer instance.

Tracking issue for initial opentracing integration.

It should be possible to create a grpc middleware to add spans for Check requests, etc. Something like the grpc-go-middleware should suffice.

we currently have an uncomfortable tension between programmer vs. operator errors.

Options.

1. We use a custom error type that carries an explicitly operator-friendly error string in addition to the developer string
2. Extend error interface with InteralError () If an error implements this interface, it is free to output any message.
   All other error must produce operator friendly error messages.

example: https://golang.org/pkg/net/#Error

Need to add basic logging adapter, capable of dumping to stdout/stderr, ideally in JSON.

This will provide a template for expansion for other, more complex loggers that can push to diverse backends, while providing simple functionality for early end-to-end testing.

Worthwhile to look at concepts in other systems to see how we map. Please add more here ....

Prometheus
https://github.com/prometheus/client_model/blob/086fe7ca28bde6cec2acd5223423c1475a362858/metrics.proto#L76-%20%20L81

New Relic
attributes: https://docs.newrelic.com/docs/agents/manage-apm-agents/agent-data/collect-custom-attributes
metrics: https://docs.newrelic.com/docs/agents/manage-apm-agents/agent-data/custom-metrics
well-known attributes: https://docs.newrelic.com/docs/insights/new-relic-insights/decorating-events/insights-attributes

AppDynamics:
custom metrics: https://docs.appdynamics.com/display/PRO42/Analytics+Events+API
well known attributes: https://docs.appdynamics.com/display/PRO42/Transaction+Analytics+Data

DynaTrace:
well-known attributes: https://community.dynatrace.com/community/display/APMSAASDOC/Data+Models+for+REST+API+Payloads

We need to add ConfigManager, for managing configuration within Mixer.

Tracking issue.

Provide adapter(s) for AWS CloudWatch integration (logging/monitoring).

Tracking issue.

Provide adapters for Stackdriver logging/monitoring. This will require setup of creds passing in config, etc.

Proposes how Istio is configured.

Since this is an active debate here is a link to the google doc.

https://docs.google.com/document/d/1nEVaJanPoI0PsEVy11xgWla2bG6jsh9Y0UcI4_D5rjo/edit#

We now have a 2 level adapter cache.

1. Adapter Wrapper Cache
2. AdapterImpl cache

type (
	CacheableMessage struct {
		inner proto.Message
		sha string
	}
)
func NewCacheableMessage (cfg proto.Message) *CacheableMessage{
	cm = &CacheableMessage{
		inner: cfg,
		sha: cfg.String(),
	}
}

When a new config is ingested proto messages should be kept with its sha.

For impl configs, sha should be computed after calling DefaultConfig followed by parsing in the ptype.Struct

Tracking issue:

Provide adapter for Google Service Control.

We need a common infrastructure to track benchmark perf results, do graphs, prevent regressions on checkins, etc.

I'd like to see the following changes in our current code base:

• Get rid of the types.go pattern. Cramming a bunch of stuff in this kind of 'trash bin' is an anti-pattern as it obfuscate the source base.

• Move adapter-related functionality to pkg/adapter. For example, the adapter registry and the primary Adapter type should be there instead of in aspectsupport.

glog.init() method registers standard golang flags.

k8sclient go client vendors a version of glog. This makes it hard to use glog inside istio with k8sclient.

kubernetes/kubernetes#35096

options

1. fork and fix glog, so it does not auto register
2. Find a way to build it in bazel by removing dependency.

Need to add an adapter for basic export to statsd.

This will provide a basic template for creating additional metrics backend adapters and serve as a measuring stick for handling the various bits of config around metrics, monitored resources, labels, etc.

There's potential to improve the end-to-end perf for the proxy->mixer attribute-related protocol. Please see the conversation thread for PR #107 for details.

Before attempting these improvements, we need to see what the client-side code in the mixer will look like and have some end-to-end benchmarks with representative traffic.

Tracking issue

This work is already mostly completed. This issue is being filed to track progress towards milestones/projects.

Please re-open if more work is required.

https://github.com/gogo/protobuf provides more convenient protobuf code gen with (ostensibly) considerably faster marshalling/demarshalling code.

This is one of classic naming issues. Many traditional names, such as "/usr/bin", "/usr/lib", "java.util", use singular names. While plural name is perfect valid choice when you look at it specifically, but it is often out of place in symbols, such as "com.google.api.services.gmail.Gmail". It looks awkward to have a plural word in the middle of many singular words.

It would be better to always use singular word for all directory names, then we don't need to debate the choice case by case.

Create a set of simple standard scenario with a combination of well known attributes and metrics and benchmark size, serialization time & deserialization time.

Will need to quickly qualify API variations and evolution

Tracking issue for adding bazel.build support for istio mixer.

Need to work through contribution guide and generate (perhaps from k8s guide).

@smawson @geeknoid

Tracking issue for discussion of the API specification in

https://github.com/istio/mixer/tree/master/api/v1

From a conversation with @wora this morning:

It should be possible to define Check adapters (and possibly Quota) that operate in a log-only like mode. In other words, an operator should be able to add adapters that execute on Check() calls but that don't impact the outcome of the call.

This would be to support the rollout / validation of new IAM checks, etc., without potentially causing outages. @wora mentioned support for "dark launches" when describing this use case.

This is required for istio config

golang proto.jsonpb does not yet support Unmarshaling proto.Any and ptype.Struct
Other languages (python / Java / C++) do not have this problem.

gogo/protobuf#197
golang/protobuf#216
golang/protobuf#208

It can be worked around by using standard go json or yaml for decoders.

The mixer needs to handle:

• Panics in adapter methods
• Timeouts in adapter methods
• Needs to perform type validation of configs before they reach the adapters.