Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not call report if decodeHeaders is not called. #150

Merged
merged 1 commit into from
Mar 7, 2017
Merged

Not call report if decodeHeaders is not called. #150

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/envoy/mixer/http_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,8 @@ class Instance : public Http::StreamFilter, public Http::AccessLog::Instance {
const HeaderMap* response_headers,
const AccessLog::RequestInfo& request_info) override {
Log().debug("Called Mixer::Instance : {}", __func__);
// If decodeHaeders() is not called, not to call Mixer report.
if (!request_data_) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

When is decode headers not called, but the filter still gets control?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at log() function which is called after the response is send

Copy link
Contributor Author

Choose a reason for hiding this comment

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

our filter register all these callback functions, it is up to envoy to decide which one to call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our check() is done at decodeHeader(), and report() is done at log().

Copy link
Contributor

Choose a reason for hiding this comment

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

any specific config and/or request to reproduce this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

aah, ok. So if there is a connection reset or other transport level error, there will not be any headers to decode, but Log will still be called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can one of you approve this PR?

// Make sure not to use any class members at the callback.
// The class may be gone when it is called.
// Log() is a static function so it is OK.
Expand Down