Skip to content

Commit b668e73

Browse files
author
Mateusz Rzeszutek
authored
Convert all logging statements from slf4j to jul (#5674)
* Convert all logging statements from slf4j to jul * code review comments * fix tests * Fix randomly failing test
1 parent 64b930f commit b668e73

File tree

38 files changed

+406
-350
lines changed

38 files changed

+406
-350
lines changed

conventions/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-instrumentation.gradle.kts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ dependencies {
1111
add("muzzleBootstrap", "io.opentelemetry.javaagent:opentelemetry-javaagent-instrumentation-api")
1212
add("muzzleTooling", "io.opentelemetry.javaagent:opentelemetry-javaagent-extension-api")
1313
add("muzzleTooling", "io.opentelemetry.javaagent:opentelemetry-javaagent-tooling")
14-
add("muzzleTooling", "org.slf4j:slf4j-simple")
1514

1615
/*
1716
Dependencies added to this configuration will be found by the muzzle gradle plugin during code

examples/extension/build.gradle

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ dependencies {
110110
add("muzzleBootstrap", "io.opentelemetry.javaagent:opentelemetry-javaagent-instrumentation-api:${versions.opentelemetryJavaagentAlpha}")
111111
add("muzzleTooling", "io.opentelemetry.javaagent:opentelemetry-javaagent-extension-api:${versions.opentelemetryJavaagentAlpha}")
112112
add("muzzleTooling", "io.opentelemetry.javaagent:opentelemetry-javaagent-tooling:${versions.opentelemetryJavaagentAlpha}")
113-
add("muzzleTooling", "org.slf4j:slf4j-simple:1.7.36")
114113
}
115114

116115
//Produces a copy of upstream javaagent with this extension jar included inside it

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientHttpAttributesGetter.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,12 @@ private static Long getLongFromJettyHttpField(HttpField httpField) {
113113
try {
114114
longFromField = httpField != null ? Long.getLong(httpField.getValue()) : null;
115115
} catch (NumberFormatException t) {
116-
logger.log(
117-
FINE,
118-
"Value {0} is not valid number format for header field: {1}",
119-
new String[] {httpField.getValue(), httpField.getName()});
116+
if (logger.isLoggable(FINE)) {
117+
logger.log(
118+
FINE,
119+
"Value {0} is not valid number format for header field: {1}",
120+
new Object[] {httpField.getValue(), httpField.getName()});
121+
}
120122
}
121123
return longFromField;
122124
}

instrumentation/micrometer/micrometer-1.5/library/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/TimeUnitHelper.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@ static TimeUnit parseConfigValue(@Nullable String value, TimeUnit defaultUnit) {
4545
case "days":
4646
return TimeUnit.DAYS;
4747
default:
48-
logger.log(
49-
WARNING,
50-
"Invalid base time unit: '{0}'; using '{1}' as the base time unit instead",
51-
new String[] {value, getUnitString(defaultUnit)});
48+
if (logger.isLoggable(WARNING)) {
49+
logger.log(
50+
WARNING,
51+
"Invalid base time unit: '{0}'; using '{1}' as the base time unit instead",
52+
new Object[] {value, getUnitString(defaultUnit)});
53+
}
5254
return defaultUnit;
5355
}
5456
}

javaagent-bootstrap/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ group = "io.opentelemetry.javaagent"
88
dependencies {
99
implementation(project(":instrumentation-api"))
1010
implementation("org.slf4j:slf4j-api")
11+
implementation("org.slf4j:slf4j-simple")
1112

1213
testImplementation(project(":testing-common"))
1314
}

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/ExceptionLogger.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,24 @@
55

66
package io.opentelemetry.javaagent.bootstrap;
77

8+
import static java.util.logging.Level.FINE;
9+
10+
import java.util.logging.Logger;
11+
812
/**
913
* Class used for exception handler logging.
1014
*
1115
* <p>See io.opentelemetry.javaagent.tooling.ExceptionHandlers
1216
*/
1317
public final class ExceptionLogger {
1418

19+
private static final Logger logger = Logger.getLogger(ExceptionLogger.class.getName());
20+
21+
/** See {@code io.opentelemetry.javaagent.tooling.ExceptionHandlers} for usages. */
22+
@SuppressWarnings("unused")
23+
public static void logSuppressedError(String message, Throwable error) {
24+
logger.log(FINE, message, error);
25+
}
26+
1527
private ExceptionLogger() {}
1628
}

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/SafeErasureMatcher.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,14 @@ static TypeDescription safeAsErasure(TypeDefinition typeDefinition) {
5555
try {
5656
return typeDefinition.asErasure();
5757
} catch (Throwable e) {
58-
logger.log(
59-
FINE,
60-
"{0} trying to get erasure for target {1}: {2}",
61-
new String[] {
62-
e.getClass().getSimpleName(), safeTypeDefinitionName(typeDefinition), e.getMessage()
63-
});
58+
if (logger.isLoggable(FINE)) {
59+
logger.log(
60+
FINE,
61+
"{0} trying to get erasure for target {1}: {2}",
62+
new Object[] {
63+
e.getClass().getSimpleName(), safeTypeDefinitionName(typeDefinition), e.getMessage()
64+
});
65+
}
6466
return null;
6567
}
6668
}

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/SafeHasSuperTypeMatcher.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,14 @@ static TypeDefinition safeGetSuperClass(TypeDefinition typeDefinition) {
102102
try {
103103
return typeDefinition.getSuperClass();
104104
} catch (Throwable e) {
105-
logger.log(
106-
FINE,
107-
"{0} trying to get super class for target {1}: {2}",
108-
new String[] {
109-
e.getClass().getSimpleName(), safeTypeDefinitionName(typeDefinition), e.getMessage()
110-
});
105+
if (logger.isLoggable(FINE)) {
106+
logger.log(
107+
FINE,
108+
"{0} trying to get super class for target {1}: {2}",
109+
new Object[] {
110+
e.getClass().getSimpleName(), safeTypeDefinitionName(typeDefinition), e.getMessage()
111+
});
112+
}
111113
return null;
112114
}
113115
}
@@ -191,12 +193,14 @@ public Iterator<TypeDefinition> iterator() {
191193
}
192194

193195
private static void logException(TypeDefinition typeDefinition, Throwable e) {
194-
logger.log(
195-
FINE,
196-
"{0} trying to get interfaces for target {1}: {2}",
197-
new String[] {
198-
e.getClass().getSimpleName(), safeTypeDefinitionName(typeDefinition), e.getMessage()
199-
});
196+
if (logger.isLoggable(FINE)) {
197+
logger.log(
198+
FINE,
199+
"{0} trying to get interfaces for target {1}: {2}",
200+
new Object[] {
201+
e.getClass().getSimpleName(), safeTypeDefinitionName(typeDefinition), e.getMessage()
202+
});
203+
}
200204
}
201205
}
202206
}

javaagent-instrumentation-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/PropagatedContext.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ void setContext(Context context) {
2929
boolean result = contextUpdater.compareAndSet(this, null, context);
3030
if (!result) {
3131
Context currentPropagatedContext = contextUpdater.get(this);
32-
if (currentPropagatedContext != context) {
32+
if (currentPropagatedContext != context && logger.isLoggable(FINE)) {
3333
logger.log(
3434
FINE,
3535
"Failed to propagate context because previous propagated context is already set; new: {0}, old: {1}",

javaagent-tooling/build.gradle.kts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ dependencies {
5050
implementation("io.opentelemetry:opentelemetry-sdk-extension-jaeger-remote-sampler")
5151

5252
api("net.bytebuddy:byte-buddy-dep")
53-
implementation("org.slf4j:slf4j-api")
5453

5554
annotationProcessor("com.google.auto.service:auto-service")
5655
compileOnly("com.google.auto.service:auto-service-annotations")
@@ -64,6 +63,21 @@ dependencies {
6463
testImplementation("com.google.guava:guava")
6564
}
6665

66+
testing {
67+
suites {
68+
val testExceptionHandler by registering(JvmTestSuite::class) {
69+
dependencies {
70+
implementation(project(":javaagent-bootstrap"))
71+
implementation(project(":javaagent-tooling"))
72+
implementation("net.bytebuddy:byte-buddy-dep")
73+
74+
// Used by byte-buddy but not brought in as a transitive dependency.
75+
compileOnly("com.google.code.findbugs:annotations")
76+
}
77+
}
78+
}
79+
}
80+
6781
// Here we only include autoconfigure but don"t include OTLP exporters to ensure they are only in
6882
// the full distribution. We need to override the default exporter setting of OTLP as a result.
6983
tasks {
@@ -78,6 +92,10 @@ tasks {
7892
isEnabled.set(false)
7993
}
8094
}
95+
96+
check {
97+
dependsOn(testing.suites)
98+
}
8199
}
82100

83101
// Mockito inline mocking uses byte-buddy but agent tooling currently uses byte-buddy-dep, which cannot be on the same

0 commit comments

Comments
 (0)