Reconcile filter state and dynamic metadata

[mattklein123]

See started PR here as well as discussion: https://github.com/envoyproxy/envoy/pull/4873

At minimum we need clear docs on when to use each one. Optimally we would merge them. I have some thoughts on how to do this and can work on this if no one gets to it first.

cc @rshriram @htuch

[lizan]

Also see #4100 for recalling early memories, some of them might not be apply anymore though.

[htuch]

@yangminzhu @qiwzhang do you think we can move JWT and other Istio derived authn/authz away from dynamic metadata to something based on filter state? I can't recall where we left this conversation.

[kyessenov]

Can we please resolve the question of accessing filter state from dynamic extensions (Lua, WASM, ext authz) before deprecating the dynamic metadata? It's not clear to us what is the generic way to obtain a field value from a filter state.

[htuch]

Yeah, we probably want something wire serializable here, I think this is the only generic way to handle it. @curiouserrandy @alyssawilk this is a design consideration that never occurred while we were thinking about filter state, thoughts?

My main meta-level concern here is that we have two distinct ways of performing inter-filter communications. We need clear guidance and shepherding of APIs use to ensure we avoid a complete fragmentation of the filter ecosystem into one or the other (or even worse, having to support multiple mechanisms at the expense of complexity).

Dynamic metadata has major performance downsides for some applications which prove prohibitive (proto (de)serialization is not free), even putting aside the typing advantages of filter state. OTOH, it's flexibility matches the model that is useful for authn/authz matching and works across language barriers. I would prefer a world in which we had something like typed filter state in which filters that work across language barriers had to offer a serialization options for when required.

[yangminzhu]

@yangminzhu @qiwzhang do you think we can move JWT and other Istio derived authn/authz away from dynamic metadata to something based on filter state? I can't recall where we left this conversation.

@htuch If we can have a generic Object in the filter state that every filter could access it (and it satisfies the current use case of dynamic metadata), technically we could migrate JWT, AuthN and RBAC filter to use it (and you'll also need to migrate Mongo, MySQL and ZooKeeper).

The dynamic metadata is used in a lot of scenarios (e.g. metrics, access control or general data sharing), and probably have external dependencies, for example, 1) Istio uses the metadata to do access control based on outputs from other filters. 2) Because the JWT and RBAC filter both exposes the metadata in its API, it's possible some users are using the dynamic metadata manually in the filter config (e.g. some advanced users are using MySQL/Mongo together with RBAC and this is replying on the dynamic metadata contract) All this means we need to do this carefully and give customer enough time to migrate.

Also it seems the other issue is how to access the Object from dynamic extensions as Kuat mentioned.

Yeah, we probably want something wire serializable here, I think this is the only generic way to handle it. @curiouserrandy @alyssawilk this is a design consideration that never occurred while we were thinking about filter state, thoughts?

My main meta-level concern here is that we have two distinct ways of performing inter-filter communications. We need clear guidance and shepherding of APIs use to ensure we avoid a complete fragmentation of the filter ecosystem into one or the other (or even worse, having to support multiple mechanisms at the expense of complexity).

I think the dynamic metadata has much better documentation: https://www.envoyproxy.io/docs/envoy/latest/configuration/well_known_dynamic_metadata. It's also used by more filters today.

Dynamic metadata has major performance downsides for some applications which prove prohibitive (proto (de)serialization is not free), even putting aside the typing advantages of filter state. OTOH, it's flexibility matches the model that is useful for authn/authz matching and works across language barriers. I would prefer a world in which we had something like typed filter state in which filters that work across language barriers had to offer a serialization options for when required.

Is it still a performance concern in the use case of authn/authz? We don't convert the protobuf.Struct to other protobuf message but instead check the values directly in it. My understanding is the main performance downside is due to converting to other protobuf messages?

[kyessenov]

I think what we need is a filter state object under the disguise of dynamic metadata. The generic interface to access metadata is nice (apart from lack of true integers), but I understand that it's internal representation is poor. We should be able to define a generic interface that provides:

• ability to serialize in and out of struct;
• ability to access values with true types (ints, strings, etc);
• ability to update values;

... and the let the filter choose how to represent it internally. This also seems like a good opportunity to break apart stream info into HTTP router filter state, TCP proxy filter state, etc.

[lizan]

@htuch just refreshing my memory: https://github.com/envoyproxy/envoy/issues/4100#issuecomment-428745612 I thought we reached an agreement that we cannot reconcile as the impedance match was too large. So we went ahead to better document dynamic metadata and filter state.

The current state is:

• data that has to be read/write from multiple filters, and need a sort of query configuration, (such as data consumed by RBAC) uses dyanmic metadata
• data that has clear purpose to a known filters, such as envoy.tcp_proxy.cluster, envoy.network.upstream_server_name, uses filter state.

Dynamic metadata has major performance downsides for some applications which prove prohibitive (proto (de)serialization is not free), even putting aside the typing advantages of filter state. OTOH, it's flexibility matches the model that is useful for authn/authz matching and works across language barriers. I would prefer a world in which we had something like typed filter state in which filters that work across language barriers had to offer a serialization options for when required.

Do we have some data that how much performance drop from using Protobuf::Struct/Message instead of C++ class, without actually using proto's (de)serialization (i.e. ParseFrom/SerializeTo)?

[htuch]

@lizan on the performance side, just anecdotes from internal teams that I have pointed away from dynamic metadata after performance issues were encountered. It's clearly the case that avoiding string copying and munging is going to be a win for some applications.

Looking at the #4100 comment history, I agree that we have no chance of full impedance match barring doing something like what @kyessenov suggests, which would be super cool but probably a lot of work.

I don't see clear guidance in the documentation today at https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/data_sharing_between_filters.html?highlight=dynamic%20metadata. Is there somewhere else we put together the above state summary you wrote?