Skip to content

Do not report latency for streaming requests.#20

Merged
chowchow316 merged 2 commits intoistio:masterfrom
chowchow316:streaming-latency
Dec 28, 2016
Merged

Do not report latency for streaming requests.#20
chowchow316 merged 2 commits intoistio:masterfrom
chowchow316:streaming-latency

Conversation

@chowchow316
Copy link
Contributor

No description provided.

@chowchow316 chowchow316 self-assigned this Dec 22, 2016
@chowchow316 chowchow316 requested a review from qiwzhang December 22, 2016 22:37
@qiwzhang
Copy link
Contributor

It seems that this is not correct.

My idea is: here

https://github.com/istio/proxy/blob/master/contrib/endpoints/src/api_manager/context/request_context.cc#L267

if either request_streaming or response_streaming, not to set Latency.

@chowchow316
Copy link
Contributor Author

Revert the change in proto.cc, and add change in request_context.cc. PTAL.

Copy link
Contributor

@qiwzhang qiwzhang left a comment

Choose a reason for hiding this comment

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

Do we have streaming t tests to verify this change?

@chowchow316
Copy link
Contributor Author

Yes, we have grpc_long_streaming.tin endpoints/esp repo.

@chowchow316 chowchow316 merged commit 4089484 into istio:master Dec 28, 2016
@chowchow316 chowchow316 deleted the streaming-latency branch December 28, 2016 19:56
qiwzhang added a commit that referenced this pull request Feb 16, 2018
* Allocate response buffer.

* lint format.
brian-avery added a commit that referenced this pull request May 12, 2020
* Update Envoy to latest SHA

* Fixed
jwendell added a commit to jwendell/proxy that referenced this pull request Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants