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.

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

Expose service port as a system property, mainly to support use-case where configured port is zero and picked by OS #686

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

Conversation

YannRobert
Copy link

Expose service port as a system property, mainly to support use-case where configured port is zero and picked by OS

When adding jmx_exporter java agent, you typically provide a port number as configuration.

However, you may want to rely on the operating system to pick a port number at startup time, so that you don't need to bother with assigning a different port number for the many JVMs that may be running on the same machine.

In that case, it become very difficult to discover which port what used for this particular process. (you may need to rely on various command lines to get the port for a process, still when the process opens multiple other ports this is becoming quite impossible to do, and platform dependantt)

Exposing the actual port as a system property is an handy solution.
In this case, it become easy to get the value of the system property, from any other code running inside the JVM.

This other (application) code could then advertise this information into any discovery service.

…r the actual port used, in case the configured port is 0

expose configured port number and actual port number as system properties

Signed-off-by: Yann ROBERT <yann.robert@anantaplex.fr>
@dhoard
Copy link
Collaborator

dhoard commented Feb 17, 2022

Most systems that access the jmx_exporter HTTP endpoint require a known declarative port for configuration.

In an ideal deployment / operating environment, CI/CD would be used using declarative configuration for deployment.

I don't see an issue with the code, but don't see any benefits.

@YannRobert
Copy link
Author

YannRobert commented Feb 19, 2022

Hello Doug, thank you for your reply.

Let me tell you more about one possible use-case :

Imagine a system composed of many microservices.
In each environment, you have more or less instances of each type of microservices.
You do not use containers (docker or kubernetes), and you use few machines (either virtual or bare metal) so that one machine will host multiple/many JVMs.
So the same network interface (either external or loopback) will have many ports opens.

Maybe also, you do not want to be limited by deployed configuration, and be able to start many more instances than it was predicted (scale up the number of instances, on the same machine).
Or you use an orchestrator that will maintain the desired number of instances, even when one of the host machine is down, increasing the number of instances on other machines.

Sure you could maintain a configuration for each environment,
or write a configuration generator in order to prepare config file and startup scripts,
or the startup script could try to guess an available port at runtime, then pass it to the JVM command line (hoping no race condition occurs and the port will still be available when jmx_exporter actually binds the port)

That will put a different port number for each instance of a JVM.
That would be possible, but that would be some (hard) work, and depending on the actual configuration/scripting implementation, it would be more or less robust.

Now imagine you use Eureka as a service discovery system (Prometheus supports Eureka, it can be any other discovery system).
When a JVM is started with jmx_exporter agent, the application that runs inside the JVM will find the port number used by jmx_exporter (parsing the command-line maybe), and put it into the service discovery (as a metadata).

So now, clients (like Prometheus) can use service discovery to locate all of the jmx_exporter instances.

Since service discovery is used in order to find jmx_exporter URLs, there is no need for clients (like Prometheus) to know the port number in advance.

It's would be only required because you cannot find the port number used by jmx_exporter if you configure it will port 0 (zero).
(you would have to use netstat or something similar, still it would be very hazardous)

Now when jmx_exporter includes the proposed change (present Pull Request), it becomes possible to configure port 0 and still be able to easily find the port number used by jmx_exporter.
Using only System.getProperty(...) (it's easier even when you use a fixed port number)

Now it's not only possible to address robustness issues, but it's also easy. And almost zero configuration or devops work is needed.

Sure there may be a few downsides in using port 0, for instance when you need to have strict network security rules (ports need to be known and declared in advance to be allowed by firewall).
But they are not concerns for everyone, or can be addressed.
When you want to use port zero and the benefits overcomes the downsides, at least now you can.

Finally, since jmx_exporter is a tool that provides observability on JVM, I think it's nice that jmx_exporter provides observability on itself.
At least to support this particular case when port zero is configured (make it not only work, but also usable in the real world).

@dhoard
Copy link
Collaborator

dhoard commented Feb 23, 2022

@YannRobert Thanks for the detailed explanation.

  1. Authentication will be added to the jmx_exporter endpoint. How would that be handled? or would it be handled at all?
  2. Are there scenarios where more than the listening port needs to be available via System properties?

I feel if we are going to provide insight to the jmx_exporter it should be in a documented way (pattern) so that other exporters could follow. (Property naming standard, namespacing, etc.)

Edit:

Not sure I see any value providing the configured port.

(Not a maintainer... just my opinions.)

@YannRobert
Copy link
Author

YannRobert commented Feb 24, 2022

@dhoard

Authentication will be added to the jmx_exporter endpoint. How would that be handled? or would it be handled at all?

I'm sorry, I cannot see a link between this PR and authentication.

Are there scenarios where more than the listening port needs to be available via System properties?

That is a very good point.

The immediate minimal need is to be able to get the port number actually used to bind the HTTP server (including when zero is configured as the port number).
Also this is probably the only information that cannot be known upfront since it is not part of the jmx_exporter configuration.

Other information are part of the configuration. So we may consider the information is available with minimal effort.

But since you ask, I would probably like to have easy access to all java agent parameters as System properties :

  • the bind address (empty or 0.0.0.0 or 127.0.0.1 or any actual ip address)
  • the path to the yaml config file
    This would avoid the application to parse the command line for the java agent parameters, or to inject the same parameters in the application's own configuration.

So as you can see, there won't ever be a lot of properties to expose.

I feel if we are going to provide insight to the jmx_exporter it should be in a documented way (pattern) so that other exporters could follow. (Property naming standard, namespacing, etc.)

I proposed a namespace in this PR's code. This can be changed to anything else if contributors/maintainers can think of a better property name.

Not sure I see any value providing the configured port.

I am not using it myself, still I find it useful to emphasize the fact that actual port can differ from configured port.
I agree in the end the application probably don't care about the configured port.

I understand this PR may trigger discussion about what else to expose and how.
Would that be reasonable to quickly accept the PR to solve raised issue about port zero, then only after to discuss about exposing or not exposing additional parameters the same way?

@fstab @tomwilkie Any thoughts ?

@YannRobert
Copy link
Author

Hello @fstab
Could you please give a look and share your thoughts ?

Could this PR or any alternative solution be considered to be merged ?

Many thanks
Yann

@dhoard
Copy link
Collaborator

dhoard commented Jun 4, 2022

@YannRobert If a SecurityManager is being used that disallows PropertyPermission(key, "write") the code will fail (throw a SecurityException. In this scenario, how do we handle the SecurityException?

Options

  • A. If the code allows the SecurityException to be thrown, it could break existing users (unintended behavior change)
  • B. If the code catches the SecurityException, the functionality doesn't work. (silent functionality failure)

I feel that we can't allow A and B could lead to frustration for those wanting the functionality.

DevOps work will be required to install the core application using the agent, so adding additional configuration as part of that work makes the most sense to me.

Additionally, System/Application administrators commonly test the agent HTTP endpoint. If the port is allocated dynamically, there is no easy way for them to know the port in use. (lack of consistency)

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

Successfully merging this pull request may close these issues.

None yet

2 participants