You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/02/16 10:36:12 UTC
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/561
better presentation for sensor publishing tasks
previously they didn't even have a name; now they have a nice name, description, and a tag.
there is some attempt to optimize the use of toString so it isn't hugely computationally expensive,
although this will increase expense a bit; i tend to think it's worth it for increased visibility.
(if we are publishing vast numbers of sensor events maybe we are doing something wrong, and if this
is identified as the bottleneck we can parameterise it, and in any case when we come to be multi-host
we'll need to revisit how we do this as currently we're thinking a good datastore is good enough for
the intended volumes, as opposed to a dedicated message bus.)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server activity-descriptions
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/561.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #561
----
commit 2c2f3fcdc456358dff108a62e0cd5ce09783048c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-02-16T10:31:50Z
better presentation for sensor publishing tasks
previously they didn't even have a name; now they have a nice name, description, and a tag.
there is some attempt to optimize the use of toString so it isn't hugely computationally expensive,
although this will increase expense a bit; i tend to think it's worth it for increased visibility.
(if we are publishing vast numbers of sensor events maybe we are doing something wrong, and if this
is identified as the bottleneck we can parameterise it, and in any case when we come to be multi-host
we'll need to revisit how we do this as currently we're thinking a good datastore is good enough for
the intended volumes, as opposed to a dedicated message bus.)
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #561: better presentation for sensor publishing tasks
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/561
noticed when working on #559 . related to but not implementing ML discussion at this time around whether `entities/xxx/activities` API should default/optionally `includeBackground`. when listing bg tasks this gets rid of the annoying blank ones in the list; now you see:
and clicking through you can see:
Note the addition of the `SENSOR` tag. This could be used in UI's to filter sensor publication tasks /cc @tbouron .
Also note that if entity `child` publishes a sensor, eg in the course of `start`, and someone else, say `parent` is subscribed to it, the event does _not_ currently show up when you ask for `child`'s background tasks (because it runs with the `parent` as the context entity) nor the `start` task's subtasks (as we don't normally want to execute it as an in-queue subtask). Thus that event will _not_ show up even if you `includeBackground` on the `start` task; it will only show up if you query `parent` entity's tasks (with `includeBackground` if we change the default for that endpoint as discussed on ML). However it will still report the `child.start` as his "submitted by". If that is a problem we should add an index on `submittedBy` for tasks so we can properly and efficiently get `includeBackground` tasks submitted by a task even across entity boundaries. However ATM I think it is a minor but liveable quirk. (In any case it isn't being introduced here, it is just becoming clea
rer because other things are being clarified.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/561#discussion_r101533579
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java ---
@@ -111,7 +115,7 @@ public long getTotalEventsDelivered() {
s.subscriberExecutionManagerTagSupplied = true;
} else {
s.subscriberExecutionManagerTag =
- s.subscriber instanceof Entity ? "subscription-delivery-entity-"+((Entity)s.subscriber).getId()+"["+s.subscriber+"]" :
+ s.subscriber instanceof Entity ? "subscription-delivery-entity-"+((Entity)s.subscriber).getId() :
--- End diff --
Do we want to remove the subscriber name here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/561#discussion_r101534679
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java ---
@@ -237,42 +212,85 @@ public synchronized boolean unsubscribe(SubscriptionHandle sh) {
if (groovyTruth(subs)) {
if (LOG.isTraceEnabled()) LOG.trace("sending {}, {} to {}", new Object[] {event.getSensor().getName(), event, join(subs, ",")});
for (Subscription s : subs) {
- if (s.eventFilter!=null && !s.eventFilter.apply(event))
- continue;
- final Subscription sAtClosureCreation = s;
-
- List<Object> tags = MutableList.builder()
- .addAll(s.subscriberExtraExecTags == null ? ImmutableList.of() : s.subscriberExtraExecTags)
- .add(s.subscriberExecutionManagerTag)
- .build()
- .asUnmodifiable();
- Map<String, ?> execFlags = MutableMap.of("tags", tags);
-
- em.submit(execFlags, new Runnable() {
- @Override
- public String toString() {
- return "LSM.publish("+event+")";
- }
- @Override
- public void run() {
- try {
- int count = sAtClosureCreation.eventCount.incrementAndGet();
- if (count > 0 && count % 1000 == 0) LOG.debug("{} events for subscriber {}", count, sAtClosureCreation);
-
- sAtClosureCreation.listener.onEvent(event);
- } catch (Throwable t) {
- if (event!=null && event.getSource()!=null && Entities.isNoLongerManaged(event.getSource())) {
- LOG.debug("Error processing subscriptions to "+this+", after entity unmanaged: "+t, t);
- } else {
- LOG.warn("Error processing subscriptions to "+this+": "+t, t);
- }
- }
- }});
+ submitPublishEvent(s, event, false);
+ // excludes initial so only do it here
totalEventsDeliveredCount.incrementAndGet();
}
}
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ private void submitPublishEvent(final Subscription s, final SensorEvent<?> event, final boolean isInitial) {
+ if (s.eventFilter!=null && !s.eventFilter.apply(event))
+ return;
+
+ List<Object> tags = MutableList.builder()
+ .addAll(s.subscriberExtraExecTags == null ? ImmutableList.of() : s.subscriberExtraExecTags)
+ .add(s.subscriberExecutionManagerTag)
+ .add(BrooklynTaskTags.SENSOR_TAG)
+ .build()
+ .asUnmodifiable();
+
+ StringBuilder name = new StringBuilder("sensor ");
+ StringBuilder description = new StringBuilder("Sensor ");
+ String sensorName = s.sensor==null ? null : s.sensor.getName();
+ String sourceName = event.getSource()==null ? null : event.getSource().getId();
+ name.append(sourceName);
+ name.append(":");
+ name.append(sensorName);
+
+ description.append(sensorName);
+ description.append(" on ");
--- End diff --
Should the `on` be appended only if the `sensorName` is different than null?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/561#discussion_r101535119
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java ---
@@ -237,42 +212,85 @@ public synchronized boolean unsubscribe(SubscriptionHandle sh) {
if (groovyTruth(subs)) {
if (LOG.isTraceEnabled()) LOG.trace("sending {}, {} to {}", new Object[] {event.getSensor().getName(), event, join(subs, ",")});
for (Subscription s : subs) {
- if (s.eventFilter!=null && !s.eventFilter.apply(event))
- continue;
- final Subscription sAtClosureCreation = s;
-
- List<Object> tags = MutableList.builder()
- .addAll(s.subscriberExtraExecTags == null ? ImmutableList.of() : s.subscriberExtraExecTags)
- .add(s.subscriberExecutionManagerTag)
- .build()
- .asUnmodifiable();
- Map<String, ?> execFlags = MutableMap.of("tags", tags);
-
- em.submit(execFlags, new Runnable() {
- @Override
- public String toString() {
- return "LSM.publish("+event+")";
- }
- @Override
- public void run() {
- try {
- int count = sAtClosureCreation.eventCount.incrementAndGet();
- if (count > 0 && count % 1000 == 0) LOG.debug("{} events for subscriber {}", count, sAtClosureCreation);
-
- sAtClosureCreation.listener.onEvent(event);
- } catch (Throwable t) {
- if (event!=null && event.getSource()!=null && Entities.isNoLongerManaged(event.getSource())) {
- LOG.debug("Error processing subscriptions to "+this+", after entity unmanaged: "+t, t);
- } else {
- LOG.warn("Error processing subscriptions to "+this+": "+t, t);
- }
- }
- }});
+ submitPublishEvent(s, event, false);
+ // excludes initial so only do it here
totalEventsDeliveredCount.incrementAndGet();
}
}
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ private void submitPublishEvent(final Subscription s, final SensorEvent<?> event, final boolean isInitial) {
+ if (s.eventFilter!=null && !s.eventFilter.apply(event))
+ return;
+
+ List<Object> tags = MutableList.builder()
+ .addAll(s.subscriberExtraExecTags == null ? ImmutableList.of() : s.subscriberExtraExecTags)
+ .add(s.subscriberExecutionManagerTag)
+ .add(BrooklynTaskTags.SENSOR_TAG)
+ .build()
+ .asUnmodifiable();
+
+ StringBuilder name = new StringBuilder("sensor ");
+ StringBuilder description = new StringBuilder("Sensor ");
+ String sensorName = s.sensor==null ? null : s.sensor.getName();
+ String sourceName = event.getSource()==null ? null : event.getSource().getId();
+ name.append(sourceName);
+ name.append(":");
+ name.append(sensorName);
+
+ description.append(sensorName);
+ description.append(" on ");
+ description.append(sourceName);
+ description.append(" publishing to ");
+ description.append(s.subscriber instanceof Entity ? ((Entity)s.subscriber).getId() : s.subscriber);
+
+ if (includeDescriptionForSensorTask(event)) {
+ name.append(" ");
+ name.append(event.getValue());
--- End diff --
Not sure we need the value in the name as we already have it in the description
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/561#discussion_r101534376
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java ---
@@ -237,42 +212,85 @@ public synchronized boolean unsubscribe(SubscriptionHandle sh) {
if (groovyTruth(subs)) {
if (LOG.isTraceEnabled()) LOG.trace("sending {}, {} to {}", new Object[] {event.getSensor().getName(), event, join(subs, ",")});
for (Subscription s : subs) {
- if (s.eventFilter!=null && !s.eventFilter.apply(event))
- continue;
- final Subscription sAtClosureCreation = s;
-
- List<Object> tags = MutableList.builder()
- .addAll(s.subscriberExtraExecTags == null ? ImmutableList.of() : s.subscriberExtraExecTags)
- .add(s.subscriberExecutionManagerTag)
- .build()
- .asUnmodifiable();
- Map<String, ?> execFlags = MutableMap.of("tags", tags);
-
- em.submit(execFlags, new Runnable() {
- @Override
- public String toString() {
- return "LSM.publish("+event+")";
- }
- @Override
- public void run() {
- try {
- int count = sAtClosureCreation.eventCount.incrementAndGet();
- if (count > 0 && count % 1000 == 0) LOG.debug("{} events for subscriber {}", count, sAtClosureCreation);
-
- sAtClosureCreation.listener.onEvent(event);
- } catch (Throwable t) {
- if (event!=null && event.getSource()!=null && Entities.isNoLongerManaged(event.getSource())) {
- LOG.debug("Error processing subscriptions to "+this+", after entity unmanaged: "+t, t);
- } else {
- LOG.warn("Error processing subscriptions to "+this+": "+t, t);
- }
- }
- }});
+ submitPublishEvent(s, event, false);
+ // excludes initial so only do it here
totalEventsDeliveredCount.incrementAndGet();
}
}
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ private void submitPublishEvent(final Subscription s, final SensorEvent<?> event, final boolean isInitial) {
+ if (s.eventFilter!=null && !s.eventFilter.apply(event))
+ return;
+
+ List<Object> tags = MutableList.builder()
+ .addAll(s.subscriberExtraExecTags == null ? ImmutableList.of() : s.subscriberExtraExecTags)
+ .add(s.subscriberExecutionManagerTag)
+ .add(BrooklynTaskTags.SENSOR_TAG)
+ .build()
+ .asUnmodifiable();
+
+ StringBuilder name = new StringBuilder("sensor ");
+ StringBuilder description = new StringBuilder("Sensor ");
+ String sensorName = s.sensor==null ? null : s.sensor.getName();
+ String sourceName = event.getSource()==null ? null : event.getSource().getId();
+ name.append(sourceName);
+ name.append(":");
--- End diff --
Should the `:` be appended **only** if the sourceName is different than `null`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/561#discussion_r101876867
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManager.java ---
@@ -111,7 +115,7 @@ public long getTotalEventsDelivered() {
s.subscriberExecutionManagerTagSupplied = true;
} else {
s.subscriberExecutionManagerTag =
- s.subscriber instanceof Entity ? "subscription-delivery-entity-"+((Entity)s.subscriber).getId()+"["+s.subscriber+"]" :
+ s.subscriber instanceof Entity ? "subscription-delivery-entity-"+((Entity)s.subscriber).getId() :
--- End diff --
yes, the tostring repeats the ID, adds the java type which might be nice, but really a bit clunky imo
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/561
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #561: better presentation for sensor publishing tasks
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/561
Also great to have a `SENSOR` tag, this will allows us to do filtering more easily.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---