Skip to content

Support collecting dubbo thread pool metrics#382

Merged
wu-sheng merged 9 commits intoapache:mainfrom
nisiyong:dubbo-threadpool
Nov 17, 2022
Merged

Support collecting dubbo thread pool metrics#382
wu-sheng merged 9 commits intoapache:mainfrom
nisiyong:dubbo-threadpool

Conversation

@nisiyong
Copy link
Copy Markdown
Contributor

@nisiyong nisiyong commented Nov 13, 2022

Inspired by the implementation of untow_thread_pool at issue#8433

Now the skywalking-javaagent could collect the Dubbo thread pool metrics. I tested it with dubbo-2.6.9 and dubbo-2.7.4.1 and ran it in our production environment with hundreds of applications, it works well.

Dubbo could use telnet to get the thread pool status, this meter plugin is related to this telnet implementation.

$ telnet localhost 20880                 
Trying ::1...
Connected to localhost.
Escape character is '^]'.

dubbo>status -l
+------------+--------+--------------------------------------------------------+
| resource   | status | message                                                |
+------------+--------+--------------------------------------------------------+
| threadpool | OK     | Pool status:OK, max:200, core:200, largest:3, active:1, task:3, service port: 20880 |
| load       | OK     | load:2.60595703125,cpu:12                              |
| memory     | OK     | max:3641M,total:1294M,used:481M,free:813M              |
| registry   | OK     | 127.0.0.1:2181(connected)                            |
| server     | OK     | /127.0.0.1:20880(clients:1)                         |
| summary    | OK     |                                                        |
+------------+--------+--------------------------------------------------------+

Please take a look. Also, cause the e2e test is friendly for the trace plugin, rather than the meter plugin. There is no test code with this PR, I'm willing to add test code if there are some guidelines.


  • Update the documentation to include this new feature.
  • Tests(including UT, IT, E2E) are added to verify the new feature.
  • If it's UI related, attach the screenshots below.
  • Update the CHANGES log.

@wu-sheng
Copy link
Copy Markdown
Member

The meter plugin could be verified by the same way like tracing plugin.

https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/plugin-test/#expecteddatayaml

Meter and Logs are all supported actually.

@wu-sheng
Copy link
Copy Markdown
Member

@nisiyong
Copy link
Copy Markdown
Contributor Author

Thanks, test code will be add soon.

@nisiyong
Copy link
Copy Markdown
Contributor Author

The dubbo-2.5.x-scenario test case runs well, but the dubbo-2.7.x-scenario test case runs failed. Because after dubbo-2.7.5, the thread pool has big changes, see apache/dubbo#6625. Maybe separate the test case, or let dubbo-2.7.x-plugin adapt the high version.

@wu-sheng
Copy link
Copy Markdown
Member

Maybe separate the test case, or let dubbo-2.7.x-plugin adapt the high version.

What do you mean separate test case? I can see one case for 2.5.x, and the other for 2.7.x. Do you mean 2.7.0-2.7.4 could pass the tests, but 2.7.5+ would fail due to the changes?

@nisiyong
Copy link
Copy Markdown
Contributor Author

Yes! There are many dubbo versions that make people crazy.

@wu-sheng
Copy link
Copy Markdown
Member

Yes! There are many dubbo versions that make people crazy.

For version compatibility, it is okay if they break something internally. We just need to choose the witness class or method(s) to identify versions(not actual version numbers, but different internal APIs) in the runtime.

4. Set up witnessClasses and/or witnessMethods if the instrumentation has to be activated in specific versions in

https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/java-plugin-development-guide/#implement-plugin

If you have to rely on different versions to compile the plugin codes, we should provide two new plugins(dubbo-2.7.4-below-pool-metrics and dubbo-2.7.5-above-pool-metrics), but keep the original dubbo-2.7.x-plugin untouched for tracing only.

Meanwhile, due to the witness class mechanism, you should be able to keep test scenarios as one.

@nisiyong
Copy link
Copy Markdown
Contributor Author

Thanks, let me check how to adapt 2.7.5+ first.

@nisiyong
Copy link
Copy Markdown
Contributor Author

Now the dubbo scenarios run well. There is a better class to enhance. We don't need the witness class and separate the plugins. Here are the brief changes:

  1. Change the enhance class to org.apache.dubbo.remoting.transport.AbstractServer.
  2. Fix the wrong default port 20080 -> 20880.
  3. Change the thread pool name to DubboServerHandler-%port%, just like the dubbo thread name DubboServerHandler-127.0.0.1:20880-thread-%. But currently, I have not added the hostname to it because the expectedData.yaml doesn't support the value operator start with, the hostname couldn't predict when running the test case.

Please take a look. :)

Comment thread docs/en/setup/service-agent/java-agent/Plugin-list.md
@wu-sheng
Copy link
Copy Markdown
Member

Could you share the screenshots about how this metric of pool likes like on the v9 UI?

@wu-sheng wu-sheng added this to the 8.14.0 milestone Nov 16, 2022
@wu-sheng
Copy link
Copy Markdown
Member

Please update the changes.md too.

@nisiyong
Copy link
Copy Markdown
Contributor Author

Could you share the screenshots about how this metric of pool likes like on the v9 UI?

We forward the metrics to other storage in the backend and still use the v8 UI. I could configure the metrics in the v9 UI, it needs takes some time.

@wu-sheng
Copy link
Copy Markdown
Member

Could you share the screenshots about how this metric of pool likes like on the v9 UI?

We forward the metrics to other storage in the backend and still use the v8 UI. I could configure the metrics in the v9 UI, it needs takes some time.

It is fine your private usage is different. But please take some time to finish the steps for the community.

@nisiyong
Copy link
Copy Markdown
Contributor Author

Sure, I will finish it ASAP.

Comment thread .github/workflows/plugins-test.0.yaml Outdated
… scenario also support 2.6.x"

This reverts commit d0108e1.
@nisiyong
Copy link
Copy Markdown
Contributor Author

Here are the Dubbo thread pool screenshots at v9 UI. In this case, tomcat and Dubbo thread pool metrics are put together.

image

image

@wu-sheng
Copy link
Copy Markdown
Member

Yes, that is what I expected.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Nov 16, 2022

Is there anything left before merging? At least one, the supported-list.md should be updated.

@nisiyong
Copy link
Copy Markdown
Contributor Author

Is there anything left before merging? At least one, the supported-list.md should be updated.

The supported-list.md has be updated. One thing I metion before, I want to make agent-test-tools support new operator start with to check the thread pool name.

@wu-sheng
Copy link
Copy Markdown
Member

The supported-list.md has be updated. One thing I metion before, I want to make agent-test-tools support new operator start with to check the thread pool name.

Feel free to send a pull request to the tool. I think this is easy to add.
And, I think that is not a block for this PR. You could enhance the test case later when the tool side is ready.

@zth9
Copy link
Copy Markdown

zth9 commented Nov 22, 2022

Well done, This plugin is exactly what I want🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants