Skip to content

AMBARI-26316: Add metrics for Agent Stomp and Api Stomp performance #3949

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.

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

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

sandeep318kumar
Copy link
Contributor

@sandeep318kumar sandeep318kumar commented Mar 3, 2025

What changes were proposed in this pull request?

added stomp stats metrics in ambari-server and a grafana dashboard as well to observe & track

(Please fill in changes proposed in this fix)

How was this patch tested?

Tested on local cluster.
image
image
image

(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)

Please review Ambari Contributing Guide before opening a pull request.

@JiaLiangC
Copy link
Contributor

@sandeep318kumar Thank you for your contribution. I've been busy with the 3.0 release, testing, compiling all Rocky 9 component RPMs, writing new website documentation, and haven't had time to review. I will review it next week.

@sandeep318kumar
Copy link
Contributor Author

@JiaLiangC Great work!!
I want to help with documentation, release work, testing. Can you tell how can I help you in these?

@JiaLiangC
Copy link
Contributor

@sandeep318kumar

I really appreciate your contributions, and I have requested the Ambari PMC to nominate you as a committer.

Ambari Development Status Update

Current Issues and Tasks

1. Trunk Branch Status

  • Release Status: Nearly ready for release
  • Outstanding Issues:
    • Frontend bug (AMBARI-26341): Component batch restart button is unresponsive
    • Trunk branch unit tests are failing
    • Ranger issues due to missing Python 3 patches in Bigtop's Ranger package, i will handle this.
      • Note: All other components work fine without Ranger

2. Documentation (3.0.0 Website)

3. Spring Branch

  • Unit tests are currently failing

4. Infrastructure

  • Currently developing RPM distribution website
  • Reason: Apache Infrastructure doesn't support distributing large packages (10GB+)

@sandeep318kumar
Copy link
Contributor Author

Wow man!! Amazing work! @JiaLiangC 👏🏻
I'll review docs and also check unit tests for spring branch.

@sandeep318kumar
Copy link
Contributor Author

@jojochuang can you review this?

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Looks almost ready to go! I think we should improve the nullity check in the configuration property. Having a better user doc would be nice but not required.

@@ -19,12 +19,13 @@

#################### Metrics Source Configs #####################

#Metric sources : jvm,database
#Metric sources : jvm,database, stats
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to add a section in user doc so users can discover this more easily.

*/
public static boolean isStompStatMetricsConfigured() {
MetricsConfiguration configuration = getMetricsConfiguration();
String commaSeparatedSources = configuration.getProperty("metric.sources");
Copy link
Contributor

Choose a reason for hiding this comment

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

no nullity check is a potential problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have handled null check for commaSeparatedSources. If stomp stats is not configured then it will return false.

}
sourceName = sourceName.trim();

String className = configuration.getProperty("source." + sourceName + ".class");
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

}
}, interval, interval, TimeUnit.SECONDS);
} catch (Exception e) {
LOG.info("Throwing exception when starting stomp stat source", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LOG.info("Throwing exception when starting stomp stat source", e);
LOG.info("Failed to start stomp stat source", e);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I've updated it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants