On 8 July 2011 14:34, Felix Meschberger <[email protected]> wrote: > Hi, > > Am Freitag, den 08.07.2011, 09:50 +0100 schrieb Alasdair Nottingham: > > On 8 July 2011 08:24, Felix Meschberger <[email protected]> wrote: > > > > > Hi Alasdair, > > > > > > Am Donnerstag, den 07.07.2011, 22:25 +0100 schrieb Alasdair Nottingham: > > > > On 5 July 2011 13:15, Felix Meschberger <[email protected]> wrote: > > > > > > > > > Hi, > > > > > > > > > > Am Dienstag, den 05.07.2011, 12:50 +0100 schrieb Alasdair > Nottingham: > > > > > > Hi, > > > > > > > > > > > > This is amazing timing. I've hit exactly this problem while > writing > > > the > > > > > code > > > > > > for ARIES-686. > > > > > > > > > > ;-) > > > > > > > > > > > > > > > > > I agree with your proposal, although when you describe 2 when you > say > > > > > you'll > > > > > > look to see if the MBean implements a *MBean interface do you > mean > > > you > > > > > will > > > > > > use reflection, or look at the interfaces published into the > service > > > > > > registry? I think I'd like it to be the latter. > > > > > > > > > > If I drop the "(objectClass=*MBean)" term from the filter, I don't > care > > > > > for what is actually registered and the only means remaining is the > > > > > jmx.objectname service property. > > > > > > > > > > Thus I use reflection. > > > > > > > > > > In any case, it seems to be essential to not require the *MBean > > > > > interface to be exported by the MBean bundles (and consequently not > to > > > > > import them in the Whiteboard support bundle. > > > > > > > > > > But I could live well with using "(objectClass=*MBean)" (instead of > > > > > (jmx.objectname=*)) as the filter and inspect the objectClass for > the > > > > > MBean interface. This would be closer to my original code ;-) > > > > > > > > > > > > > > I'm wondering if we should look at the objectClass parameter in > > > combination. > > > > I might be worrying about a weird edge case, but I could see this > > > happening. > > > > Imagine the following object: > > > > > > > > class MyManageableObject implements PoolingMBean, LifecycleMBean > > > > { > > > > } > > > > > > > > this, I think, is not normal for StandardMBeans, but with the current > 702 > > > > fix MyManageableObject could be registered using PoolingMBean, and we > > > > publish using LifecycleMBean. I'm just mainly thinking about using > > > > objectClass to aid disambiguation. What do you think? > > > > > > Good point. How about combining these approaches ? > > > > > > * inspect objectClass property and if there is an *MBean in > > > there use it > > > * otherwise revert to reflection > > > > > > > > That would work. We could also do reflection first, but validate it > against > > objectClass although that might be less efficient. > > We don't have to validate since the service object must implement the > interfaces listed in the object class property - this is verified by the > framework. > > That wasn't what I was saying. A service can implement interfaces not referenced by objectClass. So I'm saying that where you find multiple interfaces that end MBean are implemented we would decide which one to use based on the objectClass attribute. So if you implement two interfaces ending MBean we would use the one the MBean was published under.
> > > > > > > On the other hand, if neither PoolingMean nor LifecycleMBean extend the > > > DynamicMBean interface it looks kind of weird anyway .. To my knowledge > > > JMX only supports a single *MBean interface and does not combine, > > > right ? > > > > > > > I agree it does look weird, however we are already moving away from the > > "normal" StandardMBean conventions here. We don't require the impl and > MBean > > interface to be in the same package. Also while a StandardMBean pattern > > doesn't support this I'm not so sure about DynamicMBeans. I agree this is > a > > corner case, but I still think it might be worth doing. > > ok > > Regards > Felix > > > > > > > > > > > Regards > > > Felix > > > > > > > > > > > Alasdair > > > > > > > > > > > > > Regards > > > > > Felix > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > Alasdair > > > > > > > > > > > > On 5 July 2011 08:03, Felix Meschberger <[email protected]> > wrote: > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > I think I missed a problematic point: Standard MBeans > registered > > > with > > > > > > > the <classname>MBean pattern require that the implementation > and > > > the > > > > > > > MBean interface reside in the same package. Not very practical > for > > > OSGi > > > > > > > where the MBean interface of course must be exposed. > > > > > > > > > > > > > > So, I think we should probably drop the requirement for the > MBean > > > to > > > > > > > exposed with an interface matching "*MBean". > > > > > > > > > > > > > > I still don't like to require the MBeanRegistration interface > to be > > > > > used > > > > > > > in the registration. It is kind of like a helper interface not > > > > > > > identifying the primary purpose of the MBean. > > > > > > > > > > > > > > How about changing the filter to just be (jmx.objectname=*). If > the > > > > > > > actual property is an empty string (or is not a single-value > > > String), > > > > > > > the object is expected to implement the MBeanRegistration > > > interface. If > > > > > > > not, an ERROR level message is logged and the service ignored. > > > > > > > > > > > > > > Now for the actual service object being registered: > > > > > > > 1 if the service implements the DynamicMBean interface, use it > > > > > > > right away as the object to register > > > > > > > 2 otherwise see whether the object implements an interface > whose > > > > > > > name matches the <simple-class-name>MBean pattern. If so, > wrap > > > > > > > the object with a StandardMBean class using the interface as > > > > > > > the MBean interface (and log this at INFO level) > > > > > > > 3 otherwise log an ERROR level message and ignore the service > > > > > > > > > > > > > > Note that using the simple-class-name in the second step is a > > > deviation > > > > > > > from the JMX Spec which requires the MBean interface to reside > in > > > the > > > > > > > same package as the MBean object. > > > > > > > > > > > > > > To make this work, the mbeanTracker ServiceTracker must be > modified > > > to > > > > > > > actually track all services (this is a generic bug to be fixed, > > > > > > > ARIES-700). > > > > > > > > > > > > > > Regards > > > > > > > Felix > > > > > > > > > > > > > > Am Montag, den 27.06.2011, 20:45 +0100 schrieb Alasdair > Nottingham: > > > > > > > > On 27 June 2011 20:19, Felix Meschberger <[email protected] > > > > > wrote: > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > Am Montag, den 27.06.2011, 20:07 +0100 schrieb Alasdair > > > Nottingham: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > I've been looking at the way the whiteboard > implementation > > > works > > > > > and > > > > > > > I > > > > > > > > > was > > > > > > > > > > wondering if it would make sense to change the way it > detects > > > > > mbeans. > > > > > > > > > > Currently it detects them by looking for: > > > > > > > > > > > > > > > > > > > > (objectClass=*MBean). The impl then needs to either have > a > > > > > > > jmx.objectname > > > > > > > > > > property, or it needs to be > > > javax.management.MBeanRegistration > > > > > > > extension. > > > > > > > > > I > > > > > > > > > > > > > > > > > > My idea for requiring some MBean interface is that it makes > > > > > > > registration > > > > > > > > > extremely easy: > > > > > > > > > > > > > > > > > > > > > > > > > I agree with this goal. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - either it is a DynamicMBean (or some extension thereof) > > > service > > > > > > > > > - or it is an interface with MBean suffix which as per the > spec > > > > > > > > > defines the MBean interface for the bean > > > > > > > > > > > > > > > > > > > > > > > > > OK. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > For the registration then only an ObjectName is required > which > > > can > > > > > be > > > > > > > > > provided as a service registration property or by > implementing > > > the > > > > > > > > > MBeanRegistration interface (which is also similarly used > in > > > the > > > > > spec > > > > > > > > > IIRC). > > > > > > > > > > > > > > > > > > > > > > > > > > > > think it would make more sense for a service filter like > > > this: > > > > > > > > > > > > > > > > > > > > > > > > > > (|(objectClass=javax.management.MBeanRegistration)(jmx.objectname=)) > > > > > > > > > > > > > > > > > > > > what do people think? > > > > > > > > > > > > > > > > > > By going that way, you will solve the second issue with the > > > filter > > > > > but > > > > > > > > > you then have an MBean where you have to find out how to be > > > able to > > > > > > > > > register (or I may be missing something in more recent JMX > > > specs). > > > > > > > > > > > > > > > > > > But then, I don't think we should require the > MBeanRegistration > > > > > > > > > interface as a service interface. Sounds kind of incorrect. > > > > > > > > > > > > > > > > > > > > > > > > > I think if it needs to be an MBeanRegistration then we should > > > require > > > > > the > > > > > > > > object to be advertised as an MBeanRegistration. Not putting > > > > > > > > MBeanRegistration on a service and then relying on it being > one > > > is > > > > > dodgy > > > > > > > in > > > > > > > > OSGi. Sure in most cases it'll work, but if someone decides > to > > > use > > > > > > > service > > > > > > > > hooks to insert a proxy they will probably get this wrong, > also > > > you > > > > > can't > > > > > > > > make use of the service registry to get the class space > > > consistency. > > > > > > > > > > > > > > > > Overall based on this I think a more correct filter would be > > > this: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (&(objectClass=*MBean)(|(objectClass=javax.management.MBeanRegistration)(jmx.objectname=))) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > All in all, I think the original filter sounds more > correct. > > > > > > > > > > > > > > > > > > Regards > > > > > > > > > Felix > > > > > > > > > > > > > > > > > > > Alasdair > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- Alasdair Nottingham [email protected]
