Spring boot package [Threads - data stream] by sunny-elastic · Pull Request #2981 · elastic/integrations

sunny-elastic · 2022-04-03T07:59:37Z

What does this PR do?

Generated the skeleton of Spring Boot integration package.
Added Threads data stream
Added data collection logic.
Added the ingest pipelines.
Mapped fields according to the ECS schema and added Fields metadata in the appropriate yml files.
Added system test cases.

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that Threads data stream collect metrics.
I have added an entry to my package's changelog.yml file.
If I'm introducing a new feature, I have modified the Kibana version constraint in my package's manifest.yml file to point to the latest Elastic stack release (e.g. ^8.0.0).

How to test this PR locally

Clone integrations repo.
Install elastic-package locally.
Start elastic stack using elastic-package.
Move to integrations/packages/spring_boot directory.
Run the following command to run tests.

elastic-package test

Note: We have covered dashboards and the visualisations for all data streams of spring boot into separate PR. Also Kibana version will be updated to 8.1.0 in manifest.yml after testing this integration on 8.1.0.

elasticmachine · 2022-04-03T08:12:59Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-04-27T06:29:26.390+0000
Duration: 17 min 39 sec

Test stats 🧪

Test	Results
Failed	0
Passed	16
Skipped	0
Total	16

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2022-04-03T12:46:51Z

Pinging @elastic/integrations (Team:Integrations)

mtojek · 2022-04-04T10:48:33Z

+        - name: threading
+          type: group
+          fields:
+            - name: current_thread


What does the current_thread mean in terms of Jolokia request? Which one is the current thread? Should we skip it?

so this current_thread should be current running thread , it has different values than threads. I think we should keep this.

kush-elastic

@sunny-elastic that's all from my end, left some comments.
@mtojek, please take a look and feel free to drop in suggestions/questions.

kush-elastic · 2022-04-26T05:59:06Z

+        title: Collect Spring Boot metrics of Memory and Threading.
+        description: Collecting metrics from Spring Boot of Memory and Threading.


can we update this to something like following:

Suggested change

title: Collect Spring Boot metrics of Memory and Threading.

description: Collecting metrics from Spring Boot of Memory and Threading.

title: Collect Spring Boot metrics using Jolokia.

description: Collecting metrics from Spring Boot of Memory and Threading using Jolokia.

so in future if there are anymore data-stream that uses same jolokia input, we don't have provide all the types in title.

kush-elastic · 2022-04-26T05:59:35Z

+        - name: daemon_thread_count
+          type: long
+          description: Current number of live daemon threads
+        - name: object_monitor_usage_supported
+          type: boolean
+          description: Object monitor usage support
+        - name: peak_thread_count
+          type: long
+          description: Peak thread count to the current number of live threads
+        - name: synchronizer_usage_supported
+          type: boolean
+          description: Show the synchronizer usage support
+        - name: total_started_thread_count
+          type: long
+          description: Total number of threads created and also started since the Java virtual machine started


can we do this?

Suggested change

- name: daemon_thread_count

type: long

description: Current number of live daemon threads

- name: object_monitor_usage_supported

type: boolean

description: Object monitor usage support

- name: peak_thread_count

type: long

description: Peak thread count to the current number of live threads

- name: synchronizer_usage_supported

type: boolean

description: Show the synchronizer usage support

- name: total_started_thread_count

type: long

description: Total number of threads created and also started since the Java virtual machine started

- name: daemon

type: long

description: Current number of live daemon threads

- name: object_monitor_usage_supported

type: boolean

description: Object monitor usage support

- name: peak

type: long

description: Peak thread count to the current number of live threads

- name: synchronizer_usage_supported

type: boolean

description: Show the synchronizer usage support

- name: total_started

type: long

description: Total number of threads created and also started since the Java virtual machine started

yeah make sense, its updated

kush-elastic

LGTM! Let's wait for other's review.

mtojek

Few comments on my side.

I'm not sure why do we need to collect these booleans.
I think we need another level spring_boot.threading.threads.* for metrics like current and started (instead of daemon and total_started).

mtojek · 2022-04-26T07:54:45Z

+            "allocated_memory": {
+                "enabled": true,
+                "supported": true
+            },
+            "contention_monitoring": {
+                "enabled": false,
+                "supported": true
+            },


What do these metrics mean? They seem to me more like a configuration.

its updated now and removed the unwanted ones

mtojek · 2022-04-26T07:55:21Z

+            "cpu_time": {
+                "enabled": true,
+                "supported": true
+            },


Same story here. As a user, I expected cpu_time but got some booleans.

we updated the structure and kept only relevant fields

mtojek · 2022-04-26T07:55:38Z

+                "cpu_time_supported": true,
+                "user_time": 470000000
+            },
+            "daemon": 16,


What does this metric mean?

for daemon ? its number of live daemon threads.

mtojek · 2022-04-26T07:55:48Z

+                "user_time": 470000000
+            },
+            "daemon": 16,
+            "object_monitor_usage_supported": true,


its updated

mtojek · 2022-04-26T07:55:52Z

