Remove throw from ext_proc data plane when client start failed #39388
Conversation
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
yanjunxiang-google
requested review from
mattklein123,
tyxia and
yanavlasov
as code owners
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
|
/assign @stevenzzzz @tyxia |
| client_manager_.getOrCreateRawAsyncClientWithHashKey(config_with_hash_key, scope_, true); | ||
| THROW_IF_NOT_OK_REF(client_or_error.status()); | ||
| if (!client_or_error.status().ok()) { | ||
| return nullptr; |
There was a problem hiding this comment.
are there any logging/stats tracked this error case?
I feel it is good to have a logging line here?
There was a problem hiding this comment.
+1. Let's add a LOG_EVERY_N here.
There was a problem hiding this comment.
probably too spammy if there are a lot of request, let's do log_every_N_sec(10)
There was a problem hiding this comment.
This failure is very rare, and if it happens, we should just log it. BTW, there is no log_every_N_sec() in OSS.
There was a problem hiding this comment.
it will log too much when the config is in trouble, right now linear to QPS, I don't think that's worth that much.
There was a problem hiding this comment.
could you use the ENVOY_LOG_PERIODIC_MISC macro, weird naming.
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
stevenzzzz
left a comment
stevenzzzz
left a comment
There was a problem hiding this comment.
LGTM modular the logging nit.
| client_manager_.getOrCreateRawAsyncClientWithHashKey(config_with_hash_key, scope_, true); | ||
| THROW_IF_NOT_OK_REF(client_or_error.status()); | ||
| if (!client_or_error.status().ok()) { | ||
| return nullptr; |
There was a problem hiding this comment.
it will log too much when the config is in trouble, right now linear to QPS, I don't think that's worth that much.
| client_manager_.getOrCreateRawAsyncClientWithHashKey(config_with_hash_key, scope_, true); | ||
| THROW_IF_NOT_OK_REF(client_or_error.status()); | ||
| if (!client_or_error.status().ok()) { | ||
| return nullptr; |
There was a problem hiding this comment.
could you use the ENVOY_LOG_PERIODIC_MISC macro, weird naming.
Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
|
@stevenzzzz Could you take another look? |
|
kind ping |
…proxy#39388) Remove throw from ext_proc data plane when client start failed --------- Signed-off-by: Yanjun Xiang <yanjunxiang@google.com> Signed-off-by: Ting Pan <panting@google.com>
4 participants
Remove throw from ext_proc data plane when client start failed