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.

Sign up for GitHub

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

Jump to bottom

Expose server config, session queue and node as MBeans for JMX monitoring. #8838

Merged
merged 12 commits into from
Dec 22, 2020
Merged

Expose server config, session queue and node as MBeans for JMX monitoring. #8838

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c1713e6
Expose server config, session queue and node as MBeans for JMX monito…
pujagani Nov 4, 2020
2c9e6ed
Merge branch 'trunk' into expose-server-jmx
diemol Dec 21, 2020
19ecb5c
Making tests compile again
diemol Dec 21, 2020
82f4240
[grid] Add JMX queuer tests. Fix indentation.
pujagani Dec 22, 2020
6d0b83c
[grid] Use isZero() in JMXTest
pujagani Dec 22, 2020
22f686a
[grid] Fix newsessionqueue config test.
pujagani Dec 22, 2020
e215be6
[grid] Update assert for JMXTest.
pujagani Dec 22, 2020
d7dabf0
[grid] Fix node JMXTest.
pujagani Dec 22, 2020
abb6525
[grid] Avoid starting the server to test JMX registration.
pujagani Dec 22, 2020
8cd8f64
[grid] Unregister MXBean before testing.
pujagani Dec 22, 2020
c608c76
Addressing comments to merge this PR
diemol Dec 22, 2020
5c1302a
Merge branch 'trunk' into expose-server-jmx
diemol Dec 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/grid/node/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ java_library(
"//java/server/src/org/openqa/selenium/grid/security",
"//java/server/src/org/openqa/selenium/grid/web",
"//java/server/src/org/openqa/selenium/status",
"//java/server/src/org/openqa/selenium/remote/server/jmx",
artifact("com.google.guava:guava"),
],
)
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/grid/node/local/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ java_library(
"//java/server/src/org/openqa/selenium/grid/node/config",
"//java/server/src/org/openqa/selenium/grid/security",
"//java/server/src/org/openqa/selenium/grid/server",
"//java/server/src/org/openqa/selenium/remote/server/jmx",
artifact("com.google.guava:guava"),
],
)
50 changes: 50 additions & 0 deletions java/server/src/org/openqa/selenium/grid/node/local/LocalNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.concurrent.Regularly;
import org.openqa.selenium.events.EventBus;
import org.openqa.selenium.grid.data.Availability;
import org.openqa.selenium.grid.data.CreateSessionRequest;
import org.openqa.selenium.grid.data.CreateSessionResponse;
import org.openqa.selenium.grid.data.NodeDrainComplete;
Expand All @@ -53,6 +54,9 @@
import org.openqa.selenium.remote.SessionId;
import org.openqa.selenium.remote.http.HttpRequest;
import org.openqa.selenium.remote.http.HttpResponse;
import org.openqa.selenium.remote.server.jmx.JMXHelper;
import org.openqa.selenium.remote.server.jmx.ManagedAttribute;
import org.openqa.selenium.remote.server.jmx.ManagedService;
import org.openqa.selenium.remote.tracing.AttributeKey;
import org.openqa.selenium.remote.tracing.EventAttribute;
import org.openqa.selenium.remote.tracing.EventAttributeValue;
Expand Down Expand Up @@ -91,6 +95,8 @@
import static org.openqa.selenium.remote.http.Contents.string;
import static org.openqa.selenium.remote.http.HttpMethod.DELETE;

@ManagedService(objectName = "org.seleniumhq.grid:type=Node,name=LocalNode",
description = "Node running the webdriver sessions.")
public class LocalNode extends Node {

private static final Json JSON = new Json();
Expand Down Expand Up @@ -174,6 +180,8 @@ private LocalNode(
}
}
}));

new JMXHelper().register(this);
}

@Override
Expand All @@ -182,11 +190,53 @@ public boolean isReady() {
}

@VisibleForTesting
@ManagedAttribute(name = "CurrentSessions")
public int getCurrentSessionCount() {
// It seems wildly unlikely we'll overflow an int
return Math.toIntExact(currentSessions.size());
}

@ManagedAttribute(name = "MaxSessions")
public int getMaxSessionCount() {
return maxSessionCount;
}

@ManagedAttribute(name = "Status")
public Availability getAvailability() {
return isDraining() ? DRAINING : UP;
}

@ManagedAttribute(name = "TotalSlots")
public int getTotalSlots() {
return factories.size();
}

