Javamelody support #450

dvtoever · 2017-05-12T09:35:18Z

Very basic setup to add a tab for javamelody

In your SBA project add a depencendy for spring-boot-admin-server-ui-javamelody

  <dependency>
  	<groupId>de.codecentric</groupId>
  	<artifactId>spring-boot-admin-server-ui-javamelody</artifactId>
  	<version>1.5.1-SNAPSHOT</version>
  </dependency>

Requires the following Configuration class in your SBA project

package net.bull.javamelody;

import de.codecentric.boot.admin.event.ClientApplicationDeregisteredEvent;
import de.codecentric.boot.admin.event.ClientApplicationRegisteredEvent;
import de.codecentric.boot.admin.model.Application;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.boot.web.servlet.ServletRegistrationBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.event.EventListener;

import javax.servlet.Filter;
import java.io.IOException;

/**
 * Unfortunately javamelody is rather closed, therefore use the same packagename to access package private logic
 */
@Configuration
public class JavaMelodyConfiguration {

	private static final Logger LOGGER = LoggerFactory.getLogger(JavaMelodyConfiguration.class);
	private static final String CONTEXT_ROOT = "/javamelody";

	@Bean
	public FilterRegistrationBean filterRegistrationBean() {
		final Filter javaMelodyFilter = new MonitoringFilter();

		FilterRegistrationBean javaMelodyFilterBean = new FilterRegistrationBean(javaMelodyFilter);
		javaMelodyFilterBean.addServletNames("monitoring");
		javaMelodyFilterBean.addUrlPatterns("/*");
		return javaMelodyFilterBean;
	}

	@Bean
	public ServletRegistrationBean collectorServletBean() {
		final CollectorServlet servlet = new CollectorServlet();
		ServletRegistrationBean servletRegistrationBean = new ServletRegistrationBean(servlet);
		servletRegistrationBean.addUrlMappings(CONTEXT_ROOT);

		return servletRegistrationBean;
	}

	@EventListener
	public void onClientApplicationRegistered(ClientApplicationRegisteredEvent event) {
		final Application application = event.getApplication();
		final String applicationName = application.getName() + "-" + application.getId();

		try {
			Parameters.addCollectorApplication(applicationName, Parameters.parseUrl(application.getServiceUrl()));
			LOGGER.info("Added application {} to java melody", applicationName);
		} catch (IOException e) {
			LOGGER.warn("Failed to register application {} to java melody" + applicationName);
		}
	}

	@EventListener
	public void onClientApplicationDeregistered(ClientApplicationDeregisteredEvent event) {
		final Application application = event.getApplication();
		final String applicationName = application.getName() + "-" + application.getId();

		try {
			Parameters.removeCollectorApplication(applicationName);
			LOGGER.info("Removed application {} from java melody", applicationName);
		} catch (IOException e) {
			LOGGER.warn("Failed to unregister application {} from java melody" + applicationName);
		}
	}

}

…used (e.g. using method shorthands)

joshiste · 2017-05-12T12:32:15Z

You could turn the JavaMelodyConfiguration into an autoconfiguration and include it (as I did in the tubrine module)

joshiste · 2017-06-16T14:32:36Z

spring-boot-admin-server-ui-javamelody/src/main/java/net/bull/javamelody/JavaMelodyService.java

+ * register and unregister applications. Using the {@link EventListener annotations} we receive notifications from service discovery.
+ */
+@Service
+public class JavaMelodyService {


Tests are missing for this class.

joshiste · 2017-06-16T14:33:19Z

spring-boot-admin-server-ui-javamelody/src/main/java/net/bull/javamelody/JavaMelodyService.java

+ * Service class in the javamelody package, so we can access the javamelody Parameters class in which we can
+ * register and unregister applications. Using the {@link EventListener annotations} we receive notifications from service discovery.
+ */
+@Service


Please don't rely on component scan - it might be disabled in the users' projects... since the service is in the config just remove the annotation

joshiste · 2017-06-16T14:36:28Z

Sorry for coming back to this PR this late. Looking good so far. I left you some comments on minor issues. coul you also add a short chapter to the docs on how to use this module?

dvtoever · 2017-07-11T11:26:41Z

Late reply from my side as well :)

Here are the requested changes

joshiste

Additional to the blocker (see comment in net.bull.javamelody.JavaMelodyService) the JavaMelodyServiceTest fails.

