Bean subscription #426

scottopell · 2023-04-10T21:46:32Z

eCurrently there is a periodic refresh of the beans (and their attributes) which can result in CPU spikes as these remote RMI calls are expensive.

This PR implements a "subscription mode" for instances where a javax.management.NotificationListener is used to subscribe to bean registration/unregistration events.
This approach should spread out the expensive RMI calls over time and will reduce the total amount of calls needed.

Status @ Feb 8 2024:
This branch is showing flakey tests, so they need to be investigated before this can be merged.
Once this branch is clean, I intend to merge this and publish a SNAPSHOT build which can be used in nightly agent builds to get validation. This agent branch is required before the snapshot/nightly.

Latest flakey report: Ran 2000 times, 4 failures found

…tion

src/main/java/org/datadog/jmxfetch/Instance.java

…ork cut test

… failure

scottopell · 2023-05-22T21:24:41Z

Can audit logging answer "Were there any beans missed?"
Simplify bean subscription creation, can execute on main thread
Add more tests around metric counting (ie, if we reach maxMetrics and new bean subscription arrives, do we properly ignore it? If we reach maxMetrics and a new bean unregistration arrives, do we properly decrement the metric count?)
What is recommended configuration for internal testing?
~~Add metrics around instance count, collection time, metrics collected, etc (basically jmxfetch telemetry)~~ (too much scope creep, not doing this)
Remove explicit lock in favor of synchronized
Process bean notifications on dedicated thread to allow all notifications to arrive unscathed

…testing

carlosroman · 2023-06-01T09:06:43Z

src/main/java/org/datadog/jmxfetch/BeanNotificationListener.java

+            return;
+        }
+        MBeanServerNotification mbs = (MBeanServerNotification) notification;
+        queue.offer(mbs);


Do we want to do anything here if we get false back? Maybe log/warn we couldn't add the MBeanServerNotification?

# Conflicts: # tools/misbehaving-jmx-server/README.md

…ReconnectContainer

# Conflicts: # src/main/java/org/datadog/jmxfetch/Instance.java # src/main/java/org/datadog/jmxfetch/JmxAttribute.java # src/test/java/org/datadog/jmxfetch/TestCommon.java

apigirl

editorial feedback

tools/misbehaving-jmx-server/README.md

Co-authored-by: Kaylyn <[email protected]>

# Conflicts: # src/main/java/org/datadog/jmxfetch/App.java

…tion-level config

# Conflicts: # src/test/java/org/datadog/jmxfetch/TestCommon.java

# Conflicts: # src/main/java/org/datadog/jmxfetch/App.java # src/main/java/org/datadog/jmxfetch/Instance.java

scottopell added 7 commits April 10, 2023 18:48

Initial bean listening, adds attributes that match config

4fc47e2

Moves bean subscription into dedicated thread

e104ed2

Removes ConnectionListener

9fee19d

Adds test covering basic bean subscription functionality

0b996df

Consolidate attribute matching and take 'ACTION' into account

75a1bbc

Utilize 'synchronized' keyword for simpler locking

9e624cf

Adds audit logging to bean refresh to catch any gaps in bean subscrip…

75d9296

…tion

scottopell force-pushed the feat/bean-listener branch from c6c4270 to 75d9296 Compare April 12, 2023 21:32

scottopell added 4 commits April 17, 2023 19:57

Shortens test timeframes for subscription to be processed

e4b2e6d

Back to explicit locking

ff15967

Removes un-necessary lockholder class

e0600d3

implements bean unsubscription

64d8438

scottopell commented Apr 17, 2023

View reviewed changes

src/main/java/org/datadog/jmxfetch/Instance.java Outdated Show resolved Hide resolved

scottopell commented Apr 17, 2023

View reviewed changes

src/main/java/org/datadog/jmxfetch/Instance.java Outdated Show resolved Hide resolved

scottopell added 6 commits May 19, 2023 18:39

Merge branch 'master' into feat/bean-listener

fe8c168

Adds extra logging instructions for misbehaving jmx server

7f7c1a6

Adds dedicated remote container bean subscription tests

1bd15f0

Removes old mutex debugging code

4fd1b09

Adds bean unsubscription test and (failing) bean subscription w/ netw…

