payara / Payara

Payara Server is an open source middleware platform that supports reliable and secure deployments of Java EE (Jakarta EE) and MicroProfile applications in any environment: on premise, in the cloud or hybrid.
http://www.payara.fish
Other
880 stars 301 forks source link

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

Open thehpi opened 3 years ago

thehpi commented 3 years ago

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

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

OndroMih commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

MeroRai commented 3 years ago

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?

jGauravGupta commented 3 years ago

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

thehpi commented 3 years ago

@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:

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

jGauravGupta commented 3 years ago

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

thehpi commented 3 years ago

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.