Skip to content

Address RC feedback #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Nov 5, 2025
+165 −137
Merged

Address RC feedback #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b4c3b20
Remove jackson-databind
KarstenSchnitter Nov 4, 2025
a2e6973
Remove org.apacha.commons:commons-lang3
KarstenSchnitter Nov 4, 2025
1f0afb9
Use correct Java 11 build for OTel Extension
KarstenSchnitter Nov 4, 2025
4ec2eee
Downgrade Jakarta Servlet API to 6.0.0
KarstenSchnitter Nov 4, 2025
66d2ea8
Downgrade Jackson
KarstenSchnitter Nov 4, 2025
6dd2374
Add Enforcer Plugin to Ensure Java 17 Build
KarstenSchnitter Nov 4, 2025
5119152
Simplify List creation (mostly in Test)
KarstenSchnitter Nov 4, 2025
c86eef2
Update pom.xml
KarstenSchnitter Nov 4, 2025
670ea92
Align runtime dependencies with Spring Boot 3.5.7
KarstenSchnitter Nov 4, 2025
687ac5d
Align Jackson Version using BOM
KarstenSchnitter Nov 4, 2025
382e732
Upgrade java-jwt to 4.5.0
KarstenSchnitter Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions cf-java-logging-support-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
<dependency>
<groupId>com.fasterxml.jackson.jr</groupId>
<artifactId>jackson-jr-objects</artifactId>
<version>${jackson-jr.version}</version>
</dependency>

<!-- this is only used to avoid error messages during tests !!! -->
Expand All @@ -21,12 +20,6 @@
<version>${logback.version}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.19.0</version>
</dependency>
</dependencies>

<parent>
Expand Down
6 changes: 2 additions & 4 deletions ...ogging-support-core/src/main/java/com/sap/hcp/cf/logging/common/converter/LineWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.sap.hcp.cf.logging.common.converter;

import org.apache.commons.lang3.StringUtils;

