Skip to content
This repository has been archived by the owner on Sep 21, 2021. It is now read-only.

Commit

Permalink
Fix prometheus bug where running/starting containers could be wrong (#…
Browse files Browse the repository at this point in the history
…728)

* Fix prometheus bug where running/starting containers could be wrong

* Add test case
  • Loading branch information
pearj authored and diemol committed Sep 27, 2018
1 parent fdd1c9c commit 771cb5d
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 14 deletions.
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<version>2.4</version>
<configuration>
<archive>
<manifestSections>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package de.zalando.ep.zalenium.prometheus;

import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.groupingBy;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import de.zalando.ep.zalenium.container.ContainerCreationStatus;
import de.zalando.ep.zalenium.proxy.AutoStartProxySet.ContainerStatus;
import io.prometheus.client.Collector;
import io.prometheus.client.GaugeMetricFamily;

public class ContainerStatusCollectorExports extends Collector {

private enum States {
RUNNING,
STARTING;
}

private Map<ContainerCreationStatus, ContainerStatus> startedContainers;
private static List<States> STATES = Arrays.asList(States.values());

public ContainerStatusCollectorExports(Map<ContainerCreationStatus, ContainerStatus> startedContainers) {
super();
this.startedContainers = startedContainers;
}

@Override
public List<MetricFamilySamples> collect() {

GaugeMetricFamily seleniumContainersStateMetric = new GaugeMetricFamily("selenium_containers",
"The number of Selenium Containers broken down by state",
singletonList("state"));

Map<States, Long> containerStates = startedContainers.values().stream()
.collect(groupingBy(s -> {
return s.isStarted() ? States.RUNNING : States.STARTING;
}, counting()));

// Ensure that if a state is empty it is 0 instead of missing.
STATES.stream().forEach(s ->
seleniumContainersStateMetric.addMetric(singletonList(s.name().toLowerCase()),
Optional.ofNullable(containerStates.get(s)).orElse(0L)));

List<MetricFamilySamples> mfs = new ArrayList<MetricFamilySamples>();
mfs.add(seleniumContainersStateMetric);
return mfs;
}

}
19 changes: 6 additions & 13 deletions src/main/java/de/zalando/ep/zalenium/proxy/AutoStartProxySet.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import de.vandermeer.asciitable.CWC_LongestLine;
import de.vandermeer.skb.interfaces.transformers.textformat.TextAlignment;
import de.zalando.ep.zalenium.container.ContainerCreationStatus;
import io.prometheus.client.Gauge;
import net.jcip.annotations.ThreadSafe;

