Skip to content
This repository was archived by the owner on Dec 16, 2020. It is now read-only.

Use connection's filter state object when available for filter state#310

Merged
jplevyak merged 5 commits intoenvoyproxy:masterfrom
gargnupur:nup_track
Dec 3, 2019
Merged

Use connection's filter state object when available for filter state#310
jplevyak merged 5 commits intoenvoyproxy:masterfrom
gargnupur:nup_track

Conversation

@gargnupur
Copy link
Copy Markdown
Contributor

@gargnupur gargnupur commented Nov 14, 2019

Use connection's filter state object when available as compared to request's filter state object

Description: Use connection's filter state object when available as compared to request's filter state object
Risk Level: Medium
Testing: Test in Istio/Proxy
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Copy link
Copy Markdown
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not displace request filter state. These are different filter states -- one applies to many requests. I think we need to expose both for now using a different accessor, e.g. connection_filter_state.
We will need upstream filter state later, too.

@gargnupur
Copy link
Copy Markdown
Contributor Author

We should not displace request filter state. These are different filter states -- one applies to many requests. I think we need to expose both for now using a different accessor, e.g. connection_filter_state.
We will need upstream filter state later, too.

@kyessenov : thanks for the feedback. Yes, I saw stats plugin stopped working because of this logic. Added accessor for connection_filter_state.

@gargnupur gargnupur force-pushed the nup_track branch 3 times, most recently from f9387b8 to 6282eca Compare November 20, 2019 21:26
@gargnupur
Copy link
Copy Markdown
Contributor Author

@kyessenov , @PiotrSikora ,@jplevyak: This is ready for review..

Copy link
Copy Markdown
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/assign @lizan

case PropertyToken::FILTER_STATE:
value = CelValue::CreateMap(
Protobuf::Arena::Create<WasmStateWrapper>(&arena, info->filterState()));
if (getConnection()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store the connection in a local and check if it is nullptr then reuse that local to get the filter state.

…quest's filter state object

Signed-off-by: gargnupur <gargnupur@google.com>

Fixed based on feedback

Signed-off-by: gargnupur <gargnupur@google.com>

Fixed formatting

Signed-off-by: gargnupur <gargnupur@google.com>
Signed-off-by: gargnupur <gargnupur@google.com>
Signed-off-by: gargnupur <gargnupur@google.com>
Signed-off-by: gargnupur <gargnupur@google.com>
Signed-off-by: gargnupur <gargnupur@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants