Skip to content

CIAM-261: Add Jetty ThreadPool Metrics#205

Merged
Manikumar Reddy (omkreddy) merged 2 commits intoconfluentinc:masterfrom
omkreddy:threadpool
Sep 1, 2020
Merged

CIAM-261: Add Jetty ThreadPool Metrics#205
Manikumar Reddy (omkreddy) merged 2 commits intoconfluentinc:masterfrom
omkreddy:threadpool

Conversation

@omkreddy
Copy link
Copy Markdown
Member

No description provided.

MetricName busyThreadCountMetricName = metrics.metricName(busyThreadCountName,
metricGroupName, " jetty thread pool active thread count.",
tags);
Gauge<Integer> busyThreadCount = (config, now) -> getBusyThreads();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Jetty QueuedThreadPool provides #getBusyThreads

@omkreddy Manikumar Reddy (omkreddy) force-pushed the threadpool branch 5 times, most recently from 8b73b5a to 95f72ae Compare August 28, 2020 18:41

private void addJettyThreadPoolMetrics(Metrics metrics, Map<String, String> tags) {
//add metric for jetty thread pool queue size
String requestQueueSizeName = "thread-pool-queue-size";

Choose a reason for hiding this comment

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

In DbRequestMetrics.java, we have request-queue-size, active-task-count, and thread-pool-usage. Why are these names different? Why don't we have a usage metric? (getBusyThreads() / getThreads()?)

Copy link
Copy Markdown
Member Author

@omkreddy Manikumar Reddy (omkreddy) Sep 1, 2020

Choose a reason for hiding this comment

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

Updated thread-pool-queue-size => request-queue-size and add thread-pool-usage metric..
I have not changed busy-thread-count metric name as its meaning different from active-task-count in jetty thread pool implementation.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should be consistent about metric names for similar things.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the updates. Looks good.

@omkreddy Manikumar Reddy (omkreddy) merged commit 2803b6a into confluentinc:master Sep 1, 2020
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