I think that looks like a good approach, +1 from me.

Will have a think at the feed stop problem.

Cheers
Geoff


————————————————————
Gnu PGP key - http://is.gd/TTTTuI


> On 19 Jun 2016, at 15:54, Andrew Kennedy <[email protected]> 
> wrote:
> 
> Hi.
> 
> Are people happy with the initializer approach? I think the only outstanding 
> issue is something @neykov pointed out, which is that when using 
> `Entity#addFeed()` the feeds are not stopped when the entity is stopped. I 
> will have another look at fixing that, but suggestions welcome.
> 
> Andrew.
> 
> On Thu, 16 Jun 2016 at 04:49 Andrew Kennedy <[email protected] 
> <mailto:[email protected]>> wrote:
> Thanks for the feedback.
> 
> I decided on an `EntityInitializer` instead, because it also adds enrichers 
> of its own. The pull request has been updated with this change now. To use 
> it, add YAML like the following:
> 
> ```
> services:
>   - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
>     brooklyn.initializers:
>       - type: org.apache.brooklyn.entity.machine.AddMachineMetrics
> ```
> 
> Cheers,    
> Andrew.
> 
> On Thu, 16 Jun 2016 at 11:53 Geoff Macartney 
> <[email protected] 
> <mailto:[email protected]>> wrote:
> I agree with Sam and Svet, + 1 to making it an enricher, if possible, or at 
> least somehow keeping it separate from SoftwareProcess.
> 
> 
> ————————————————————
> Gnu PGP key - http://is.gd/TTTTuI <http://is.gd/TTTTuI>
> 
> 
>> On 16 Jun 2016, at 10:41, Sam Corbett <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> I share your concern that the placement of the feed is wrong. I feel like we 
>> would be bending Brooklyn's abstractions to fit the case rather than working 
>> out a better abstraction. Of course, deciding what data reflects a software 
>> process is subjective and as you point out we already break the 
>> encapsulation in a variety of places. Was the plan not always to make 
>> locations into entities too? This would be trivial if that were the case.
>> 
>> I also think we're continually making SoftwareProcess too important. Svet's 
>> suggestion of an enricher is a good one - I want to explicitly mix the 
>> capability in to entities, not enable it with a flag.
>> 
>> Sam
>> 
>> 
>> On 16/06/2016 09:13, Andrew Kennedy wrote:
>>> Hi.
>>> 
>>> For the project I am working on, we would like to use the CPU utilization
>>> as one of the metrics for scaling a cluster. The existing `MachineEntity`
>>> has a sensor feed that produces this data, along with uptime and memory
>>> usage information. The feed works on Linux VMs only, currently, as is uses
>>> SSH commands on the host to generate the values i.e. the `uptime` command,
>>> or the contents of files in `/proc/`.
>>> 
>>> I would like to propose moving the feed to `SoftwareProcess` so that it is
>>> available to all entities. It would be disabled normally, set by a
>>> `ConfigKey<Boolean>` flag. This would be named "metrics.machine.retrieve"
>>> to correspond to "metrics.usage.retrieve" which enables sensors in feeds
>>> that return application or process specific information. The
>>> `MachineEntity` would obviously have the default value set to "true", to
>>> maintain current behaviour.
>>> 
>>> The only issue with this change is that the placement of the sensor feed
>>> feels slightly wrong. These are returning data about the _machine_ but the
>>> entity represents a _process_ on that machine, and there may in fact be
>>> multiple entities sharing a single machine, via `SameServerEntity`. The
>>> `MachineEntity` is used to represent a VM without any applications running
>>> on it, and would not normally be part of a blueprint, so these sensors are
>>> not normally accessible. There is some precedent for placing machine data
>>> on an entity, such as the `HOSTNAME` sensor, so I think the break in
>>> encapsulation is quite small.
>>> 
>>> The PR containing the change is here:
>>> 
>>> - https://github.com/apache/brooklyn-server/pull/204 
>>> <https://github.com/apache/brooklyn-server/pull/204>
>>> 
>>> I'd appreciate any comments on whether this is a useful change, as well as
>>> a review of the pull request...
>>> 
>>> Thanks,
>>> Andrew.
>> 
> 
> -- 
> Andrew Kennedy ; Founder clocker.io <http://clocker.io/> project ; @grkvlt ; 
> Cloudsoft
> 
> -- 
> Andrew Kennedy ; Founder clocker.io <http://clocker.io/> project ; @grkvlt ; 
> Cloudsoft
> 

Reply via email to