Payara icon indicating copy to clipboard operation
Payara copied to clipboard

Setting default-web-module disables /metrics - FISH-785

Open thehpi opened this issue 5 years ago • 8 comments

Description


When I set the default-web-module, e.g using asadmin:

set configs.config.server-config.http-service.virtual-server.server.default-web-module=test-web

Then when I request the root context / this will be forwarded to test-web application so these calls do the same

  • /test-web/rest/hello/world
  • /rest/hello/world

However the /metrics endpoint doesn't work anymore after this setting. This kind of makes sense because the root context is mapped to my webapp. But it doesn't feel ok that the /metrics endpoint (and probably more like /health) don't work anymore.

Expected Outcome

I would expect the /metrics endpoint to keep on working and return things like

# HELP vendor_system_cpu_load Display the "recent cpu usage" for the whole system. This value is a double in the [0.0,1.0] interval. A value of 0.0 means that all CPUs were idle during the recent period of time observed, while a value of 1.0 means that all CPUs were actively running 100% of the time during the recent period being observed. All values betweens 0.0 and 1.0 are possible depending of the activities going on in the system. If the system recent cpu usage is not available, the method returns a negative value.
vendor_system_cpu_load 0.02313624678663239
# TYPE base_memory_maxHeap_bytes gauge
# HELP base_memory_maxHeap_bytes Displays the maximum amount of memory in bytes that can be used for HeapMemory.
base_memory_maxHeap_bytes 7.29808896E9
# TYPE base_memory_committedNonHeap_bytes gauge

Current Outcome

When the default web module is set then the /metrics call returns 404.

I did find the following in MetricsServletContainerInitializer

public class MetricsServletContainerInitializer implements ServletContainerInitializer {

@Override
public void onStartup(Set<Class<?>> set, ServletContext ctx) throws ServletException {

    MetricsServiceConfiguration configuration = Globals.getDefaultBaseServiceLocator()
            .getService(MetricsServiceConfiguration.class);
    if (!"".equals(ctx.getContextPath())) {
        return;
    }

This indicates that metrics is only enabled for the root context. But this is only called on deployment AND when clearing the default-web-module. When setting the default web module this is not called but the /metrics endpoint doesn't work afterwards. So it looks like this relates to the problem but is not the only one.

Steps to reproduce (Only for bug reports)

See: https://github.com/thehpi/metrics-with-default-web-module

Follow the README

Environment

  • Payara Version: tested with 5.201 and 5.2020.4
  • Edition: Full
  • JDK Version: 8 Zulu
  • Operating System: Mac

thehpi avatar Sep 30 '20 09:09 thehpi

Yes, the MetricsServletContainerInitializer only binds to the root context if the application is already bound to the root context. If it becomes the default web module after it's deployed, it's not reinitialized and /metrics isn't accessible.

The current code of MetricsServletContainerInitializer works for Payara Micro, which can deploy an application to the root context at boot, but doesn't work with Payara Server, if the app is first deployed under an application context and then it becomes the default module and serves the root context without redeployment.

OndroMih avatar Sep 30 '20 11:09 OndroMih

Hi @jGauravGupta, in case @thehpi would like to create a PR with a fix, can you help him how to know whether the application is the default webmodule?

OndroMih avatar Oct 05 '20 11:10 OndroMih

I assume that all what's needed is to add a hook to the code that sets an app as the default web module and install the MetricsServlet, similarly to how it's installed during deployment in case the context root is empty. And also another hook to clean up if an app is unset as the default module.

OndroMih avatar Oct 05 '20 11:10 OndroMih

Hi @thehpi,

Thank you you for providing us with a very detailed reproducer. You seem to be very familiar with our codebase, is this something you will be interested in creating a PR with a fix?

MeroRai avatar Nov 24 '20 10:11 MeroRai

Reproduced the issue and created the internal ticket FISH-785 to address it.

jGauravGupta avatar Nov 25 '20 09:11 jGauravGupta

@MeroRai I'm not at all familiar with the payara codebase but I am familiar with using it :) But I can investigate of course and find a solution. But I would need some pointers.

As OndroMih noted in an earlier comment MetricsServletContainerInitializer doesn't work in payara server if an app is made default module after deployment.

What I find strange is that it IS called when the default module is cleared.

I guess what we need is:

  • when the default module is set this initializer is also called (need a pointer where that is done)
  • the initializer should also initialize metrcs when the app is the default module. I need a pointer on how te determine what default module is. Should I access the server config (how). Or is there a higher level service I can call (which one)

If you can help me a bit I will be able to create a PR

thehpi avatar Nov 27 '20 09:11 thehpi

I created a commit for ref which fixes the issue for metrics endpoint and requires restart on config change: https://github.com/jGauravGupta/Payara/commit/f8e09add4ef51c1eeed611d261680396f2489e01 A similar fix is needed for other MP endpoints

jGauravGupta avatar Nov 27 '20 15:11 jGauravGupta

I investigated why the setting default-web-module setting did not activate immediately but clearing it did. I ended in com.sun.enterprise.web.WebContainer and found that when setting the default-web-module the current default web module (__default-web-module) is unloaded and the web module to become default is updated but not restarted, hence the MetricsServiceContainerInitializer is not called. But when clearing the default-web-module the old default web module is actually (re)loaded and as such calls the initializer. So the solution could be to also reload the web module that is configured to become the default web module, but I don't know if thats a good idea.

thehpi avatar Nov 30 '20 21:11 thehpi