import java.io.IOException;
import java.io.StringWriter;
import java.io.Writer;
Expand All @@ -12,7 +10,7 @@ public class LineWriter extends Writer {

StringWriter sw = new StringWriter();

private List<String> lines = new LinkedList<String>();
private final List<String> lines = new LinkedList<String>();

public LineWriter() {
}
Expand All @@ -26,7 +24,7 @@ public List<String> getLines() {
*/
@Override
public void write(String str, int off, int len) {
if (StringUtils.isNotBlank(str)) {
if (str != null && !str.isBlank()) {
Copy link
Member

@vtintillier vtintillier Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason for not providing a shared method in support-core module to be used everywhere?

Copy link
Contributor Author

@KarstenSchnitter KarstenSchnitter Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too small of a functionality to do that. Using the StringUtils for it was already overkill.

lines.add(str);
}
}
Expand Down
11 changes: 7 additions & 4 deletions ...ort-log4j2/src/main/java/com/sap/hcp/cf/log4j2/filter/DynamicLevelPrefixLoggerFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.sap.hcp.cf.log4j2.filter;

import com.sap.hcp.cf.logging.common.helper.DynamicLogLevelHelper;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Marker;
import org.apache.logging.log4j.core.LogEvent;
Expand Down Expand Up @@ -32,7 +31,11 @@ public Result filter(LogEvent event) {

private Level getDynamicLevel(LogEvent event) {
String logLevel = getContextValue(event, DynamicLogLevelHelper.MDC_DYNAMIC_LOG_LEVEL_KEY);
return StringUtils.isNotBlank(logLevel) ? Level.getLevel(logLevel) : null;
return isNotBlank(logLevel) ? Level.getLevel(logLevel) : null;
}

private static boolean isNotBlank(String string) {
return string != null && !string.isBlank();
}

private String getContextValue(LogEvent event, String key) {
Expand All @@ -54,7 +57,7 @@ private Result filter(Level level, Level dynamicLevel, String loggerFqcn, String
}

private boolean checkPackages(String loggerFqcn, String logLevelPackages) {
if (StringUtils.isNotBlank(logLevelPackages)) {
if (isNotBlank(logLevelPackages)) {
for (String current: logLevelPackages.split(",")) {
if (loggerFqcn.startsWith(current)) {
return true;
Expand Down Expand Up @@ -84,7 +87,7 @@ public Result filter(final Logger logger, final Level level, final Marker marker

private Level getMdcLevel() {
String mdcLevel = MDC.get(DynamicLogLevelHelper.MDC_DYNAMIC_LOG_LEVEL_KEY);
return StringUtils.isNotBlank(mdcLevel) ? Level.getLevel(mdcLevel) : null;
return isNotBlank(mdcLevel) ? Level.getLevel(mdcLevel) : null;
}

private String getMdcPackages() {
Expand Down
7 changes: 5 additions & 2 deletions ...upport-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/suppliers/BaseFieldSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.sap.hcp.cf.log4j2.converter.api.Log4jContextFieldSupplier;
import com.sap.hcp.cf.logging.common.Defaults;
import com.sap.hcp.cf.logging.common.Fields;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.core.LogEvent;

import java.time.Instant;
Expand Down Expand Up @@ -32,13 +31,17 @@ public Map<String, Object> map(LogEvent event) {
if (event.getThrown() != null) {
Throwable throwable = event.getThrown();
fields.put(Fields.EXCEPTION_TYPE, throwable.getClass().getName());
if (StringUtils.isNotBlank(throwable.getMessage())) {
if (isNotBlank(throwable.getMessage())) {
fields.put(Fields.EXCEPTION_MESSAGE, throwable.getMessage());
}
}
return fields;
}

private static boolean isNotBlank(String string) {
return string != null && !string.isBlank();
}

private String getIsoTs(LogEvent event) {
org.apache.logging.log4j.core.time.Instant instant = event.getInstant();
return Instant.ofEpochSecond(instant.getEpochSecond(), instant.getNanoOfSecond()).toString();
Expand Down
7 changes: 5 additions & 2 deletions ...gging-support-logback/src/main/java/com/sap/hcp/cf/logback/encoder/BaseFieldSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import com.sap.hcp.cf.logging.common.Defaults;
import com.sap.hcp.cf.logging.common.Fields;
import com.sap.hcp.cf.logging.common.Markers;
import org.apache.commons.lang3.StringUtils;

import java.time.Instant;
import java.util.HashMap;
Expand Down Expand Up @@ -34,13 +33,17 @@ public Map<String, Object> map(ILoggingEvent event) {
if (event.getThrowableProxy() != null && event.getThrowableProxy() instanceof ThrowableProxy) {
Throwable throwable = ((ThrowableProxy) event.getThrowableProxy()).getThrowable();
fields.put(Fields.EXCEPTION_TYPE, throwable.getClass().getName());
if (StringUtils.isNotBlank(throwable.getMessage())) {
if (isNotBlank(throwable.getMessage())) {
fields.put(Fields.EXCEPTION_MESSAGE, throwable.getMessage());
}
}
return fields;
}

private static boolean isNotBlank(String string) {
return string != null && !string.isBlank();
}

private String now() {
Instant now = Instant.now();
long timestamp = now.getEpochSecond() * 1_000_000_000L + now.getNano();
Expand Down
7 changes: 5 additions & 2 deletions ...back/src/main/java/com/sap/hcp/cf/logback/filter/DynamicLevelPrefixLoggerTurboFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import ch.qos.logback.classic.turbo.TurboFilter;
import ch.qos.logback.core.spi.FilterReply;
import com.sap.hcp.cf.logging.common.helper.DynamicLogLevelHelper;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.MDC;
import org.slf4j.Marker;

Expand All @@ -23,7 +22,7 @@ public FilterReply decide(final Marker marker, final Logger logger, final Level

private boolean checkPackages(final Logger logger) {
final String logLevelPackages = MDC.get(DynamicLogLevelHelper.MDC_DYNAMIC_LOG_LEVEL_PREFIXES);
if (StringUtils.isNotBlank(logLevelPackages)) {
if (isNotBlank(logLevelPackages)) {
for (String current: logLevelPackages.split(",")) {
if (logger.getName().startsWith(current)) {
return true;
Expand All @@ -33,4 +32,8 @@ private boolean checkPackages(final Logger logger) {
return false;
}

private static boolean isNotBlank(String string) {
return string != null && !string.isBlank();
}

}
11 changes: 8 additions & 3 deletions cf-java-logging-support-opentelemetry-agent-extension/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
</parent>

<properties>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
<opentelemetry.sdk.version>1.55.0</opentelemetry.sdk.version>
</properties>

Expand Down Expand Up @@ -62,7 +60,6 @@
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
<version>${jackson-jr.version}</version>
</dependency>
<dependency>
<groupId>com.squareup.okhttp3</groupId>
Expand All @@ -86,6 +83,14 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>${compiler.plugin.version}</version>
<configuration>
<release>11</release>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
Expand Down
6 changes: 2 additions & 4 deletions .../com/sap/hcf/cf/logging/opentelemetry/agent/ext/binding/CloudLoggingServicesProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
import java.util.function.Supplier;
import java.util.stream.Stream;

import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toList;

public class CloudLoggingServicesProvider implements Supplier<Stream<CloudFoundryServiceInstance>> {
Expand All @@ -23,8 +21,8 @@ public CloudLoggingServicesProvider(ConfigProperties config) {
}

CloudLoggingServicesProvider(ConfigProperties config, CloudFoundryServicesAdapter adapter) {
List<String> serviceLabels = asList(getUserProvidedLabel(config), getCloudLoggingLabel(config));
List<String> serviceTags = singletonList(getCloudLoggingTag(config));
List<String> serviceLabels = List.of(getUserProvidedLabel(config), getCloudLoggingLabel(config));
List<String> serviceTags = List.of(getCloudLoggingTag(config));
this.services = adapter.stream(serviceLabels, serviceTags).collect(toList());
}

Expand Down
7 changes: 2 additions & 5 deletions ...java/com/sap/hcf/cf/logging/opentelemetry/agent/ext/binding/DynatraceServiceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
import java.util.List;
import java.util.function.Supplier;

import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;

public class DynatraceServiceProvider implements Supplier<CloudFoundryServiceInstance> {

private static final String DEFAULT_USER_PROVIDED_LABEL = "user-provided";
Expand All @@ -21,8 +18,8 @@ public DynatraceServiceProvider(ConfigProperties config) {
}

DynatraceServiceProvider(ConfigProperties config, CloudFoundryServicesAdapter adapter) {
List<String> serviceLabels = asList(getUserProvidedLabel(config), getDynatraceLabel(config));
List<String> serviceTags = singletonList(getDynatraceTag(config));
List<String> serviceLabels = List.of(getUserProvidedLabel(config), getDynatraceLabel(config));
List<String> serviceTags = List.of(getDynatraceTag(config));
this.service = adapter.stream(serviceLabels, serviceTags).findFirst().orElse(null);
}

Expand Down
112 changes: 55 additions & 57 deletions ...m/sap/hcf/cf/logging/opentelemetry/agent/ext/binding/CloudFoundryServicesAdapterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,71 +4,68 @@
import org.assertj.core.api.ObjectAssert;
import org.junit.jupiter.api.Test;

import java.util.Collections;
import java.util.List;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;

public class CloudFoundryServicesAdapterTest {
private static final String DEFAULT_VCAP_SERVICES = """
{
"managed-find-me-service": [
{
"label": "managed-find-me-service",
"tags": ["Find Me!"],
"name": "managed-find-me1"
},
{
"label": "managed-find-me-service",
"tags": ["Find Me!"],
"name": "managed-find-me2"
},
{
"label": "managed-find-me-service",
"tags": ["You can't see me!"],
"name": "managed-other"
}
],
"managed-notice-me-not-service": [
{
"label": "managed-notice-me-not-service",
"tags": ["Find Me!"],
"name": "managed-other1"
},
{
"label": "managed-notice-me-not-service",
"tags": ["You can't see me!"],
"name": "managed-other2"
}
],
"user-provided": [
{
"label": "user-provided",
"tags": ["Find Me!"],
"name": "ups-find-me1"
},
{
"label": "user-provided",
"tags": ["Find Me!"],
"name": "ups-find-me2"
},
{
"label": "user-provided",
"tags": ["You can't see me!"],
"name": "ups-other"
}
]
}""";
private static final String DEFAULT_VCAP_SERVICES = "{\n" + //
" \"managed-find-me-service\": [\n" + //
" {\n" + //
" \"label\": \"managed-find-me-service\",\n" + //
" \"tags\": [\"Find Me!\"],\n" + //
" \"name\": \"managed-find-me1\"\n" + //
" },\n" + //
" {\n" + //
" \"label\": \"managed-find-me-service\",\n" + //
" \"tags\": [\"Find Me!\"],\n" + //
" \"name\": \"managed-find-me2\"\n" + //
" },\n" + //
" {\n" + //
" \"label\": \"managed-find-me-service\",\n" + //
" \"tags\": [\"You can't see me!\"],\n" + //
" \"name\": \"managed-other\"\n" + //
" }\n" + //
" ],\n" + //
" \"managed-notice-me-not-service\": [\n" + //
" {\n" + //
" \"label\": \"managed-notice-me-not-service\",\n" + //
" \"tags\": [\"Find Me!\"],\n" + //
" \"name\": \"managed-other1\"\n" + //
" },\n" + //
" {\n" + //
" \"label\": \"managed-notice-me-not-service\",\n" + //
" \"tags\": [\"You can't see me!\"],\n" + //
" \"name\": \"managed-other2\"\n" + //
" }\n" + //
" ],\n" + //
" \"user-provided\": [\n" + //
" {\n" + //
" \"label\": \"user-provided\",\n" + //
" \"tags\": [\"Find Me!\"],\n" + //
" \"name\": \"ups-find-me1\"\n" + //
" },\n" + //
" {\n" + //
" \"label\": \"user-provided\",\n" + //
" \"tags\": [\"Find Me!\"],\n" + //
" \"name\": \"ups-find-me2\"\n" + //
" },\n" + //
" {\n" + //
" \"label\": \"user-provided\",\n" + //
" \"tags\": [\"You can't see me!\"],\n" + //
" \"name\": \"ups-other\"\n" + //
" }\n" + //
" ]\n" + //
"}";

private static final CloudFoundryServicesAdapter DEFAULT_ADAPTER =
new CloudFoundryServicesAdapter(DEFAULT_VCAP_SERVICES);

@Test
void getsAllServicesWithNullParameters() {
List<CloudFoundryServiceInstance> services = DEFAULT_ADAPTER.stream(null, null).toList();
List<CloudFoundryServiceInstance> services = DEFAULT_ADAPTER.stream(null, null).collect(toList());
assertServiceNames(services).containsExactly("managed-find-me1", "managed-find-me2", "managed-other",
"managed-other1", "managed-other2", "ups-find-me1", "ups-find-me2",
"ups-other");
Expand All @@ -83,31 +80,32 @@ private static AbstractListAssert<?, List<? extends String>, String, ObjectAsser
@Test
void filtersBySingleLabel() {
List<CloudFoundryServiceInstance> services =
DEFAULT_ADAPTER.stream(Collections.singletonList("managed-find-me-service"), emptyList()).toList();
DEFAULT_ADAPTER.stream(List.of("managed-find-me-service"), emptyList()).collect(toList());
assertServiceNames(services).containsExactlyInAnyOrder("managed-find-me1", "managed-find-me2", "managed-other");
}

@Test
void priotizesByServiceLabel() {
List<CloudFoundryServiceInstance> services =
DEFAULT_ADAPTER.stream(asList("user-provided", "managed-find-me-service"), emptyList()).toList();
DEFAULT_ADAPTER.stream(List.of("user-provided", "managed-find-me-service"), emptyList())
.collect(toList());
assertServiceNames(services).containsExactly("ups-find-me1", "ups-find-me2", "ups-other", "managed-find-me1",
"managed-find-me2", "managed-other");
}

@Test
void filtersBySingleTag() {
List<CloudFoundryServiceInstance> services =
DEFAULT_ADAPTER.stream(emptyList(), Collections.singletonList("Find Me!")).toList();
DEFAULT_ADAPTER.stream(emptyList(), List.of("Find Me!")).collect(toList());
assertServiceNames(services).containsExactlyInAnyOrder("managed-find-me1", "managed-find-me2", "managed-other1",
"ups-find-me1", "ups-find-me2");
}

@Test
void standardUseCase() {
List<CloudFoundryServiceInstance> services =
DEFAULT_ADAPTER.stream(asList("user-provided", "managed-find-me-service"),
Collections.singletonList("Find Me!")).collect(toList());
DEFAULT_ADAPTER.stream(List.of("user-provided", "managed-find-me-service"), List.of("Find Me!"))
.collect(toList());
assertServiceNames(services).containsExactly("ups-find-me1", "ups-find-me2", "managed-find-me1",
"managed-find-me2");
}
Expand Down
Loading