Skip to content

Commit 3751720

Browse files
Fix inconsistent exception logging patterns and add guidance (#8231)
Co-authored-by: Jack Berg <34418638+jack-berg@users.noreply.github.com>
1 parent f674cdd commit 3751720

11 files changed

Lines changed: 24 additions & 33 deletions

File tree

docs/knowledge/general-patterns.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,17 @@ automatically by `otel.java-conventions`).
149149
private static final Logger LOGGER = Logger.getLogger(MyClass.class.getName());
150150
```
151151

152+
When logging exceptions, pass the exception as the `Throwable` parameter to the logger rather
153+
than stringifying it via `getMessage()` or concatenation. This ensures logging frameworks can
154+
render the full stack trace.
155+
156+
```java
157+
// Do:
158+
logger.log(Level.WARNING, "Failed to process request", exception);
159+
// Don't:
160+
logger.warning("Failed to process request: " + exception.getMessage());
161+
```
162+
152163
## toString()
153164

154165
Adding `toString()` overrides is encouraged for debugging assistance. All `toString()`

exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporter.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,7 @@ private void onError(
126126
Throwable e) {
127127
metricRecording.finishFailed(e);
128128
logger.log(
129-
Level.SEVERE,
130-
"Failed to export "
131-
+ type
132-
+ "s. The request could not be executed. Error message: "
133-
+ e.getMessage(),
134-
e);
129+
Level.SEVERE, "Failed to export " + type + "s. The request could not be executed.", e);
135130
if (logger.isLoggable(Level.FINEST)) {
136131
logger.log(Level.FINEST, "Failed to export " + type + "s. Details follow:", e);
137132
}

exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,7 @@ private void onError(
117117
Throwable e) {
118118
metricRecording.finishFailed(e);
119119
logger.log(
120-
Level.SEVERE,
121-
"Failed to export "
122-
+ type
123-
+ "s. The request could not be executed. Full error message: "
124-
+ e.getMessage(),
125-
e);
120+
Level.SEVERE, "Failed to export " + type + "s. The request could not be executed.", e);
126121
result.failExceptionally(FailedExportException.httpFailedExceptionally(e));
127122
}
128123

opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,7 @@ public <C> SpanContext extract(Format<C> format, C carrier) {
108108
}
109109
} catch (RuntimeException e) {
110110
logger.log(
111-
Level.WARNING,
112-
"Exception caught while extracting span context; returning null. "
113-
+ "Exception: [{0}] Message: [{1}]",
114-
new String[] {e.getClass().getName(), e.getMessage()});
111+
Level.WARNING, "Exception caught while extracting span context; returning null.", e);
115112
}
116113

117114
return null;

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,8 +501,7 @@ private AutoConfiguredOpenTelemetrySdk buildImpl() {
501501
logger.fine("Closing " + closeable.getClass().getName());
502502
closeable.close();
503503
} catch (IOException ex) {
504-
logger.warning(
505-
"Error closing " + closeable.getClass().getName() + ": " + ex.getMessage());
504+
logger.log(Level.WARNING, "Error closing " + closeable.getClass().getName(), ex);
506505
}
507506
}
508507
if (e instanceof ConfigurationException) {

sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ void configurationError_ClosesResources() {
703703
verify(tracerProvider).close();
704704
verify(meterProvider).close();
705705

706-
logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider: Error!");
706+
logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider");
707707
}
708708

709709
@Test

sdk-extensions/declarative-config/src/main/java/io/opentelemetry/sdk/autoconfigure/declarativeconfig/DeclarativeConfiguration.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,7 @@ static <M, R> R createAndMaybeCleanup(
284284
logger.fine("Closing " + closeable.getClass().getName());
285285
closeable.close();
286286
} catch (IOException ex) {
287-
logger.warning(
288-
"Error closing " + closeable.getClass().getName() + ": " + ex.getMessage());
287+
logger.log(Level.WARNING, "Error closing " + closeable.getClass().getName(), ex);
289288
}
290289
}
291290
if (e instanceof DeclarativeConfigException) {

sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,9 @@ private void onResponse(GrpcResponse grpcResponse) {
128128

129129
private static void onError(Throwable e) {
130130
logger.log(
131-
Level.SEVERE,
132-
"Failed to execute "
133-
+ TYPE
134-
+ "s. The request could not be executed. Error message: "
135-
+ e.getMessage(),
136-
e);
131+
Level.SEVERE, "Failed to execute " + TYPE + "s. The request could not be executed.", e);
137132
if (logger.isLoggable(Level.FINEST)) {
138-
logger.log(Level.FINEST, "Failed to execute " + TYPE + "s. Details follow: " + e);
133+
logger.log(Level.FINEST, "Failed to execute " + TYPE + "s. Details follow:", e);
139134
}
140135
}
141136

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public DoubleHistogramBuilder setExplicitBucketBoundariesAdvice(List<Double> buc
9999
Objects.requireNonNull(bucketBoundaries, "bucketBoundaries must not be null");
100100
ExplicitBucketHistogramUtils.validateBucketBoundaries(bucketBoundaries);
101101
} catch (IllegalArgumentException | NullPointerException e) {
102-
logger.warning("Error setting explicit bucket boundaries advice: " + e.getMessage());
102+
logger.log(Level.WARNING, "Error setting explicit bucket boundaries advice", e);
103103
return this;
104104
}
105105
builder.setExplicitBucketBoundaries(bucketBoundaries);

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public LongHistogramBuilder setExplicitBucketBoundariesAdvice(List<Long> bucketB
104104
boundaries = bucketBoundaries.stream().map(Long::doubleValue).collect(Collectors.toList());
105105
ExplicitBucketHistogramUtils.validateBucketBoundaries(boundaries);
106106
} catch (IllegalArgumentException | NullPointerException e) {
107-
logger.warning("Error setting explicit bucket boundaries advice: " + e.getMessage());
107+
logger.log(Level.WARNING, "Error setting explicit bucket boundaries advice", e);
108108
return this;
109109
}
110110
builder.setExplicitBucketBoundaries(boundaries);

0 commit comments

Comments
 (0)