joshiste · 2017-07-24T19:25:27Z

spring-boot-admin-server-ui-javamelody/src/main/java/net/bull/javamelody/JavaMelodyService.java

@@ -0,0 +1,46 @@
+package net.bull.javamelody;


Sorry that I spotted the foreign package declaration this late, but we can't include classes in foreign packages. It must be a de.codecentric package.

The downside seems that you cannot access the net.bull.javamelody.Parameters. So either you ask the javamelody guys to add a java-api to register applications in java melody or you need to make a post request to javameldoy's CollectorServlet.

dvtoever · 2017-08-15T13:18:32Z

I created an issue with the Javamelody project javamelody/javamelody#661

proposed changes for review in codecentric#450

coveralls · 2017-09-02T08:15:33Z

Coverage increased (+0.3%) to 86.771% when pulling 46dbaa5 on dvtoever:feature-javamelody into 0a029fc on codecentric:master.

evernat · 2017-08-15T13:56:08Z

spring-boot-admin-docs/src/main/asciidoc/server-ui-modules.adoc

+    javaMelody.addUrlPatterns("/*");
+    javaMelody.setFilter(filter);
+    return javaMelody;
+}


Instead of javamelody-core dependency and of javaMelody FilterRegistrationBean (which is not enough anyway), I suggest to show an example to add javamelody-spring-boot-starter in pom.xml. For example:

 <dependency> <groupId>net.bull.javamelody</groupId> <artifactId>javamelody-spring-boot-starter</artifactId> <version>${javamelody.version}</version> </dependency>

evernat · 2017-08-15T14:14:21Z

spring-boot-admin-server-ui-javamelody/pom.xml

+			<groupId>net.sf.ehcache</groupId>
+			<artifactId>ehcache-core</artifactId>
+			<version>${ehcache.version}</version>
+		</dependency>


ehcache dependency is not needed here anymore: it can be removed.
And I suggest to add the iText 2.1.7 dependency (licence MPL or LGPL at your choice) and the Log4J dependency, in order to have PDF links in reports (I think PDF links are important in the reports) and to have some logs:

<dependency> <groupId>com.lowagie</groupId> <artifactId>itext</artifactId> <version>2.1.7</version> <exclusions> <exclusion> <groupId>bouncycastle</groupId> <artifactId>bcmail-jdk14</artifactId> </exclusion> <exclusion> <groupId>bouncycastle</groupId> <artifactId>bcprov-jdk14</artifactId> </exclusion> <exclusion> <artifactId>bctsp-jdk14</artifactId> <groupId>bouncycastle</groupId> </exclusion> </exclusions> </dependency> <dependency> <groupId>log4j</groupId> <artifactId>log4j</artifactId> <version>1.2.17</version> </dependency>

You could also copy the collector server log4j.xml file in resources of this module.

And XStream 1.4.10 dependency and aws-java-sdk-cloudwatch dependency could possibly be added to be able to export data as JSON or XML and to export data to AWS CloudWatch, but they are less important and I suppose that they could be added by the "admin" user.

I suggest we add this in the documentation instead of the pom.xml so users can choose for themselves if they want javamelody reporting to work. Personally I don't use it and rather don't have a transitive log4j/itext dependency

No, I think that many users would be really happy to have the pdf reports. And, adding a iText dependency would be debatable in a monitored app but this dependency (with exclusions on transitive dependencies) is in my opinion not a problem in spring-boot-admin. So I think that we should really keep the iText dependency like it is now.
For Log4J, it is not included at the moment and finally it seems not needed since there is already a log4j-over-slf4j dependency in spring-boot-admin.

evernat · 2017-08-15T14:26:09Z

...dy/src/main/java/spring/boot/admin/javamelody/configuration/JavamelodyAutoConfiguration.java

+        javaMelodyFilterBean.addServletNames("monitoring");
+        javaMelodyFilterBean.addUrlPatterns("/*");
+        return javaMelodyFilterBean;
+    }


This filterRegistrationBean() is not needed to make the collector server work and it should be removed I suppose. (There is a CollectorServlet below.)

Or if what you want is to monitor the spring-boot-admin server itself, then it is not enough (javamelody-spring-boot-starter may be better for this).

The idea was to monitor spring boot admin itself. Why is this not enough?

