Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[metering] Fix dependency issue in io.monitor bundle (#2288) #2289

Merged
merged 5 commits into from
Apr 28, 2021
Merged

[metering] Fix dependency issue in io.monitor bundle (#2288) #2289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ee9a15c
[metering] Fix dependency issue in io.monitor bundle (#2288)
Apr 13, 2021
246460d
[metering] incorporate feedback from CR; fix build warnings (#2288)
Apr 18, 2021
e1411ff
[metering] incorporate feedback from CR; fix build warnings (#2288)
Apr 18, 2021
7ae4e91
remove usage of Objects.requireNonNull
kaikreuzer Apr 23, 2021
a728f15
Merge pull request #1 from kaikreuzer/issue-2288
pravussum Apr 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion bundles/org.openhab.core.io.monitor/bnd.bnd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ Import-Package: \
org.osgi.service.*,\
org.slf4j.*,\
com.google.gson.*;version="[2.8,3)"
Export-Package: io.micrometer.core.*
Export-Package: io.micrometer.core.*;-split-package:=merge-first,\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the split-package good for?

Copy link
Contributor Author

@pravussum pravussum Apr 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added them because there was a warning in the bnd-maven plugin:
Split package, multiple jars provide the same package - thb I don't know where these duplicates originate from, I'm quite inexperienced with bnd/OSGI.
The split-package directive basically just sets the default strategy explicitly and thus removes the warning, not really the root cause. Any advice appreciated. Seems to work, though - I built the distribution and ran the Metrics IO addon and it produces results with no issues whatsoever.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where these duplicates originate from

My assumption is that this is because the original jar is on the build path as well as the extracted classes - so bnd thinks that two different bundles provide this package. The setting should thus be useless at runtime. It only suppresses the warning at build time. Not sure if we should keep it for this.

@wborn As our bnd expert: Do you have any opinion here?

org.HdrHistogram.*;-split-package:=merge-first,\
kaikreuzer marked this conversation as resolved.
Show resolved Hide resolved
org.LatencyUtils.*;-split-package:=merge-first
18 changes: 18 additions & 0 deletions bundles/org.openhab.core.io.monitor/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@
<version>1.6.3</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.hdrhistogram</groupId>
<artifactId>HdrHistogram</artifactId>
<version>2.1.12</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.latencyutils</groupId>
<artifactId>LatencyUtils</artifactId>
<version>2.0.3</version>
<scope>compile</scope>
<exclusions>
<exclusion>
<groupId>org.hdrhistogram</groupId>
<artifactId>HdrHistogram</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<build>
Expand Down
56 changes: 30 additions & 26 deletions ...ore.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/RuleMetric.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public RuleMetric(BundleContext bundleContext, Collection<Tag> tags, RuleRegistr
}

@Override
public void bindTo(@io.micrometer.core.lang.NonNull MeterRegistry meterRegistry) {
public void bindTo(MeterRegistry meterRegistry) {
unbind();
logger.debug("RuleMetric is being bound...");
this.meterRegistry = meterRegistry;
Expand All @@ -77,18 +77,20 @@ public void bindTo(@io.micrometer.core.lang.NonNull MeterRegistry meterRegistry)

@Override
public void unbind() {
if (meterRegistry == null) {
MeterRegistry mReg = meterRegistry;
if (mReg == null) {
return;
}
for (Meter meter : meterRegistry.getMeters()) {
if (meter.getId().getTags().contains(CORE_RULE_METRIC_TAG)) {
meterRegistry.remove(meter);
} else {
for (Meter meter : mReg.getMeters()) {
if (meter.getId().getTags().contains(CORE_RULE_METRIC_TAG)) {
mReg.remove(meter);
}
}
meterRegistry = null;
if (this.eventSubscriberRegistration != null) {
this.eventSubscriberRegistration.unregister();
this.eventSubscriberRegistration = null;
}
}
meterRegistry = null;
if (this.eventSubscriberRegistration != null) {
this.eventSubscriberRegistration.unregister();
this.eventSubscriberRegistration = null;
}
}

Expand All @@ -106,25 +108,27 @@ public Set<String> getSubscribedEventTypes() {

@Override
public void receive(Event event) {
if (meterRegistry == null) {
MeterRegistry mReg = meterRegistry;
if (mReg == null) {
logger.trace("Measurement not started. Skipping rule event processing");
return;
}
String topic = event.getTopic();
String ruleId = topic.substring(RULES_TOPIC_PREFIX.length(), topic.indexOf(RULES_TOPIC_SUFFIX));
if (!event.getPayload().contains(RuleStatus.RUNNING.name())) {
logger.trace("Skipping rule status info with status other than RUNNING {}", event.getPayload());
return;
}
} else {
String topic = event.getTopic();
String ruleId = topic.substring(RULES_TOPIC_PREFIX.length(), topic.indexOf(RULES_TOPIC_SUFFIX));
if (!event.getPayload().contains(RuleStatus.RUNNING.name())) {
logger.trace("Skipping rule status info with status other than RUNNING {}", event.getPayload());
return;
}

logger.debug("Rule {} RUNNING - updating metric.", ruleId);
Set<Tag> tagsWithRule = new HashSet<>(tags);
tagsWithRule.add(Tag.of(RULE_ID_TAG_NAME, ruleId));
String ruleName = getRuleName(ruleId);
if (ruleName != null) {
tagsWithRule.add(Tag.of(RULE_NAME_TAG_NAME, ruleName));
logger.debug("Rule {} RUNNING - updating metric.", ruleId);
Set<Tag> tagsWithRule = new HashSet<>(tags);
tagsWithRule.add(Tag.of(RULE_ID_TAG_NAME, ruleId));
String ruleName = getRuleName(ruleId);
if (ruleName != null) {
tagsWithRule.add(Tag.of(RULE_NAME_TAG_NAME, ruleName));
}
mReg.counter(METRIC_NAME, tagsWithRule).increment();
}
meterRegistry.counter(METRIC_NAME, tagsWithRule).increment();
}

private String getRuleName(String ruleId) {
Expand Down