search

opensearch-project / performance-analyzer-commons

Apache License 2.0
4 stars 18 forks source link

Feature/disk stats metrics #12

Closed stevanbz closed 1 year ago

stevanbz commented 1 year ago

Is your feature request related to a problem? Please provide an existing Issue # , or describe. Added cpu/disk related metrics from performance-analyzer-rca project needed for distributed performance tracing. Client (user of the commons lib) in order to be able to use the metrics, will have to add the permissions (described in src/main/resources/plugin-security.policy). Copy-pasted code base for getting the native thread id, code for getting the diskIO, cpu and sched metrics.

Publishing to local maven is useful once you want to test the usage of the lib: .

./gradlew clean build publishToMavenLocal

and then usage the lib as a dependency (if there is a need - exclude the clashing the lib's or maybe we should align the dependencies in this project according to the versions defined in Open search - check it out here):

api ("org.opensearch:performance-analyzer-commons:3.0.0.0-SNAPSHOT") {
    exclude (group: 'com.fasterxml.jackson.core', module: 'jackson-core')
    exclude (group: 'com.fasterxml.jackson.core', module: 'jackson-annotations')
    exclude (group: 'com.fasterxml.jackson', module: 'jackson-bom')
    exclude (group: 'com.fasterxml.jackson.core', module: 'jackson-databind')
  }

Edited a gradle task for publishing - since we had 2 tasks - one was publishing the jar to local mvn while the second one published only javadoc and sources jar.

Describe the solution you are proposing A clear and concise description of what you want to happen.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

Check List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Tjofil commented 1 year ago

Looks good to me.

khushbr commented 1 year ago

Couple of high level comments:

  1. Naming Convention suggestion: Instead of using *ActivityGenerator, we should rename the interfaces to more apt *MetricStore after moving the HashMap here instead of the class.
  2. Service stats are missing here, I am working on a separate PR and will add them.
  3. There are no Unit Tests in this PR. I expect a follow-up PR to add these tests.
stevanbz commented 1 year ago

Closing the PR in favor of https://github.com/opensearch-project/performance-analyzer-commons/pull/31

We decided not to go with adding the sample per one thread. Instead, we decided to have re-usable units of code that will be responsible for getting the metrics for the threads