search

RestComm / jain-slee

The World's #1 Open Source JAIN-SLEE (JSLEE) 1.1 Implementation
http://www.restcomm.com/
GNU Affero General Public License v3.0
24 stars 50 forks source link

Support Slee Mbean java standard interfaces/Openmbean #106

Closed jaimecasero closed 6 years ago

jaimecasero commented 7 years ago

Current slee mbean interfaces uses nonStandard JMX java type like EvenTypeID that makes the consumption of this monitroing interfaces hard (it requires to load slee classes on client tool). Standard tools like JConsole fail to consume those operations because of this reason

Particularly EventRoterStatistics MBean exposes operations like ... image

This operation has lots of issues: -The operation name is bad because it suggests an attribute rather than an operation. "getXX" should be reserved for JMX attributes, so introspection libs work properly.Instead of verbs like "get/set" its better to use "retrieve/modify" for JMX operations. -The operation uses "int" as parameter, and "long" as result type. It would be better to use wrapper classes like java.lang.Integer and java.long.Long. Although primitives types are supported in JMX, OpenMBeans convention requires wrapper classes instead. -Use custom non standard EventTypeID class. This requiers the client to load this class locally just to invoke this simple operation, an String could have been used instead.

Consider to use JMX standard types (String,Integer,Long), and follow suggested practices.Old methods maybe left unmodified, and new ones created using these suggestions, so old/legacy clients are not affected by this enhancement (discontinue existing methods until future removal on next release)

duyanh030 commented 7 years ago

Hi @jaimecasero

I agree all with your points. @SergeyLee . What is your through on this?

Currently, We could do some changes as below:

  1. Duplicate functions in EventRouterStatistics MBean. Change get/set to "retrieve/modify". Keep all "get" function in this class to make the old clients are not affected (all "get" function could be removed next major release)
  2. Duplicate all functions having input parameter is "EventTypeID" type (EventRouterStatistics and ActivityManagementMbean). use String instead. Old function (EventTypeID type could be remove next major release).

Need to review all Mbean interface to adapt above points.

If @SergeyLee agree . I could implement the fix for this issue :) All other ideas are welcomed.

Regards Anh Dao

jaimecasero commented 7 years ago

@duyanh030 if you have a clear understanding of the requirement, as it seems, please start right away.

Would be nice if you could start with EventRouterStatistics MBean first, and then we could cover the rest of SLEE core MBeans

duyanh030 commented 7 years ago

@jaimecasero . OK I am starting to do it. :)

duyanh030 commented 7 years ago

@jaimecasero . I pushed commit.

jaimecasero commented 7 years ago

@duyanh030 find my comments in the commit.

@duyanh030 are you testing all these methods using jconsole or other JMX client?

duyanh030 commented 7 years ago

@jaimecasero yes. I did manual test jconsole with all new methods. It works fine:) I also tested with twiddle tools. But I see a lot exception there. Not sure I did the right thing because new changes work fine with jconsole. I will try to test with twiddle later.

duyanh030 commented 7 years ago

Hi @jaimecasero and @SergeyLee . I just pushed new commit and adapt with your comments. Please help me to review it :) Issue is tested with jconsole.

jaimecasero commented 7 years ago

@duyanh030 good on new event condition, but i think we could reduce number of changes by introducing the commented "findEvent" helper method, and then just call the regular methods... please check my comment in commit...

duyanh030 commented 7 years ago

Hi @jaimecasero . Thank you very much for a perfect comment on my commit. Your solutions now is the best thing. I keep all your "findEvent" function and just put 1 more condition for checking "eventComponentIDs is null" (just for safety reason).

duyanh030 commented 7 years ago

Hi @jaimecasero . Do you have more comments on this PR? is it possible that I could start to implement the remaining Mbean interfaces?

jaimecasero commented 7 years ago

@duyanh030 Im on trip in India. Let me come back home, and i will provide you my comments next week. Sorry for this delay.

deruelle commented 7 years ago

@SergeyLee @jaimecasero would you have some time to look at this ?

satanatoly commented 6 years ago

Sorry for late response. Its true that we may improve some of mbeans and switch to standard parameters. However as I see most of mbeans are defined in JAIN SLEE spec and part of them are using nonStandard JMX types. Since our JAIN SLEE implementation requires to be SLEE compatible we will not be able to complete this task. @duyanh030 @jaimecasero JAIN SLEE spec - http://download.oracle.com/otn-pub/jcp/jain_slee-1_1-final-eval-oth-JSpec/jslee-1_1-fr-spec.pdf

jaimecasero commented 6 years ago

the idea qould be to provide extensions methods to the spec noes

not to modify the defined in spec

so,for every mbean defined in spec, provide complement mthods with equivalent features, but using nonCustom types

i still see this applicable...

satanatoly commented 6 years ago

Closing. Confirmed by @deruelle