openshift / origin-server

OpenShift 2 (deprecated)

889 stars 516 forks source link

Add ENABLE_BROKER_STATS to broker.conf #6389

Open Miciah opened 8 years ago

Miciah commented 8 years ago

Add instrumentation in the controller to log Passenger memory usage statistics after every request.

Add ENABLE_BROKER_STATS setting (default false) to /etc/openshift/broker.conf to enable collecting and logging of these statistics.

These log entries can be used by the oo-plot-broker-stats command, added in commit 10d64633d0d37d48ab782727957e818935edf8e2, to generate data for gnuplot.

openshift-bot, please [test] [extended:broker,site]!

thrasher-redhat commented 8 years ago

LGTM. Would we want to update the help options or man pages to include this?

Miciah commented 8 years ago

Yeah. In fact, https://bugzilla.redhat.com/show_bug.cgi?id=1204251 already exists for the manpage.

thrasher-redhat commented 8 years ago

Repeated failure:

1) Error:
test_marketplace_order_handler_paid_event_tolerates_409(MarketplaceIntegrationTest):
Marketplace::OrderHandler::OrderPaidException: Failed sending order paid event to marketplace: 500 - [{"code":"Internal Server Error","message":"class java.lang.NullPointerException: null"}]
    /root/openshift-test/site/lib/marketplace/order_handler.rb:34:in `paid'
    /root/openshift-test/site/test/integration/marketplace_test.rb:25:in `block in <class:MarketplaceIntegrationTest>'

Looks like this might have to do with marketplace changes? @sallyom

sallyom commented 8 years ago

3098 should fix that failure

Miciah commented 8 years ago

openshift-bot, please [test] again now that the site extended tests are fixed!

Miciah commented 8 years ago

@sallyom, would you mind editing your comment? The link to the already-merged PR in the li repository is confounding openshift-bot.

Miciah commented 8 years ago

@sallyom, never mind, false alarm! The tests are running despite the "Linked pull request 3098 in repo 'li' is not mergeable" warning.

Miciah commented 8 years ago

@pravisankar, I cherry-picked this old change from you because I noticed we had merged oo-plot-broker-stats into master when it was written, so the tool is in Origin and Enterprise, but we never merged the instrumentation in the broker into master—that, we only merged into stage (see https://github.com/openshift/puppet-openshift-online/issues/870). Do you see any reason not to merge the instrumentation into master now? (The alternative is to delete oo-plot-broker-stats—no sense in having the tool without the instrumentation.)

thrasher-redhat commented 8 years ago

Re-[test] for what looks to be unrelated test failures.

thrasher-redhat commented 8 years ago

Looks like the same failures. Will need to investigate further.

Miciah commented 8 years ago

Rerebased.

Miciah commented 8 years ago

Rererebased.

Miciah commented 8 years ago

Correctly rererebased.

openshift-bot commented 8 years ago

Evaluated for online test up to c4dcfb24d0fe1a7e4fbef589b34d23164d286523

openshift-bot commented 8 years ago

Online Test Results: FAILURE (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9376/) (Extended Tests: broker, site)