From a2122c5c34bed66a11607ea4f3c0f586f0b153d1 Mon Sep 17 00:00:00 2001 From: automation Date: Sun, 14 Sep 2025 02:05:11 +0100 Subject: [PATCH 1/4] build: bump version to 0.1.3 and document logging + config improvements --- CHANGELOG.md | 10 ++++++++++ phoenixd-base/pom.xml | 6 +++++- phoenixd-mock/pom.xml | 2 +- phoenixd-model/pom.xml | 2 +- phoenixd-rest/pom.xml | 8 ++++++-- phoenixd-test/pom.xml | 2 +- pom.xml | 2 +- 7 files changed, 25 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18bef3f..cf26215 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,4 +48,14 @@ ### Notes - If you previously consumed `0.1.1`, update to `0.1.2` to pick up the dependency fix. Dependency metadata for `0.1.1` may be cached by your build; a version bump ensures the fix is resolved. +## 0.1.3 — 2025-09-13 + +### Enhancements +- Add structured logging via Lombok `@Slf4j` in core components. + - Logs config resolution source (ENV, system property, file) and file load location. + - Logs HTTP requests (method, URI, timeout) and responses (status), with safe truncation and redaction. +- Improve configuration discovery in fat jars/containers: try TCCL first, then class loader; support JVM system properties. + +### Notes +- Enable debug logs with `logging.level.xyz.tcheeric=DEBUG` to see detailed request and configuration traces. diff --git a/phoenixd-base/pom.xml b/phoenixd-base/pom.xml index 9945008..aed2dfd 100644 --- a/phoenixd-base/pom.xml +++ b/phoenixd-base/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric phoenixd-java - 0.1.2 + 0.1.3 phoenixd-base @@ -18,6 +18,10 @@ + + org.slf4j + slf4j-api + org.projectlombok lombok diff --git a/phoenixd-mock/pom.xml b/phoenixd-mock/pom.xml index 6a23bfe..ff6adc3 100644 --- a/phoenixd-mock/pom.xml +++ b/phoenixd-mock/pom.xml @@ -6,7 +6,7 @@ xyz.tcheeric phoenixd-java - 0.1.2 + 0.1.3 phoenixd-mock diff --git a/phoenixd-model/pom.xml b/phoenixd-model/pom.xml index 9a29a43..a1fa00e 100644 --- a/phoenixd-model/pom.xml +++ b/phoenixd-model/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric phoenixd-java - 0.1.2 + 0.1.3 phoenixd-model diff --git a/phoenixd-rest/pom.xml b/phoenixd-rest/pom.xml index e549991..132c390 100644 --- a/phoenixd-rest/pom.xml +++ b/phoenixd-rest/pom.xml @@ -4,12 +4,12 @@ xyz.tcheeric phoenixd-java - 0.1.2 + 0.1.3 phoenixd-rest jar - 0.1.2 + 0.1.3 phoenixd-rest https://maven.apache.org @@ -18,6 +18,10 @@ + + org.slf4j + slf4j-api + ${project.groupId} phoenixd-model diff --git a/phoenixd-test/pom.xml b/phoenixd-test/pom.xml index d6db104..46d5b1e 100644 --- a/phoenixd-test/pom.xml +++ b/phoenixd-test/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric phoenixd-java - 0.1.2 + 0.1.3 phoenixd-test diff --git a/pom.xml b/pom.xml index 86f0ac9..e67abc7 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 xyz.tcheeric phoenixd-java - 0.1.2 + 0.1.3 pom From 9a4b8b423a276f882f656a571a49af4d554aff32 Mon Sep 17 00:00:00 2001 From: automation Date: Sun, 14 Sep 2025 02:06:03 +0100 Subject: [PATCH 2/4] fix(operation): add logging for HTTP request preparation and response handling - Introduced logging at various stages of HTTP request processing to aid in debugging and monitoring. - Enhanced error logging to include a preview of the response body on failure. - Added debug logs for configuration resolution from environment and system properties. --- .../common/rest/util/Configuration.java | 20 +++++++++++--- .../phoenixd/operation/AbstractOperation.java | 27 ++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/phoenixd-base/src/main/java/xyz/tcheeric/phoenixd/common/rest/util/Configuration.java b/phoenixd-base/src/main/java/xyz/tcheeric/phoenixd/common/rest/util/Configuration.java index a860638..532bbc6 100644 --- a/phoenixd-base/src/main/java/xyz/tcheeric/phoenixd/common/rest/util/Configuration.java +++ b/phoenixd-base/src/main/java/xyz/tcheeric/phoenixd/common/rest/util/Configuration.java @@ -4,6 +4,7 @@ import lombok.NonNull; import lombok.RequiredArgsConstructor; import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; import java.io.InputStream; import java.net.URL; @@ -13,6 +14,7 @@ import java.util.Set; @RequiredArgsConstructor +@Slf4j @Data public class Configuration { @@ -30,10 +32,18 @@ private String envValue(@NonNull String key) { private String resolve(@NonNull String key) { String env = envValue(key); - if (env != null) return env; + if (env != null) { + if (log.isDebugEnabled()) log.debug("Config '{}' resolved from ENV", key); + return env; + } String sys = System.getProperty(prefix + "." + key); - if (sys != null && !sys.isEmpty()) return sys; - return properties.getProperty(prefix + "." + key); + if (sys != null && !sys.isEmpty()) { + if (log.isDebugEnabled()) log.debug("Config '{}' resolved from system properties", key); + return sys; + } + String fileVal = properties.getProperty(prefix + "." + key); + if (fileVal != null && log.isDebugEnabled()) log.debug("Config '{}' resolved from app.properties", key); + return fileVal; } @SneakyThrows @@ -62,9 +72,13 @@ public Configuration(@NonNull String prefix) { try (InputStream in = resource.openStream()) { if (in != null) { this.properties.load(in); + if (log.isInfoEnabled()) log.info("Loaded configuration from '{}'", resource); } } } + if (properties.isEmpty()) { + log.warn("No app.properties found on classpath; relying on ENV and system properties only"); + } } public List keys() { diff --git a/phoenixd-rest/src/main/java/xyz/tcheeric/phoenixd/operation/AbstractOperation.java b/phoenixd-rest/src/main/java/xyz/tcheeric/phoenixd/operation/AbstractOperation.java index 8d5bdce..088136d 100644 --- a/phoenixd-rest/src/main/java/xyz/tcheeric/phoenixd/operation/AbstractOperation.java +++ b/phoenixd-rest/src/main/java/xyz/tcheeric/phoenixd/operation/AbstractOperation.java @@ -3,6 +3,7 @@ import lombok.Data; import lombok.NonNull; import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; import xyz.tcheeric.phoenixd.operation.impl.PostOperation; import xyz.tcheeric.phoenixd.common.rest.Operation; import xyz.tcheeric.phoenixd.common.rest.Request; @@ -25,6 +26,7 @@ import java.util.stream.Stream; @Data +@Slf4j public abstract class AbstractOperation implements Operation { protected HttpRequest httpRequest; @@ -46,6 +48,7 @@ private static long getLongProperty(String key, long defaultValue) { private static String ensureScheme(String baseUrl) { if (baseUrl == null || baseUrl.isBlank()) { + log.error("Missing required configuration 'phoenixd.base_url'"); throw new IllegalArgumentException("phoenixd.base_url is not set"); } String trimmed = baseUrl.trim(); @@ -53,6 +56,7 @@ private static String ensureScheme(String baseUrl) { if (lower.startsWith("http://") || lower.startsWith("https://")) { return trimmed; } + if (log.isDebugEnabled()) log.debug("No scheme in base_url '{}', defaulting to http://", baseUrl); return "http://" + trimmed; } @@ -85,6 +89,9 @@ public AbstractOperation(@NonNull String method, @NonNull String path, String re .timeout(Duration.ofMillis(timeout)) .method(method, bodyPublisher) .build(); + if (log.isDebugEnabled()) { + log.debug("Prepared HTTP request: method={}, uri={}, timeoutMs={}", method, this.httpRequest.uri(), timeout); + } } @SneakyThrows @@ -112,19 +119,30 @@ public AbstractOperation(@NonNull String method, @NonNull String path, @NonNull .timeout(Duration.ofMillis(timeout)) .method(method, bodyPublisher) .build(); + if (log.isDebugEnabled()) { + log.debug("Prepared HTTP request: method={}, uri={}, timeoutMs={} (with params)", method, this.httpRequest.uri(), timeout); + } } @SneakyThrows @Override public Operation execute() { + if (log.isDebugEnabled()) { + log.debug("Sending HTTP request: {} {}", httpRequest.method(), httpRequest.uri()); + } CompletableFuture> response = HttpClient.newBuilder() .build() .sendAsync(httpRequest, HttpResponse.BodyHandlers.ofString()); HttpResponse httpResp = response.get(); this.responseBody = httpResp.body(); var statusCode = httpResp.statusCode(); + if (log.isDebugEnabled()) { + log.debug("Received response: status={} uri={}", statusCode, httpRequest.uri()); + } if (statusCode < 200 || statusCode >= 300) { - throw new IOException("Failed to create invoice: " + statusCode + " " + responseBody); + String preview = responseBody == null ? "" : (responseBody.length() > 512 ? responseBody.substring(0, 512) + "..." : responseBody); + log.error("HTTP request failed: status={} uri={} bodyPreview={}", statusCode, httpRequest.uri(), preview); + throw new IOException("Failed HTTP request: " + statusCode + " " + preview); } return this; } @@ -154,6 +172,10 @@ public Operation addHeader(@NonNull String key, @NonNull String value) { .build(); this.setHttpRequest(newHttpRequest); + if (log.isDebugEnabled()) { + String safeVal = key.equalsIgnoreCase("Authorization") ? "" : value; + log.debug("Updated header: {}={} for {} {}", key, safeVal, httpRequest.method(), httpRequest.uri()); + } return this; } @@ -185,6 +207,9 @@ public String replacePathVariables(String path, Request.Param param) { throw new RuntimeException(e); } } + if (log.isDebugEnabled()) { + log.debug("Resolved path with variables: {}", path); + } return path; } } From 8451cfb46e3dfa12476cb377788a738da2b6ca8b Mon Sep 17 00:00:00 2001 From: automation Date: Sun, 14 Sep 2025 02:09:59 +0100 Subject: [PATCH 3/4] test: enable SLF4J simple logger at DEBUG during tests to cover logging branches; add slf4j-simple as test dependency --- phoenixd-rest/pom.xml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/phoenixd-rest/pom.xml b/phoenixd-rest/pom.xml index 132c390..c852ee6 100644 --- a/phoenixd-rest/pom.xml +++ b/phoenixd-rest/pom.xml @@ -18,6 +18,12 @@ + + org.slf4j + slf4j-simple + ${slf4j.version} + test + org.slf4j slf4j-api @@ -63,6 +69,16 @@ + + org.apache.maven.plugins + maven-surefire-plugin + + + debug + debug + + + com.google.cloud.tools jib-maven-plugin From c1ef1892ffde72c2ff66fd0cec274303218dd0d6 Mon Sep 17 00:00:00 2001 From: automation Date: Sun, 14 Sep 2025 02:12:49 +0100 Subject: [PATCH 4/4] test: cover missing/blank base_url and path normalization to raise phoenixd-rest coverage --- .../operation/BaseUrlMissingTest.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 phoenixd-rest/src/test/java/xyz/tcheeric/phoenixd/operation/BaseUrlMissingTest.java diff --git a/phoenixd-rest/src/test/java/xyz/tcheeric/phoenixd/operation/BaseUrlMissingTest.java b/phoenixd-rest/src/test/java/xyz/tcheeric/phoenixd/operation/BaseUrlMissingTest.java new file mode 100644 index 0000000..a1877fb --- /dev/null +++ b/phoenixd-rest/src/test/java/xyz/tcheeric/phoenixd/operation/BaseUrlMissingTest.java @@ -0,0 +1,36 @@ +package xyz.tcheeric.phoenixd.operation; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import xyz.tcheeric.phoenixd.operation.impl.PostOperation; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class BaseUrlMissingTest { + + @AfterEach + void cleanup() { + // Reset any system property override to avoid side effects on other tests + System.clearProperty("phoenixd.base_url"); + } + + // Ensures an explicit blank base_url triggers a clear configuration error + @Test + void blankBaseUrlThrows() { + System.setProperty("phoenixd.base_url", " "); + assertThatThrownBy(() -> new PostOperation("/items", "data")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("phoenixd.base_url is not set"); + } + + // Verifies path normalization when base_url has scheme but path lacks leading slash + @Test + void pathWithoutLeadingSlashIsNormalized() { + System.setProperty("phoenixd.base_url", "http://localhost:9740"); + PostOperation op = new PostOperation("items", "data"); + // URI should be http://localhost:9740/items (leading slash added) + org.assertj.core.api.Assertions.assertThat(op.getHttpRequest().uri().toString()) + .isEqualTo("http://localhost:9740/items"); + } +} +