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.

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

stats: fix crash on invalid json #3658

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

timonwong
Copy link
Contributor

@timonwong timonwong commented Jan 14, 2022

Signed-off-by: Tianpeng Wang tpwang@alauda.io

What this PR does / why we need it:

When the stats plugin starts with wrong JSON config (ill-formatted for example), it will crash on any request (healthcheck for example) which makes pod crash repeatedly (or fail to start if healthcheck configured).

@istio-testing istio-testing added do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test labels Jan 14, 2022
@istio-testing
Copy link
Collaborator

Hi @timonwong. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@timonwong timonwong changed the title WIP stats: fix crash on invalid json (WIP) stats: fix crash on invalid json Jan 14, 2022
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@timonwong timonwong marked this pull request as ready for review April 7, 2022 02:15
@timonwong timonwong requested a review from a team as a code owner April 7, 2022 02:15
@timonwong timonwong changed the title (WIP) stats: fix crash on invalid json stats: fix crash on invalid json Apr 7, 2022
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Apr 7, 2022
@kyessenov kyessenov added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Apr 7, 2022
@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Apr 7, 2022
@kyessenov kyessenov reopened this Apr 7, 2022
@kyessenov
Copy link
Contributor

/retest

Copy link
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.

LG.
What's the crash btw? We should probably make it not crash as well.

@istio-testing istio-testing merged commit 77b805d into istio:master Apr 7, 2022
@timonwong timonwong deleted the stats-crash-invalid-json branch April 7, 2022 06:26
@timonwong
Copy link
Contributor Author

@kyessenov The crash occurred when reporting metrics, when stats plugin is improperly configured, memberistio_dimensions_ for example won't get initialized.

The crash log looks like:

