From d42e192c8704f2c1cd0bce05cce2163867c97941 Mon Sep 17 00:00:00 2001 From: Phillip Schichtel Date: Thu, 2 Jul 2020 23:16:04 +0200 Subject: [PATCH] Make UdpSender lazy to be able to recover from early DNS issues this should close #369 Signed-off-by: Phillip Schichtel --- .../thrift/internal/senders/UdpSender.java | 35 +++++++++++++++--- .../senders/ThriftSenderFactoryTest.java | 3 +- .../internal/senders/UdpLazinessTest.java | 37 +++++++++++++++++++ 3 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/UdpLazinessTest.java diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/UdpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/UdpSender.java index 9d0539d5d..4d0288610 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/UdpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/UdpSender.java @@ -27,8 +27,11 @@ public class UdpSender extends ThriftSender { public static final String DEFAULT_AGENT_UDP_HOST = "localhost"; public static final int DEFAULT_AGENT_UDP_COMPACT_PORT = 6831; - @ToString.Exclude private Agent.Client agentClient; - @ToString.Exclude private ThriftUdpTransport udpTransport; + private final String host; + private final int port; + + @ToString.Exclude private volatile Agent.Client agentClient; + @ToString.Exclude private volatile ThriftUdpTransport udpTransport; /** * This constructor expects Jaeger running running on {@value #DEFAULT_AGENT_UDP_HOST} @@ -54,14 +57,30 @@ public UdpSender(String host, int port, int maxPacketSize) { port = DEFAULT_AGENT_UDP_COMPACT_PORT; } - udpTransport = ThriftUdpTransport.newThriftUdpClient(host, port); - agentClient = new Agent.Client(protocolFactory.getProtocol(udpTransport)); + this.host = host; + this.port = port; + } + + private Agent.Client getAgentClient() { + Agent.Client localRef = this.agentClient; + if (localRef == null) { + synchronized (this) { + localRef = this.agentClient; + if (localRef == null) { + udpTransport = ThriftUdpTransport.newThriftUdpClient(host, port); + localRef = new Agent.Client(protocolFactory.getProtocol(udpTransport)); + this.agentClient = localRef; + } + } + } + + return localRef; } @Override public void send(Process process, List spans) throws SenderException { try { - agentClient.emitBatch(new Batch(process, spans)); + getAgentClient().emitBatch(new Batch(process, spans)); } catch (Exception e) { throw new SenderException(String.format("Could not send %d spans", spans.size()), e, spans.size()); } @@ -72,7 +91,11 @@ public int close() throws SenderException { try { return super.close(); } finally { - udpTransport.close(); + synchronized (this) { + if (udpTransport != null) { + udpTransport.close(); + } + } } } } diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactoryTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactoryTest.java index 9babab7dd..6dd1b8a3d 100644 --- a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactoryTest.java +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactoryTest.java @@ -17,7 +17,6 @@ import static org.junit.Assert.assertTrue; import io.jaegertracing.Configuration; -import io.jaegertracing.internal.senders.NoopSender; import io.jaegertracing.spi.Sender; import org.junit.Before; import org.junit.Test; @@ -47,7 +46,7 @@ public void testSenderWithAgentDataFromEnv() { System.setProperty(Configuration.JAEGER_AGENT_HOST, "jaeger-agent"); System.setProperty(Configuration.JAEGER_AGENT_PORT, "6832"); Sender sender = Configuration.SenderConfiguration.fromEnv().getSender(); - assertTrue(sender instanceof NoopSender); + assertTrue(sender instanceof UdpSender); } @Test diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/UdpLazinessTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/UdpLazinessTest.java new file mode 100644 index 000000000..eb0b6d68b --- /dev/null +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/UdpLazinessTest.java @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2020, The Jaeger Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package io.jaegertracing.thrift.internal.senders; + +import io.jaegertracing.internal.exceptions.SenderException; +import org.junit.Test; + +import java.net.SocketException; +import java.util.Collections; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class UdpLazinessTest { + @Test + public void testLazyInitializationOfAgent() { + UdpSender udpSender = new UdpSender("agent.acme.test", 55555, 0); + + try { + udpSender.send(null, Collections.emptyList()); + fail("Send should fail!"); + } catch (SenderException e) { + assertTrue("send should throw a socket exception", e.getCause().getCause() instanceof SocketException); + } + } +}