Skip to content

Commit

Permalink
Move closing DatagramChannel to the close method (#1080) (#1101)
Browse files Browse the repository at this point in the history
We can align `GraphiteUDP` with the TCP based `Graphite` and close the
socket in the `close` method instead of the connect method. The close
method will be invoked every time after a portion of metrics has been
sent, so we can close the socket there. We also do not need to call
the method `isConnected` during sending metrics because the socket is
opened right before sending data, so it can't be closed by timeout.

(cherry picked from commit d18b6ab)
  • Loading branch information
arteam authored Mar 10, 2017
1 parent 068eefb commit c6d8ecc
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,10 @@ public GraphiteUDP(InetSocketAddress address) {

@Override
public void connect() throws IllegalStateException, IOException {
// Only open the channel the first time...
if (isConnected()) {
throw new IllegalStateException("Already connected");
}

if (datagramChannel != null) {
datagramChannel.close();
}

// Resolve hostname
if (hostname != null) {
address = new InetSocketAddress(hostname, port);
Expand All @@ -72,11 +67,6 @@ public boolean isConnected() {

@Override
public void send(String name, String value, long timestamp) throws IOException {
// Underlying socket can be closed by ICMP
if (!isConnected()) {
connect();
}

try {
StringBuilder buf = new StringBuilder();
buf.append(sanitize(name));
Expand Down Expand Up @@ -107,11 +97,32 @@ public void flush() throws IOException {

@Override
public void close() throws IOException {
// Leave channel & socket open for next metrics
if (datagramChannel != null) {
try {
datagramChannel.close();
} finally {
datagramChannel = null;
}
}
}

protected String sanitize(String s) {
return WHITESPACE.matcher(s).replaceAll("-");
}

DatagramChannel getDatagramChannel() {
return datagramChannel;
}

void setDatagramChannel(DatagramChannel datagramChannel) {
this.datagramChannel = datagramChannel;
}

InetSocketAddress getAddress() {
return address;
}

void setAddress(InetSocketAddress address) {
this.address = address;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package com.codahale.metrics.graphite;

import org.junit.Test;
import org.mockito.Mockito;

import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.DatagramChannel;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.verify;

public class GraphiteUDPTest {

private final String host = "example.com";
private final int port = 1234;

private GraphiteUDP graphiteUDP;

@Test
public void connects() throws Exception {
graphiteUDP = new GraphiteUDP(host, port);
graphiteUDP.connect();

assertThat(graphiteUDP.getDatagramChannel()).isNotNull();
assertThat(graphiteUDP.getAddress()).isEqualTo(new InetSocketAddress(host, port));

graphiteUDP.close();
}

@Test
public void writesValue() throws Exception {
graphiteUDP = new GraphiteUDP(host, port);
DatagramChannel mockDatagramChannel = Mockito.mock(DatagramChannel.class);
graphiteUDP.setDatagramChannel(mockDatagramChannel);
graphiteUDP.setAddress(new InetSocketAddress(host, port));

graphiteUDP.send("name woo", "value", 100);
verify(mockDatagramChannel).send(ByteBuffer.wrap("name-woo value 100\n".getBytes("UTF-8")),
new InetSocketAddress(host, port));
}

}

0 comments on commit c6d8ecc

Please sign in to comment.