The FilterRegistrationBean is not enough because in general a SessionListener is also installed. See https://github.com/javamelody/javamelody/blob/master/javamelody-spring-boot-starter/src/main/java/net/bull/javamelody/JavaMelodyAutoConfiguration.java#L87
Then the spring-boot-starter also adds monitoring of RestControllers, RestTemplates, Services, spring context and more.
So adding the javamelody-spring-boot-starter to monitor the spring-boot-admin server should be much simpler than adding filter, sessionlistener, etc.

evernat · 2017-08-25T21:01:31Z

spring-boot-admin-server-ui-javamelody/src/main/java/net/bull/javamelody/JavaMelodyService.java

+        final String applicationName = application.getName() + "-" + application.getId();
+
+        try {
+            Parameters.addCollectorApplication(applicationName, Parameters.parseUrl(application.getServiceUrl()));


Parameters.addCollectorApplication(applicationName, Parameters.parseUrl(application.getServiceUrl())); to be replaced by CollectorServlet.addCollectorApplication(applicationName, application.getServiceUrl())
(using the latest javamelody-core snapshot)

evernat · 2017-08-25T21:02:25Z

spring-boot-admin-server-ui-javamelody/src/main/java/net/bull/javamelody/JavaMelodyService.java

+        final String applicationName = application.getName() + "-" + application.getId();
+
+        try {
+            Parameters.removeCollectorApplication(applicationName);


to be replaced by CollectorServlet.removeCollectorApplication(applicationName);

evernat · 2017-08-27T00:11:40Z

spring-boot-admin-server-ui-javamelody/pom.xml

+	<artifactId>spring-boot-admin-server-ui-javamelody</artifactId>
+
+    <properties>
+        <javamelody.version>1.67.0</javamelody.version>


<javamelody.version>1.69.0</javamelody.version> can now be used to use the new methods in CollectorServlet

dvtoever · 2017-10-04T08:17:23Z

What is currently preventing this pull request from being merged?

joshiste · 2017-10-05T15:29:05Z

Sorry I'm currently short on time. And currently want to invest the spare time into the adaption of spring boot 2.0.

joshiste · 2018-03-05T10:50:20Z

So will you rework the code for 2.x?

evernat · 2018-05-11T23:22:30Z

@joshiste

So will you rework the code for 2.x?

Do you mean rework the PR (as a new PR certainly) to work with spring-boot-admin 2.0.0-SNAPSHOT?
If yes, can you help us by saying what files in the PR are not good anymore and if possible how those files should be changed? And is there an example similar to this PR or should the (new) PR be the new example of such integration?

joshiste · 2018-05-13T13:35:55Z

@evernat I guess (mostly) all of it has to be reworked. The ui is now using Vue instead of Angular and also the backend has now a slightly different domain model.

joshiste · 2018-05-14T21:23:10Z

@evernat I do really like the fact that you published the module 1.5.x as external module, as it doesn't adds the burden of maintaining this on me.
Once #683 is resolved I'd like to go down this route for 2.0.x as well. Do you agree, so that I can close this PR?

evernat · 2018-05-15T23:38:14Z

@joshiste I agree for the module in version 2.0.0 as an external module, if the module is migrated one day.
But @dvtoever is the main concerned by this PR and maybe also by the module migration and by #683 whatever it means, so I suggest to have his voice on this PR.

dvtoever · 2018-05-16T09:41:44Z

I agree. Will close this PR

- initial version for javamelody support in SBA

2971093

dvtoever mentioned this pull request May 12, 2017

integrate JavaMelody #388

Closed

- fix 2 out of 4 codacy issues, don't know if ECMAscript 6 should be …

9b3d549

…used (e.g. using method shorthands)

Daniel van 't Oever added 2 commits May 15, 2017 13:16

- Added autoconfiguration as suggested by Johannes Edmeier

d991c65

fix conditionalOn property and fix angular controller

9cc8bdc

joshiste changed the title ~~- initial version for javamelody support in SBA~~ Javamelody support in SBA Jun 8, 2017

joshiste changed the title ~~Javamelody support in SBA~~ Javamelody support Jun 8, 2017

joshiste reviewed Jun 16, 2017

View reviewed changes

codecentric deleted a comment Jun 16, 2017

joshiste added enhancement server server-ui labels Jun 16, 2017

- Added documentation, unit test and other review comments

df4b967

- removed unused import

6ff4a22

codecentric deleted a comment Jul 11, 2017

joshiste requested changes Jul 24, 2017

View reviewed changes

codecentric deleted a comment Jul 24, 2017

joshiste removed enhancement server server-ui labels Jul 24, 2017

dvtoever mentioned this pull request Aug 15, 2017

Open up Parameters class for integration in Spring Boot Admin project javamelody/javamelody#661

Closed

evernat added a commit to javamelody/javamelody that referenced this pull request Aug 25, 2017

change addCollectorApplication for codecentric/spring-boot-admin#450

510ad0a

evernat and others added 3 commits September 1, 2017 22:40

Merge remote-tracking branch 'refs/remotes/origin/feature-javamelody'

139bc42

proposed changes for review in codecentric#450

5149825

Merge pull request #1 from evernat/master

46dbaa5

proposed changes for review in codecentric#450

evernat approved these changes Sep 6, 2017

View reviewed changes

joshiste force-pushed the master branch from b90b4cf to d1fc2fc Compare January 13, 2018 17:16

joshiste force-pushed the master branch 2 times, most recently from bdfc23b to 1279ae8 Compare February 22, 2018 19:26

joshiste force-pushed the master branch 2 times, most recently from 8ef0059 to d7acee4 Compare March 18, 2018 13:14

evernat added a commit to javamelody/spring-boot-admin-server-ui-javamelody that referenced this pull request May 12, 2018

by dvtoever, from codecentric/spring-boot-admin#450

f9eb88d

evernat mentioned this pull request May 13, 2018

Publish spring-boot-admin-server-ui-javamelody javamelody/spring-boot-admin-server-ui-javamelody#1

Open

dvtoever closed this May 16, 2018

goldyliang pushed a commit to goldyliang/javamelody that referenced this pull request Mar 17, 2022

change addCollectorApplication for codecentric/spring-boot-admin#450

46712f7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Javamelody support #450

Javamelody support #450

dvtoever commented May 12, 2017

joshiste commented May 12, 2017

joshiste Jun 16, 2017

joshiste Jun 16, 2017 •

edited

Loading

joshiste commented Jun 16, 2017

dvtoever commented Jul 11, 2017

joshiste left a comment •

edited

Loading

joshiste Jul 24, 2017 •

edited

Loading

dvtoever commented Aug 15, 2017

coveralls commented Sep 2, 2017

evernat Aug 15, 2017

evernat Aug 15, 2017

dvtoever Sep 7, 2017

evernat Sep 7, 2017

evernat Aug 15, 2017

dvtoever Sep 7, 2017

evernat Sep 7, 2017 •

edited

Loading

evernat Aug 25, 2017

evernat Aug 25, 2017

evernat Aug 27, 2017

dvtoever commented Oct 4, 2017

joshiste commented Oct 5, 2017

joshiste commented Mar 5, 2018

evernat commented May 11, 2018 •

edited

Loading

joshiste commented May 13, 2018

joshiste commented May 14, 2018 •

edited

Loading

evernat commented May 15, 2018

dvtoever commented May 16, 2018

Javamelody support #450

Javamelody support #450

Conversation

dvtoever commented May 12, 2017

joshiste commented May 12, 2017

Choose a reason for hiding this comment

joshiste Jun 16, 2017 • edited Loading

Choose a reason for hiding this comment

joshiste commented Jun 16, 2017

dvtoever commented Jul 11, 2017

joshiste left a comment • edited Loading

Choose a reason for hiding this comment

joshiste Jul 24, 2017 • edited Loading

Choose a reason for hiding this comment

dvtoever commented Aug 15, 2017

coveralls commented Sep 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evernat Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvtoever commented Oct 4, 2017

joshiste commented Oct 5, 2017

joshiste commented Mar 5, 2018

evernat commented May 11, 2018 • edited Loading

joshiste commented May 13, 2018

joshiste commented May 14, 2018 • edited Loading

evernat commented May 15, 2018

dvtoever commented May 16, 2018

joshiste Jun 16, 2017 •

edited

Loading

joshiste left a comment •

edited

Loading

joshiste Jul 24, 2017 •

edited

Loading

evernat Sep 7, 2017 •

edited

Loading

evernat commented May 11, 2018 •

edited

Loading

joshiste commented May 14, 2018 •

edited

Loading