Geoff, I have added a `StopFeedsAtMachineTask` to the lifecycle stop tasks, which will take care of this. Have a look at the updated PR. However, as Svet pointed out, the Feed management code in Brooklyn is definitely in need of some attention and refactoring...
Andrew. On Mon, 20 Jun 2016 at 01:41 Geoff Macartney < geoff.macart...@cloudsoftcorp.com> wrote: > 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 <andrew.kenn...@cloudsoftcorp.com> > 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 < > andrew.kenn...@cloudsoftcorp.com> 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 < >> geoff.macart...@cloudsoftcorp.com> 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 >>> >>> >>> On 16 Jun 2016, at 10:41, Sam Corbett <sam.corb...@cloudsoftcorp.com> >>> 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 >>> >>> 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 project ; @grkvlt ; Cloudsoft >> > -- > > Andrew Kennedy ; Founder clocker.io project ; @grkvlt ; Cloudsoft > > > -- Andrew Kennedy ; Founder clocker.io project ; @grkvlt ; Cloudsoft
