From 86538a6911dfe8ef560fbc78d2f8f8e7e5c9f587 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Fri, 17 Jan 2025 15:57:38 +0000 Subject: [PATCH 1/2] Fix word choice in comment because somehow in English the verb of deduction is deduce, not deduct. --- .../devresource/DevResourceLifecycleManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/observability-devservices/testlibs/devresource-common/src/main/java/io/quarkus/observability/devresource/DevResourceLifecycleManager.java b/extensions/observability-devservices/testlibs/devresource-common/src/main/java/io/quarkus/observability/devresource/DevResourceLifecycleManager.java index 57d7f2008a37e..cecc7bf97e392 100644 --- a/extensions/observability-devservices/testlibs/devresource-common/src/main/java/io/quarkus/observability/devresource/DevResourceLifecycleManager.java +++ b/extensions/observability-devservices/testlibs/devresource-common/src/main/java/io/quarkus/observability/devresource/DevResourceLifecycleManager.java @@ -75,8 +75,8 @@ default Container container(T config, ModulesConfiguration root) { } /** - * Deduct current config from params. - * If port are too dynamic / configured, it's hard to deduct, + * Deduce current config from params. + * If port are too dynamic / configured, it's hard to deduce, * since configuration is not part of the devservice state. * e.g. different ports then usual - Grafana UI is 3000, if you do not use 3000, * it's hard or impossible to know which port belongs to certain property. From 8450e15462c026133124cefdc30369b3219f4d72 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Fri, 17 Jan 2025 19:57:22 +0000 Subject: [PATCH 2/2] Consolidate logic on two config paths to solve problem of missing config on container re-use path --- .../testcontainers/LgtmContainer.java | 14 ++-- .../devresource/lgtm/LgtmResource.java | 71 +++++++++++++------ 2 files changed, 59 insertions(+), 26 deletions(-) diff --git a/extensions/observability-devservices/testcontainers/src/main/java/io/quarkus/observability/testcontainers/LgtmContainer.java b/extensions/observability-devservices/testcontainers/src/main/java/io/quarkus/observability/testcontainers/LgtmContainer.java index 1f9fdc301c3a0..b50940f933452 100644 --- a/extensions/observability-devservices/testcontainers/src/main/java/io/quarkus/observability/testcontainers/LgtmContainer.java +++ b/extensions/observability-devservices/testcontainers/src/main/java/io/quarkus/observability/testcontainers/LgtmContainer.java @@ -100,18 +100,22 @@ public String getOtlpProtocol() { } public int getOtlpPort() { - int port = getOtlpPortInternal(); + int port = getPrivateOtlpPort(); return getMappedPort(port); } - private int getOtlpPortInternal() { + private int getPrivateOtlpPort() { + return getPrivateOtlpPort(getOtlpProtocol()); + } + + public static int getPrivateOtlpPort(String otlpProtocol) { // use ignore-case here; grpc == gRPC - if (ContainerConstants.OTEL_GRPC_PROTOCOL.equalsIgnoreCase(getOtlpProtocol())) { + if (ContainerConstants.OTEL_GRPC_PROTOCOL.equalsIgnoreCase(otlpProtocol)) { return ContainerConstants.OTEL_GRPC_EXPORTER_PORT; - } else if (ContainerConstants.OTEL_HTTP_PROTOCOL.equals(getOtlpProtocol())) { + } else if (ContainerConstants.OTEL_HTTP_PROTOCOL.equals(otlpProtocol)) { return ContainerConstants.OTEL_HTTP_EXPORTER_PORT; } else { - throw new IllegalArgumentException("Unsupported OTEL protocol: " + getOtlpProtocol()); + throw new IllegalArgumentException("Unsupported OTEL protocol: " + otlpProtocol); } } diff --git a/extensions/observability-devservices/testlibs/devresource-lgtm/src/main/java/io/quarkus/observability/devresource/lgtm/LgtmResource.java b/extensions/observability-devservices/testlibs/devresource-lgtm/src/main/java/io/quarkus/observability/devresource/lgtm/LgtmResource.java index 2c835036c789d..ba383cfde699d 100644 --- a/extensions/observability-devservices/testlibs/devresource-lgtm/src/main/java/io/quarkus/observability/devresource/lgtm/LgtmResource.java +++ b/extensions/observability-devservices/testlibs/devresource-lgtm/src/main/java/io/quarkus/observability/devresource/lgtm/LgtmResource.java @@ -15,10 +15,13 @@ public class LgtmResource extends ContainerResource { private ExtensionsCatalog catalog; + private LgtmConfig config; @Override public LgtmConfig config(ModulesConfiguration configuration) { - return configuration.lgtm(); + LgtmConfig config = configuration.lgtm(); + this.config = config; + return config; } @Override @@ -32,17 +35,53 @@ public Container container(LgtmConfig config, ModulesConfiguration r return set(new LgtmContainer(config)); } - // FIXME consolidate config methods. + private int getPrivateOtlpPort() { + if (config != null) { + return LgtmContainer.getPrivateOtlpPort(config.otlpProtocol()); + } else { + return -1; + } + } + + private Map config(int privatePort, String host) { + return config(privatePort, host, container.getMappedPort(privatePort)); + } + @Override public Map config(int privatePort, String host, int publicPort) { + + Map containerConfigs = new HashMap<>(); + switch (privatePort) { case ContainerConstants.GRAFANA_PORT: - return Map.of("grafana.endpoint", String.format("http://%s:%s", host, publicPort)); - case ContainerConstants.OTEL_GRPC_EXPORTER_PORT: + containerConfigs.put("grafana.endpoint", String.format("http://%s:%s", host, publicPort)); + break; case ContainerConstants.OTEL_HTTP_EXPORTER_PORT: - return Map.of("otel-collector.url", String.format("%s:%s", host, publicPort)); + if (catalog != null && catalog.hasMicrometerOtlp()) { + + containerConfigs.put("quarkus.micrometer.export.otlp.url", + String.format("http://%s:%s/v1/metrics", host, + publicPort)); + } + // No break, fall through + case ContainerConstants.OTEL_GRPC_EXPORTER_PORT: + containerConfigs.put("otel-collector.url", String.format("%s:%s", host, publicPort)); + break; + } + + // The OTLP port is probably one of the ports we already compared against, but at compile-time we don't know which one, + // so instead of doing this check as a fallthrough on the switch, do a normal if-check + if (catalog != null && catalog.hasOpenTelemetry()) { + final int privateOtlpPort = getPrivateOtlpPort(); + if (privateOtlpPort == privatePort) { + containerConfigs.put("quarkus.otel.exporter.otlp.endpoint", + String.format("http://%s:%s", host, publicPort)); + String otlpProtocol = config.otlpProtocol(); // If we got to this stage, config must be not null + containerConfigs.put("quarkus.otel.exporter.otlp.protocol", otlpProtocol); + } + } - return Map.of(); + return containerConfigs; } @Override @@ -53,23 +92,13 @@ protected LgtmContainer defaultContainer() { @Override public Map doStart() { String host = container.getHost(); - int otlpPort = container.getOtlpPort(); - - //Set non Quarkus properties for convenience and testing. Map containerConfigs = new HashMap<>(); - containerConfigs.put("grafana.endpoint", String.format("http://%s:%s", host, container.getGrafanaPort())); - containerConfigs.put("otel-collector.url", String.format("%s:%s", host, otlpPort)); - // set relevant properties for Quarkus extensions directly - if (catalog != null && catalog.hasOpenTelemetry()) { - containerConfigs.put("quarkus.otel.exporter.otlp.endpoint", String.format("http://%s:%s", host, otlpPort)); - containerConfigs.put("quarkus.otel.exporter.otlp.protocol", container.getOtlpProtocol()); - } - if (catalog != null && catalog.hasMicrometerOtlp()) { - // always use http -- as that's what Micrometer supports - containerConfigs.put("quarkus.micrometer.export.otlp.url", - String.format("http://%s:%s/v1/metrics", host, - container.getMappedPort(ContainerConstants.OTEL_HTTP_EXPORTER_PORT))); + containerConfigs.putAll(config(ContainerConstants.GRAFANA_PORT, host)); + containerConfigs.putAll(config(ContainerConstants.OTEL_HTTP_EXPORTER_PORT, host)); + // Iff GRPC is the OTLP protocol, overwrite the otel-collector.url we just wrote with the correct grpc one, and set up the otlp endpoints + if (ContainerConstants.OTEL_GRPC_PROTOCOL.equals(container.getOtlpProtocol())) { + containerConfigs.putAll(config(ContainerConstants.OTEL_GRPC_EXPORTER_PORT, host)); } return containerConfigs; }