+            "daemon": 16,
+            "object_monitor_usage_supported": true,
+            "peak": 20,
+            "synchronizer_usage_supported": true,


its updated

mtojek · 2022-04-26T07:56:04Z

+            "object_monitor_usage_supported": true,
+            "peak": 20,
+            "synchronizer_usage_supported": true,
+            "total_started": 23


total_started what?

Total number of threads created and also started since the Java virtual machine started. Updated this hierarchy in structure

mtojek

requested changes

sunny-elastic · 2022-04-26T13:29:13Z

Few comments on my side.

I'm not sure why do we need to collect these booleans.

I think we need another level spring_boot.threading.threads.* for metrics like current and started (instead of daemon and total_started).

@mtojek is this following structure looks good to you, since only kept relevant fields which has meaningful values to collect

"spring_boot"{
    "threading"{
        "threads"{
            "current"{
                "allocated_bytes",
                "time"{
                    "cpu",
                    "user"
                },
            },
            "count",
            "daemon",
            "total_started"
        }
    }
}

Here if you see the current object it only specifies the details regarding current thread, but count , daemon and total started are referring to multiple threads so coming under threads object. And it has appropriate description given in fields.yml

mtojek · 2022-04-26T13:38:04Z

Yes, it's a better fit. I'm wondering if we improve it more by applying this guidance.

Then:

count will be value
total_started will be started

sunny-elastic · 2022-04-26T13:50:03Z

yeah but count has a specific meaning here as count of live threads (Current number of live threads including both daemon and non-daemon threads.)
and total_started is Total number of threads created and also started since the Java virtual machine started, we can keep this as started. let me know what you think

mtojek · 2022-04-26T13:59:24Z

Ok, let's just change the total_started -> started. Otherwise, it LGTM.

elasticmachine · 2022-04-27T06:46:18Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`2/2`)	💚
Files	100.0% (`2/2`)	💚 2.918
Classes	100.0% (`2/2`)	💚 2.918
Methods	100.0% (`24/24`)	💚 11.29
Lines	93.289% (`139/149`)	👍 3.348
Conditionals	100.0% (`0/0`)	💚

sunny-elastic · 2022-04-27T07:03:57Z

Ok, let's just change the total_started -> started. Otherwise, it LGTM.

Yeah its updated. Thanks!

Spring boot package [Threads - data stream]

7277801

sunny-elastic requested a review from a team as a code owner April 3, 2022 07:59

sunny-elastic requested a review from mtojek April 3, 2022 08:17

sunny-elastic self-assigned this Apr 3, 2022

sunny-elastic added enhancement New feature or request Team:Integrations Label for the Integrations team New Integration Issue or pull request for creating a new integration package. labels Apr 3, 2022

mtojek requested a review from ruflin April 4, 2022 09:35

mtojek reviewed Apr 4, 2022

View reviewed changes

yug-rajani and others added 6 commits April 6, 2022 16:37

Merge branch 'elastic:main' into package_spring_boot_threads

b23e175

rename data stream and addressed review comments

4d4feac

add description to fields

2883861

Merge branch 'elastic:main' into package_spring_boot_threads

c9078e4

update CODEOWNER

49e3521

update ecs fields

904ab51

sunny-elastic requested a review from a team as a code owner April 20, 2022 11:44

sunny-elastic added 2 commits April 25, 2022 17:51

resolve merge conflicts

9322e88

update README.md

ff8abf6

sunny-elastic requested review from kush-elastic and mtojek April 25, 2022 15:13

kush-elastic suggested changes Apr 26, 2022

View reviewed changes

updated field.yml

a8a3dc8

kush-elastic approved these changes Apr 26, 2022

View reviewed changes

mtojek reviewed Apr 26, 2022

View reviewed changes

mtojek self-requested a review April 26, 2022 07:58

mtojek suggested changes Apr 26, 2022

View reviewed changes

update fields.yml and default.yml

b317d51

sunny-elastic requested a review from mtojek April 27, 2022 07:04

mtojek approved these changes Apr 27, 2022

View reviewed changes

mtojek merged commit 614a118 into elastic:main Apr 27, 2022

		title: Collect Spring Boot metrics of Memory and Threading.
		description: Collecting metrics from Spring Boot of Memory and Threading.

Conversation

sunny-elastic commented Apr 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Checklist

How to test this PR locally

Uh oh!

elasticmachine commented Apr 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

Uh oh!

elasticmachine commented Apr 3, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kush-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kush-elastic left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtojek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtojek left a comment

Choose a reason for hiding this comment

Uh oh!

sunny-elastic commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtojek commented Apr 26, 2022

Uh oh!

sunny-elastic commented Apr 26, 2022

Uh oh!

mtojek commented Apr 26, 2022

Uh oh!

elasticmachine commented Apr 27, 2022

🌐 Coverage report

Uh oh!

sunny-elastic commented Apr 27, 2022

sunny-elastic commented Apr 3, 2022 •

edited

Loading

elasticmachine commented Apr 3, 2022 •

edited

Loading

kush-elastic left a comment •

edited

Loading

sunny-elastic commented Apr 26, 2022 •

edited

Loading