/**
Expand Down Expand Up @@ -89,13 +88,7 @@ public class AutoStartProxySet extends ProxySet implements Iterable<RemoteProxy>
private long timeOfLastReport = 0;

private Clock clock;

static final Gauge seleniumContainersStarting = Gauge.build()
.name("selenium_containers_starting").help("The number of Selenium Containers that are starting.").register();

static final Gauge seleniumContainersRunning = Gauge.build()
.name("selenium_containers_running").help("The number of Selenium Containers that are running.").register();


public AutoStartProxySet(boolean throwOnCapabilityNotPresent, long minContainers, long maxContainers,
long timeToWaitToStart, boolean waitForAvailableNodes, DockeredSeleniumStarter starter, Clock clock) {
super(throwOnCapabilityNotPresent);
Expand Down Expand Up @@ -198,7 +191,6 @@ public RemoteProxy remove(RemoteProxy proxy) {
try {
LOGGER.info("Stopping removed container [" + containerId + "].");
starter.stopContainer(containerId);
seleniumContainersRunning.dec();
} catch (Exception e) {
LOGGER.error("Failed to stop container [" + containerId + "].", e);
}
Expand Down Expand Up @@ -277,8 +269,6 @@ private boolean register(DockerSeleniumRemoteProxy proxy) {
return false;
} else {
LOGGER.info("Registered a container {} {}.", containerId, proxy);
seleniumContainersStarting.dec();
seleniumContainersRunning.inc();
return true;
}
}
Expand Down Expand Up @@ -391,7 +381,6 @@ private synchronized void startContainer(Map<String, Object> desiredCapabilities
startedContainers.put(startedContainer,
new ContainerStatus(startedContainer.getContainerName(), clock.millis()));
LOGGER.info("Created {}.", startedContainer);
seleniumContainersStarting.inc();
}
}

Expand Down Expand Up @@ -473,8 +462,12 @@ private static String dateTime(long epochMillis) {
LocalDateTime date = Instant.ofEpochMilli(epochMillis).atZone(ZoneId.systemDefault()).toLocalDateTime();
return formatter.format(date);
}

public Map<ContainerCreationStatus, ContainerStatus> getStartedContainers() {
return Collections.unmodifiableMap(startedContainers);
}

private static final class ContainerStatus {
public static final class ContainerStatus {
private final String containerId;
private final long timeCreated;
private Optional<Long> timeStarted = Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.zalando.ep.zalenium.registry;

import de.zalando.ep.zalenium.dashboard.Dashboard;
import de.zalando.ep.zalenium.prometheus.ContainerStatusCollectorExports;
import de.zalando.ep.zalenium.prometheus.TestSessionCollectorExports;
import net.jcip.annotations.ThreadSafe;

Expand Down Expand Up @@ -98,10 +99,12 @@ public ZaleniumRegistry(Hub hub) {
Dashboard.loadTestInformationFromFile();
Dashboard.setShutDownHook();

proxies = new AutoStartProxySet(false, minContainers, maxContainers, timeToWaitToStart, waitForAvailableNodes, starter, Clock.systemDefaultZone());
AutoStartProxySet autoStart = new AutoStartProxySet(false, minContainers, maxContainers, timeToWaitToStart, waitForAvailableNodes, starter, Clock.systemDefaultZone());
proxies = autoStart;
this.matcherThread.setUncaughtExceptionHandler(new UncaughtExceptionHandler());

new TestSessionCollectorExports(proxies).register();
new ContainerStatusCollectorExports(autoStart.getStartedContainers()).register();
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package de.zalando.ep.zalenium.prometheus;


import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.Mockito.mock;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import de.zalando.ep.zalenium.container.ContainerCreationStatus;
import de.zalando.ep.zalenium.proxy.AutoStartProxySet.ContainerStatus;
import io.prometheus.client.CollectorRegistry;

public class ContainerStatusCollectorExportsTest {

@Before
@After
public void resetPrometheus() {
CollectorRegistry.defaultRegistry.clear();
}

@Test
public void testEmptyMapBothValuesZero() {
Map<ContainerCreationStatus,ContainerStatus> map = new HashMap<ContainerCreationStatus, ContainerStatus>();
new ContainerStatusCollectorExports(map).register(CollectorRegistry.defaultRegistry);

Double startingValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"starting"});
Double runningValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"running"});
assertThat(startingValue, equalTo(0.0));
assertThat(runningValue, equalTo(0.0));
}

@Test
public void testIncrementChanges() {
// Given an empty map
Map<ContainerCreationStatus,ContainerStatus> map = new HashMap<ContainerCreationStatus, ContainerStatus>();
new ContainerStatusCollectorExports(map).register(CollectorRegistry.defaultRegistry);

// We get an empty values for both states
Double startingValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"starting"});
Double runningValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"running"});
assertThat(startingValue, equalTo(0.0));
assertThat(runningValue, equalTo(0.0));

// After we add 2 starting containers
ContainerStatus container1 = new ContainerStatus("123", 0l);
ContainerStatus container2 = new ContainerStatus("1234", 0l);

map.put(mock(ContainerCreationStatus.class), container1);
map.put(mock(ContainerCreationStatus.class), container2);

// We expect 2 starting and 0 running
startingValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"starting"});
runningValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"running"});
assertThat(startingValue, equalTo(2.0));
assertThat(runningValue, equalTo(0.0));

// Once we add a started time
container1.setTimeStarted(Optional.of(0l));
container2.setTimeStarted(Optional.of(0l));

// We expect 0 starting and 2 running
startingValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"starting"});
runningValue = CollectorRegistry.defaultRegistry.getSampleValue("selenium_containers", new String[] {"state"}, new String[] {"running"});
assertThat(startingValue, equalTo(0.0));
assertThat(runningValue, equalTo(2.0));
}
}

0 comments on commit 771cb5d

Please sign in to comment.