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

Jump to bottom

WIP: AMQ-9552: Add metric lastMessageTimestamp to StatisticsBroker based o… #1285

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP: AMQ-9552: Add metric lastMessageTimestamp to StatisticsBroker based o… #1285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c307df
AMQ-9552: Add metric lastMessageTimestamp to StatisticsBroker based o…
Aug 23, 2024
7a2421b
Merge branch 'apache:main' into AMQ-9552
ggkochanski Sep 22, 2024
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
5 changes: 5 additions & 0 deletions activemq-broker/src/main/java/org/apache/activemq/plugin/StatisticsBroker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ else if (dest instanceof Topic) {
tempFirstMessage.clear();
}
}
if ( stats.getEnqueues().hasUpdated() ) {
// NOTICE: Client-side, you may get the broker "now" Timestamp by msg.getJMSTimestamp()
// This allows for calculating inactivity.
statsMessage.setLong("lastMessageTimestamp", stats.getEnqueues().getLastSampleTime());
Copy link
Author

Choose a reason for hiding this comment

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

@mattrpav I can see your change is merged, so I'm refreshing the pull request.

I understand the name unification motivation. Should I change name lastMessageTimestamp into lastEnqueTimestamp here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ggkochanski The field naming is a broader conversation. The full scope is not one field as you have in this PR. We currently have a number of metrics (and config params) that are not fully aligned in naming convention, so I would press for an organized approach for these new metrics.

Related JIRAs:
ref: https://issues.apache.org/jira/browse/AMQ-4358
ref: https://issues.apache.org/jira/browse/AMQ-5497

Copy link
Author

Choose a reason for hiding this comment

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

What is the meaning LastDequeueTime and FirstDequeueTime? I believe one of them cannot be determined (if the last means the newest enqueued message then it is not dequeued yet)

@mattrpav could you explain in details how you imagine this new approach to metrics based on some example preferably?

Maybe I try to use statistics in the wrong way. What I need to know to assess the condition of mq system is: enqueue count and last update timestamp, dequeue count and last update, last process time (iso "averageEnqueueTime") and the age of the oldest message in queue. What do you suggest me?

Copy link
Contributor

@mattrpav mattrpav Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@ggkochanski yeah, the challenge is settling on the terminology for the metric values. For example, does 'last' mean last-as-in-time, or last-as-in-last-message-in-the-queue (most recently added).

This feature will be enabled via PolicyEntry property, such as advancedTimeStatisticsEnabled="true". This will allow existing users to not incur the potential latency penalty and users that wish to have increased observability can simply toggle the parameter.

}
statsMessage.setJMSCorrelationID(messageSend.getCorrelationId());
sendStats(producerExchange.getConnectionContext(), statsMessage, replyTo);
}
Expand Down
7 changes: 7 additions & 0 deletions activemq-client/src/main/java/org/apache/activemq/management/StatisticImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class StatisticImpl implements Statistic, Resettable {
private String description;
private long startTime;
private long lastSampleTime;
private boolean hasUpdated;
private boolean doReset = true;

public StatisticImpl(String name, String unit, String description) {
Expand All @@ -38,17 +39,20 @@ public StatisticImpl(String name, String unit, String description) {
this.description = description;
this.startTime = System.currentTimeMillis();
this.lastSampleTime = this.startTime;
this.hasUpdated = false;
}

public synchronized void reset() {
if(isDoReset()) {
this.startTime = System.currentTimeMillis();
this.lastSampleTime = this.startTime;
this.hasUpdated = false;
}
}

protected synchronized void updateSampleTime() {
this.lastSampleTime = System.currentTimeMillis();
this.hasUpdated = true;
}

public synchronized String toString() {
Expand Down Expand Up @@ -101,6 +105,9 @@ public boolean isDoReset() {
return this.doReset;
}

public boolean hasUpdated(){
return hasUpdated;
}
/**
* @param doReset the doReset to set
*/
Expand Down
4 changes: 4 additions & 0 deletions activemq-unit-tests/src/test/java/org/apache/activemq/management/TimeStatisticTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ public void testStatistic() throws Exception {
TimeStatisticImpl stat = new TimeStatisticImpl("myTimer", "millis", "myDescription");
assertStatistic(stat, "myTimer", "millis", "myDescription");

assertFalse(stat.hasUpdated());

assertEquals(0, stat.getCount());

stat.addTime(100);
assertTrue(stat.hasUpdated());
assertEquals(1, stat.getCount());
assertEquals(100, stat.getMinTime());
assertEquals(100, stat.getMaxTime());
Expand All @@ -59,6 +62,7 @@ public void testStatistic() throws Exception {
LOG.info("Stat is: " + stat);

stat.reset();
assertFalse(stat.hasUpdated());

assertEquals(0, stat.getCount());
assertEquals(0, stat.getMinTime());
Expand Down
1 change: 1 addition & 0 deletions activemq-unit-tests/src/test/java/org/apache/activemq/plugin/BrokerStatisticsPluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ public void testDestinationStatsWithFirstMessageTimestamp() throws Exception {
assertEquals(1, reply.getLong("size"));
assertTrue(reply.getJMSTimestamp() > 0);
assertTrue(reply.getLong("firstMessageTimestamp") > 0);
assertTrue(reply.getLong("lastMessageTimestamp") > 0);
// Assert that we got the brokerInTime for the first message in queue as value of key "firstMessageTimestamp"
assertTrue(System.currentTimeMillis() >= reply.getLong("firstMessageTimestamp"));
assertEquals(Message.DEFAULT_PRIORITY, reply.getJMSPriority());
Expand Down