57f1970

…ork cut test

Adds JMX connection listener to mark instance as broken on connection…

6670056

… failure

scottopell added 9 commits May 23, 2023 19:19

Removes un-needed bean subscriber thread

d74319a

Clarifies bean-audit logging

fe7705c

Updates in-proc bean subscription tests with more attribute counting …

c26e91b

…testing

Removes explicit locking in favor of 'synchronized'

d746e40

Adds owned thread to process mbean subscription events

090f0dc

Remove some unused logs and accidental regressions

c3fbfe6

Addresses linting errors

addb80f

Renames ambiguous interface from BeanListener -> BeanTracker

08cee1b

Fixes bug where metricReached would display multiple times incorrectly

27dc057

scottopell dismissed gh123man’s stale review via 642ee12 May 31, 2023 16:53

scottopell added 2 commits June 5, 2023 17:31

Merge branch 'master' into feat/bean-listener

4d9ce86

Adds toggle to turn on bean subscription globally

e8c2b68

carlosroman reviewed Jun 5, 2023

View reviewed changes

scottopell added 2 commits July 5, 2023 19:24

Merge branch 'master' into feat/bean-listener

d1bd7b8

# Conflicts: # tools/misbehaving-jmx-server/README.md

Adds env vars to enable subscription globally for validation

79ce047

scottopell force-pushed the feat/bean-listener branch from b93caeb to 79ce047 Compare July 5, 2023 20:32

scottopell added 2 commits July 6, 2023 20:48

Updates TestBeanSubscription to use container IP directly similar to …

dcf963c

…ReconnectContainer

Merge branch 'master' into feat/bean-listener

781db37

# Conflicts: # src/main/java/org/datadog/jmxfetch/Instance.java # src/main/java/org/datadog/jmxfetch/JmxAttribute.java # src/test/java/org/datadog/jmxfetch/TestCommon.java

scottopell requested a review from a team as a code owner December 27, 2023 21:07

scottopell requested a review from a team December 27, 2023 21:07

scottopell added 6 commits December 27, 2023 16:12

Re-land changes from #481

905532a

Removes random idea for deployment validation

bad14d0

Relands bean match ratio from #487

be3c9b4

Fixes whoopsie

c118cc2

Fixes divide by zero bug introduced in previous commit

ea0bf5b

Adds tlm for beans registered and unregistered

82bbc94

apigirl reviewed Dec 28, 2023

View reviewed changes

tools/misbehaving-jmx-server/README.md Outdated Show resolved Hide resolved

scottopell and others added 11 commits December 28, 2023 21:33

Update tools/misbehaving-jmx-server/README.md

b40c5d6

Co-authored-by: Kaylyn <[email protected]>

Fixes linting errors

34290a1

Merge branch 'master' into feat/bean-listener

617f4f6

# Conflicts: # src/main/java/org/datadog/jmxfetch/App.java

Removes per-instance bean-subscription enablement in favor of applica…

01e760c

…tion-level config

Merge branch 'master' into feat/bean-listener

34ab16b

# Conflicts: # src/test/java/org/datadog/jmxfetch/TestCommon.java

Merge branch 'master' into feat/bean-listener

03eeff0

Fixes linter warning

95820e9

Fixes spy usage in bean sub tests

f916745

Adds env var enablement for backwards compatibility during rollout

ff5e64f

Merge branch 'master' into feat/bean-listener

5dfcd96

Merge branch 'master' into feat/bean-listener

f25c1c2

# Conflicts: # src/main/java/org/datadog/jmxfetch/App.java # src/main/java/org/datadog/jmxfetch/Instance.java

scottopell requested a review from a team as a code owner October 28, 2025 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bean subscription #426

Bean subscription #426

Uh oh!

scottopell commented Apr 10, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

scottopell commented May 22, 2023 •

edited

Loading

Uh oh!

carlosroman Jun 1, 2023

Uh oh!

apigirl left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Bean subscription #426

Are you sure you want to change the base?

Bean subscription #426

Uh oh!

Conversation

scottopell commented Apr 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottopell commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlosroman Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

apigirl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

scottopell commented Apr 10, 2023 •

edited

Loading

scottopell commented May 22, 2023 •

edited

Loading