[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[asfgit]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[ahgittin]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[tbouron]

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 tags = MutableList.builder()
-.addAll(s.subscriberExtraExecTags == null ? 
ImmutableList.of() : s.subscriberExtraExecTags)
-.add(s.subscriberExecutionManagerTag)
-.build()
-.asUnmodifiable();
-Map 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[tbouron]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[tbouron]

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 tags = MutableList.builder()
-.addAll(s.subscriberExtraExecTags == null ? 
ImmutableList.of() : s.subscriberExtraExecTags)
-.add(s.subscriberExecutionManagerTag)
-.build()
-.asUnmodifiable();
-Map 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[tbouron]

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 tags = MutableList.builder()
-.addAll(s.subscriberExtraExecTags == null ? 
ImmutableList.of() : s.subscriberExtraExecTags)
-.add(s.subscriberExecutionManagerTag)
-.build()
-.asUnmodifiable();
-Map 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 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] brooklyn-server pull request #561: better presentation for sensor publishing...

[ahgittin]

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 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---