From 48d0a3089a507126ac42b3506531e130ca025273 Mon Sep 17 00:00:00 2001 From: Viktor Lofgren Date: Thu, 2 Jan 2025 20:40:53 +0100 Subject: [PATCH] (service) Improve logging around grpc This change adds a marker for the gRPC-specific logging, as well as improves the clarity and meaningfulness of the log messages. --- .../client/GrpcMultiNodeChannelPool.java | 4 +-- .../client/GrpcSingleNodeChannelPool.java | 13 ++++++---- .../discovery/property/ServiceEndpoint.java | 5 ++++ .../discovery/property/ServiceKey.java | 25 +++++++++++++++++++ 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/code/common/service/java/nu/marginalia/service/client/GrpcMultiNodeChannelPool.java b/code/common/service/java/nu/marginalia/service/client/GrpcMultiNodeChannelPool.java index 9ce835af..c37b6cc0 100644 --- a/code/common/service/java/nu/marginalia/service/client/GrpcMultiNodeChannelPool.java +++ b/code/common/service/java/nu/marginalia/service/client/GrpcMultiNodeChannelPool.java @@ -7,8 +7,6 @@ import nu.marginalia.service.discovery.property.PartitionTraits; import nu.marginalia.service.discovery.property.ServiceEndpoint; import nu.marginalia.service.discovery.property.ServiceKey; import nu.marginalia.service.discovery.property.ServicePartition; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -24,7 +22,7 @@ import java.util.function.Function; public class GrpcMultiNodeChannelPool { private final ConcurrentHashMap> pools = new ConcurrentHashMap<>(); - private static final Logger logger = LoggerFactory.getLogger(GrpcMultiNodeChannelPool.class); + private final ServiceRegistryIf serviceRegistryIf; private final ServiceKey serviceKey; private final Function channelConstructor; diff --git a/code/common/service/java/nu/marginalia/service/client/GrpcSingleNodeChannelPool.java b/code/common/service/java/nu/marginalia/service/client/GrpcSingleNodeChannelPool.java index b1f72b93..5db779c9 100644 --- a/code/common/service/java/nu/marginalia/service/client/GrpcSingleNodeChannelPool.java +++ b/code/common/service/java/nu/marginalia/service/client/GrpcSingleNodeChannelPool.java @@ -10,6 +10,8 @@ import nu.marginalia.service.discovery.property.ServiceKey; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.Marker; +import org.slf4j.MarkerFactory; import java.time.Duration; import java.util.*; @@ -26,6 +28,7 @@ import java.util.function.Function; public class GrpcSingleNodeChannelPool extends ServiceChangeMonitor { private final Map channels = new ConcurrentHashMap<>(); + private final Marker grpcMarker = MarkerFactory.getMarker("GRPC"); private static final Logger logger = LoggerFactory.getLogger(GrpcSingleNodeChannelPool.class); private final ServiceRegistryIf serviceRegistryIf; @@ -59,10 +62,10 @@ public class GrpcSingleNodeChannelPool extends ServiceChangeMonitor { for (var route : Sets.symmetricDifference(oldRoutes, newRoutes)) { ConnectionHolder oldChannel; if (newRoutes.contains(route)) { - logger.info("Adding route {}", route); + logger.info(grpcMarker, "Adding route {} => {}", serviceKey, route); oldChannel = channels.put(route, new ConnectionHolder(route)); } else { - logger.info("Expelling route {}", route); + logger.info(grpcMarker, "Expelling route {} => {}", serviceKey, route); oldChannel = channels.remove(route); } if (oldChannel != null) { @@ -100,7 +103,7 @@ public class GrpcSingleNodeChannelPool extends ServiceChangeMonitor { } try { - logger.info("Creating channel for {}:{}", serviceKey, address); + logger.info(grpcMarker, "Creating channel for {} => {}", serviceKey, address); value = channelConstructor.apply(address); if (channel.compareAndSet(null, value)) { return value; @@ -111,7 +114,7 @@ public class GrpcSingleNodeChannelPool extends ServiceChangeMonitor { } } catch (Exception e) { - logger.error("Failed to get channel for " + address, e); + logger.error(grpcMarker, "Failed to get channel for " + address, e); return null; } } @@ -203,7 +206,7 @@ public class GrpcSingleNodeChannelPool extends ServiceChangeMonitor { } for (var e : exceptions) { - logger.error("Failed to call service {}", serviceKey, e); + logger.error(grpcMarker, "Failed to call service {}", serviceKey, e); } throw new ServiceNotAvailableException(serviceKey); diff --git a/code/common/service/java/nu/marginalia/service/discovery/property/ServiceEndpoint.java b/code/common/service/java/nu/marginalia/service/discovery/property/ServiceEndpoint.java index 0e25ce19..8709f2f8 100644 --- a/code/common/service/java/nu/marginalia/service/discovery/property/ServiceEndpoint.java +++ b/code/common/service/java/nu/marginalia/service/discovery/property/ServiceEndpoint.java @@ -48,5 +48,10 @@ public record ServiceEndpoint(String host, int port) { public int port() { return endpoint.port(); } + + @Override + public String toString() { + return endpoint().host() + ":" + endpoint.port() + " [" + instance + "]"; + } } } diff --git a/code/common/service/java/nu/marginalia/service/discovery/property/ServiceKey.java b/code/common/service/java/nu/marginalia/service/discovery/property/ServiceKey.java index 5a7aab4c..4916339e 100644 --- a/code/common/service/java/nu/marginalia/service/discovery/property/ServiceKey.java +++ b/code/common/service/java/nu/marginalia/service/discovery/property/ServiceKey.java @@ -48,6 +48,19 @@ public sealed interface ServiceKey

{ { throw new UnsupportedOperationException(); } + + @Override + public String toString() { + final String shortName; + + int periodIndex = name.lastIndexOf('.'); + + if (periodIndex >= 0) shortName = name.substring(periodIndex+1); + else shortName = name; + + return "rest:" + shortName; + } + } record Grpc

(String name, P partition) implements ServiceKey

{ public String baseName() { @@ -64,6 +77,18 @@ public sealed interface ServiceKey

{ { return new Grpc<>(name, partition); } + + @Override + public String toString() { + final String shortName; + + int periodIndex = name.lastIndexOf('.'); + + if (periodIndex >= 0) shortName = name.substring(periodIndex+1); + else shortName = name; + + return "grpc:" + shortName + "[" + partition.identifier() + "]"; + } } }