Add way to signal encoding used for CompressedAttributes to Mixer

[douglas-reid]

Based on discussions around istio/istio#11151, this PR is meant to add a way to distinguish between the mixed ways in which release-1.0 and release-1.1 handle encoding of attributes in the client.

I'm looking for feedback on naming, etc.

[kyessenov]

If I were to design this again, I would not reuse the same field to mean two different things.
Perhaps, we need a new field:

  repeated CompressedAttributes delta_attributes

to separate delta-encoded from regular attribute bags.

You can in theory send both at the same time.

[douglas-reid]

Ping: @geeknoid @mandarjog . This is a P0 for release-1.1. We need some consensus on a resolution.

[geeknoid]

I left a comment in the bug about the fact I don't think we need to support non-compressed batches. Instead, when an attribute needs to be deleted, just start a fresh batch.

[douglas-reid]

@geeknoid are you suggesting that we increase the number of ReportRequests sent when we encounter "missing" attributes? That seems like a larger architectural change. I'm not sure how to evaluate that in the context of 1.1.

It seems that we already switched to non-compressed batches... but we don't have a way to distinguish between versions of the client library.

I'm hoping to find some relatively safe way of moving forward.

[geeknoid]

Correct, if an attribute is being deleted, send whatever has been accumulated so far. Then start a new batch from there. So it's really just about adding one check for attribute deletion, and immediately sending whatever is outstanding, instead of waitng for the whole batch to fill up. I would imagine this could be done in a few lines of code.

…

On Wed, Jan 23, 2019 at 1:18 PM Douglas Reid ***@***.***> wrote: @geeknoid <https://github.com/geeknoid> are you suggesting that we increase the number of ReportRequests sent when we encounter "missing" attributes? That seems like a larger architectural change. I'm not sure how to evaluate that in the context of 1.1. It seems that we already switched to non-compressed batches... but we don't have a way to distinguish between versions of the client library. I'm hoping to find some relatively safe way of moving forward. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#770 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVucHUB1L5yDOW4fNiVIYRRrV5F9rllIks5vGNG9gaJpZM4aNpTL> .

[douglas-reid]

@geeknoid you are much more optimistic than I am :).

@qiwzhang how hard would it be to modify the client to detect "deletions" and break apart batches (resulting in increased number of Report() calls) ?

[geeknoid]

/lgtm
/approve

[istio-testing]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: douglas-reid, geeknoid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [geeknoid]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[douglas-reid]

@wenchenglu would you be able to merge this PR?

[kyessenov]

@douglas-reid are you working on the proxy change to set the field property?

[douglas-reid]

@kyessenov sorta. i'm working on a few changes in the proxy... but progress is slow, as I try to test.