@ManagedAttribute(name = "UsedSlots")
public long getUsedSlots() {
return factories.stream().filter(sessionSlot -> !sessionSlot.isAvailable()).count();
}

@ManagedAttribute(name = "Load")
public float getLoad() {
long inUse = factories.stream().filter(sessionSlot -> !sessionSlot.isAvailable()).count();
return inUse / (float) maxSessionCount * 100f;
}

@ManagedAttribute(name = "RemoteNodeUri")
public URI getExternalURI() {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: getExternalUri We try and AVOID SHOUTING in Selenium (which is why we have HttpMethod, and so on)

return externalUri;
}

@ManagedAttribute(name = "GridUri")
public URI getGridURI() {
return gridUri;
}

@ManagedAttribute(name = "NodeId")
public String getNodeId() {
return getId().toString();
}

@Override
public boolean isSupporting(Capabilities capabilities) {
return factories.parallelStream().anyMatch(factory -> factory.test(capabilities));
Expand Down
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/grid/server/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ java_library(
"//java/server/src/org/openqa/selenium/grid/component",
"//java/server/src/org/openqa/selenium/grid/config",
"//java/server/src/org/openqa/selenium/grid/web",
"//java/server/src/org/openqa/selenium/remote/server/jmx",
artifact("com.beust:jcommander"),
artifact("com.google.guava:guava"),
artifact("javax.servlet:javax.servlet-api"),
Expand Down
19 changes: 19 additions & 0 deletions java/server/src/org/openqa/selenium/grid/server/BaseServerOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,20 @@
import org.openqa.selenium.net.HostIdentifier;
import org.openqa.selenium.net.NetworkUtils;
import org.openqa.selenium.net.PortProber;
import org.openqa.selenium.remote.server.jmx.JMXHelper;
import org.openqa.selenium.remote.server.jmx.ManagedAttribute;
import org.openqa.selenium.remote.server.jmx.ManagedService;

import java.io.File;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Optional;
import java.util.logging.Logger;

@ManagedService(objectName = "org.seleniumhq.grid:type=Config,name=BaseServerConfig",
description = "Server config")
public class BaseServerOptions {

private static final String SERVER_SECTION = "server";
Expand All @@ -40,12 +47,14 @@ public class BaseServerOptions {

public BaseServerOptions(Config config) {
this.config = config;
new JMXHelper().register(this);
}

public Optional<String> getHostname() {
return config.get(SERVER_SECTION, "hostname");
}

@ManagedAttribute(name = "Port")
public int getPort() {
if (port == -1) {
int newPort = config.getInt(SERVER_SECTION, "port")
Expand All @@ -61,6 +70,7 @@ public int getPort() {
return port;
}

@ManagedAttribute(name = "MaxServerThreads")
public int getMaxServerThreads() {
int count = config.getInt(SERVER_SECTION, "max-threads")
.orElse(200);
Expand Down Expand Up @@ -94,6 +104,15 @@ public URI getExternalUri() {
}
}

@ManagedAttribute(name = "URL")
public URL getExternalUrl() {
Copy link
Member

Choose a reason for hiding this comment

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

Daft question, but why don't we just return the URI?

try {
return getExternalUri().toURL();
} catch (MalformedURLException e) {
throw new ConfigException("Cannot convert URI to URL" + e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

nit: We probably want a space after URL

}
}

public boolean getAllowCORS() {
return config.getBool(SERVER_SECTION, "allow-cors").orElse(false);
}
Expand Down
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/grid/sessionqueue/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ java_library(
"//java/server/src/org/openqa/selenium/grid/data",
"//java/server/src/org/openqa/selenium/grid/log",
"//java/server/src/org/openqa/selenium/status",
"//java/server/src/org/openqa/selenium/remote/server/jmx",
artifact("com.google.guava:guava"),
],
)
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/grid/sessionqueue/config/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ java_library(
"//java:auto-service",
"//java/server/src/org/openqa/selenium/grid/config",
"//java/server/src/org/openqa/selenium/grid/sessionqueue",
"//java/server/src/org/openqa/selenium/remote/server/jmx",
artifact("com.beust:jcommander"),
artifact("io.opentelemetry:opentelemetry-api"),
],
Expand Down
17 changes: 17 additions & 0 deletions java/server/src/org/openqa/selenium/grid/sessionqueue/config/NewSessionQueueOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,15 @@

import org.openqa.selenium.grid.config.Config;
import org.openqa.selenium.grid.sessionqueue.NewSessionQueue;
import org.openqa.selenium.remote.server.jmx.JMXHelper;
import org.openqa.selenium.remote.server.jmx.ManagedAttribute;
import org.openqa.selenium.remote.server.jmx.ManagedService;

import java.time.Duration;
import java.time.temporal.ChronoUnit;

@ManagedService(objectName = "org.seleniumhq.grid:type=Config,name=NewSessionQueueConfig",
description = "New session queue config")
public class NewSessionQueueOptions {

private static final String SESSIONS_QUEUE_SECTION = "sessionqueue";
Expand All @@ -35,6 +41,7 @@ public class NewSessionQueueOptions {

public NewSessionQueueOptions(Config config) {
this.config = config;
new JMXHelper().register(this);
}

public Duration getSessionRequestTimeout() {
Expand All @@ -57,6 +64,16 @@ public Duration getSessionRequestRetryInterval() {
return Duration.ofSeconds(interval);
}

@ManagedAttribute(name = "RequestTimeoutSeconds")
public long getRequestTimeoutSeconds() {
return getSessionRequestTimeout().get(ChronoUnit.SECONDS);
}

@ManagedAttribute(name = "RetryIntervalSeconds")
public long getRetryIntervalSeconds() {
return getSessionRequestRetryInterval().get(ChronoUnit.SECONDS);
}

public NewSessionQueue getSessionQueue() {
return config
.getClass(SESSIONS_QUEUE_SECTION, "implementation", NewSessionQueue.class,
Expand Down
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/grid/sessionqueue/local/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ java_library(
"//java/server/src/org/openqa/selenium/grid/server",
"//java/server/src/org/openqa/selenium/grid/sessionqueue",
"//java/server/src/org/openqa/selenium/grid/sessionqueue/config",
"//java/server/src/org/openqa/selenium/remote/server/jmx",
artifact("com.google.guava:guava"),
],
)
17 changes: 17 additions & 0 deletions java/server/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.remote.http.HttpRequest;
import org.openqa.selenium.remote.http.HttpResponse;
import org.openqa.selenium.remote.server.jmx.JMXHelper;
import org.openqa.selenium.remote.server.jmx.ManagedAttribute;
import org.openqa.selenium.remote.server.jmx.ManagedService;
import org.openqa.selenium.remote.tracing.AttributeKey;
import org.openqa.selenium.remote.tracing.EventAttribute;
import org.openqa.selenium.remote.tracing.EventAttributeValue;
Expand All @@ -52,6 +55,8 @@
import java.util.logging.Level;
import java.util.logging.Logger;

@ManagedService(objectName = "org.seleniumhq.grid:type=SessionQueue,name=LocalSessionQueue",
description = "New session queue")
public class LocalNewSessionQueue extends NewSessionQueue {

private static final Logger LOG = Logger.getLogger(LocalNewSessionQueue.class.getName());
Expand All @@ -67,6 +72,7 @@ public LocalNewSessionQueue(Tracer tracer, EventBus bus, Duration retryInterval,
super(tracer, retryInterval, requestTimeout);
this.bus = Require.nonNull("Event bus", bus);
Runtime.getRuntime().addShutdownHook(shutdownHook);
new JMXHelper().register(this);
}

public static NewSessionQueue create(Config config) {
Expand All @@ -82,6 +88,17 @@ public boolean isReady() {
return bus.isReady();
}

@ManagedAttribute(name = "NewSessionQueueSize")
public int getQueueSize() {
Lock writeLock = lock.writeLock();
Copy link
Member

Choose a reason for hiding this comment

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

This need only be a read lock

writeLock.lock();
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to lock to read the queue size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it uses ConcurrentLinkedDeque, the size() requires a traversal and value may not be accurate if it is being modified when size is calculated. Hence the lock. But for exposing values if we don't need that kind of accuracy, then we can get rid of the lock. @diemol What would you suggest?

try {
return sessionRequests.size();
} finally {
writeLock.unlock();
}
}

@Override
public boolean offerLast(HttpRequest request, RequestId requestId) {
Require.nonNull("New Session request", request);
Expand Down
1 change: 1 addition & 0 deletions java/server/src/org/openqa/selenium/remote/server/jmx/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ java_library(
srcs = glob(["*.java"]),
visibility = [
"//java/server/src/org/openqa/selenium/grid/session:__pkg__",
"//java/server/src/org/openqa/selenium/grid:__subpackages__",
],
)
Loading