2022-04-07 2022-04-07T06:31:01.291495Z	info	Envoy proxy is ready
2022-04-07 2022-04-07T06:31:14.644469Z	critical	envoy backtrace	Caught Segmentation fault, suspect faulting address 0xf0
2022-04-07 2022-04-07T06:31:14.644560Z	critical	envoy backtrace	Backtrace (use tools/stack_decode.py to get line numbers):
2022-04-07 2022-04-07T06:31:14.644579Z	critical	envoy backtrace	Envoy version: 119f44fab6a17471c7e9acec311af5fb1fd6f70b/1.20.1/Modified/RELEASE/BoringSSL
2022-04-07 2022-04-07T06:31:14.644994Z	critical	envoy backtrace	#0: __restore_rt [0x7ff171b7b980]
2022-04-07 2022-04-07T06:31:14.814022Z	critical	envoy backtrace	#1: proxy_wasm::null_plugin::Stats::PluginRootContext::report() [0x55eac23d0831]
2022-04-07 2022-04-07T06:31:14.915397Z	critical	envoy backtrace	#2: proxy_wasm::null_plugin::Stats::PluginContext::onLog() [0x55eac23d3cf7]
2022-04-07 2022-04-07T06:31:15.018993Z	critical	envoy backtrace	#3: std::__1::__function::__func<>::operator()() [0x55eac2b6961b]
2022-04-07 2022-04-07T06:31:15.211963Z	critical	envoy backtrace	#4: proxy_wasm::ContextBase::onLog() [0x55eac2b74225]
2022-04-07 2022-04-07T06:31:15.316562Z	critical	envoy backtrace	#5: Envoy::Extensions::Common::Wasm::Context::log() [0x55eac2976cbc]
2022-04-07 2022-04-07T06:31:15.510410Z	critical	envoy backtrace	#6: Envoy::Http::FilterManager::log() [0x55eac41916dd]
2022-04-07 2022-04-07T06:31:15.613836Z	critical	envoy backtrace	#7: Envoy::Http::ConnectionManagerImpl::doDeferredStreamDestroy() [0x55eac4191347]
2022-04-07 2022-04-07T06:31:15.715907Z	critical	envoy backtrace	#8: Envoy::Http::ConnectionManagerImpl::doEndStream() [0x55eac4190e32]
2022-04-07 2022-04-07T06:31:15.819930Z	critical	envoy backtrace	#9: Envoy::Http::FilterManager::encodeData() [0x55eac41ab88e]
2022-04-07 2022-04-07T06:31:16.014518Z	critical	envoy backtrace	#10: Envoy::Router::UpstreamRequest::decodeData() [0x55eac4272771]
2022-04-07 2022-04-07T06:31:16.117170Z	critical	envoy backtrace	#11: Envoy::Http::ResponseDecoderWrapper::decodeData() [0x55eac40567cb]
2022-04-07 2022-04-07T06:31:16.117216Z	critical	envoy backtrace	#12: Envoy::Http::ResponseDecoderWrapper::decodeData() [0x55eac40567cb]
2022-04-07 2022-04-07T06:31:16.218930Z	critical	envoy backtrace	#13: Envoy::Http::Http1::ClientConnectionImpl::onMessageCompleteBase() [0x55eac41dde8b]
2022-04-07 2022-04-07T06:31:16.412339Z	critical	envoy backtrace	#14: Envoy::Http::Http1::ConnectionImpl::onMessageComplete() [0x55eac41d970d]
2022-04-07 2022-04-07T06:31:16.525208Z	critical	envoy backtrace	#15: Envoy::Http::Http1::LegacyHttpParserImpl::Impl::Impl()::{lambda()#3}::__invoke() [0x55eac41e1fbf]
2022-04-07 2022-04-07T06:31:16.710743Z	critical	envoy backtrace	#16: http_parser_execute [0x55eac452cfa9]
2022-04-07 2022-04-07T06:31:16.813024Z	critical	envoy backtrace	#17: Envoy::Http::Http1::LegacyHttpParserImpl::execute() [0x55eac41e19df]
2022-04-07 2022-04-07T06:31:16.914034Z	critical	envoy backtrace	#18: Envoy::Http::Http1::ConnectionImpl::dispatchSlice() [0x55eac41d7ad4]
2022-04-07 2022-04-07T06:31:17.017890Z	critical	envoy backtrace	#19: Envoy::Http::Http1::ConnectionImpl::dispatch() [0x55eac41d726c]
2022-04-07 2022-04-07T06:31:17.209562Z	critical	envoy backtrace	#20: Envoy::Http::Http1::ClientConnectionImpl::dispatch() [0x55eac41d6dbd]
2022-04-07 2022-04-07T06:31:17.311100Z	critical	envoy backtrace	#21: Envoy::Http::CodecClient::onData() [0x55eac40c3030]
2022-04-07 2022-04-07T06:31:17.415407Z	critical	envoy backtrace	#22: Envoy::Http::CodecClient::CodecReadFilter::onData() [0x55eac40c41b5]
2022-04-07 2022-04-07T06:31:17.515742Z	critical	envoy backtrace	#23: Envoy::Network::FilterManagerImpl::onContinueReading() [0x55eac444463f]
2022-04-07 2022-04-07T06:31:17.617222Z	critical	envoy backtrace	#24: Envoy::Network::ConnectionImpl::onReadReady() [0x55eac443dd06]
2022-04-07 2022-04-07T06:31:17.717499Z	critical	envoy backtrace	#25: Envoy::Network::ConnectionImpl::onFileEvent() [0x55eac443b74f]
2022-04-07 2022-04-07T06:31:17.909517Z	critical	envoy backtrace	#26: std::__1::__function::__func<>::operator()() [0x55eac44218b1]
2022-04-07 2022-04-07T06:31:18.013169Z	critical	envoy backtrace	#27: Envoy::Event::FileEventImpl::assignEvents()::$_1::__invoke() [0x55eac4422dbc]
2022-04-07 2022-04-07T06:31:18.114961Z	critical	envoy backtrace	#28: event_process_active_single_queue [0x55eac454c8e0]
2022-04-07 2022-04-07T06:31:18.317056Z	critical	envoy backtrace	#29: event_base_loop [0x55eac454b2f1]
2022-04-07 2022-04-07T06:31:18.416357Z	critical	envoy backtrace	#30: Envoy::Server::WorkerImpl::threadRoutine() [0x55eac3e37cbd]
2022-04-07 2022-04-07T06:31:18.521674Z	critical	envoy backtrace	#31: Envoy::Thread::ThreadImplPosix::ThreadImplPosix()::{lambda()#1}::__invoke() [0x55eac4780783]
2022-04-07 2022-04-07T06:31:18.521758Z	critical	envoy backtrace	#32: start_thread [0x7ff171b706db]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
4 participants