diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9cb55cf8a37..f5915074fc4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,6 +146,15 @@ uses [google-java-format](https://github.com/google/google-java-format) library: synchronized (lock) { ... } } ``` +* When logging exceptions, pass the exception as the `Throwable` parameter to the logger + rather than stringifying it via `getMessage()` or concatenation. This ensures logging + frameworks can render the full stack trace. + ```java + // Do: + logger.log(Level.WARNING, "Failed to process request", exception); + // Don't: + logger.warning("Failed to process request: " + exception.getMessage()); + ``` * Don't use [gradle test fixtures](https://docs.gradle.org/current/userguide/java_testing.html#sec:java_test_fixtures) ( i.e. `java-test-fixtures` plugin) to reuse code for internal testing. The test fixtures plugin has diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporter.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporter.java index 8fd12d82909..e23fc57f652 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporter.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporter.java @@ -126,12 +126,7 @@ private void onError( Throwable e) { metricRecording.finishFailed(e); logger.log( - Level.SEVERE, - "Failed to export " - + type - + "s. The request could not be executed. Error message: " - + e.getMessage(), - e); + Level.SEVERE, "Failed to export " + type + "s. The request could not be executed.", e); if (logger.isLoggable(Level.FINEST)) { logger.log(Level.FINEST, "Failed to export " + type + "s. Details follow:", e); } diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java index 277177eda19..db2dd70a10c 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java @@ -117,12 +117,7 @@ private void onError( Throwable e) { metricRecording.finishFailed(e); logger.log( - Level.SEVERE, - "Failed to export " - + type - + "s. The request could not be executed. Full error message: " - + e.getMessage(), - e); + Level.SEVERE, "Failed to export " + type + "s. The request could not be executed.", e); result.failExceptionally(FailedExportException.httpFailedExceptionally(e)); } diff --git a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java index ace67e893e6..8f3ba836328 100644 --- a/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java +++ b/opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java @@ -108,10 +108,7 @@ public SpanContext extract(Format format, C carrier) { } } catch (RuntimeException e) { logger.log( - Level.WARNING, - "Exception caught while extracting span context; returning null. " - + "Exception: [{0}] Message: [{1}]", - new String[] {e.getClass().getName(), e.getMessage()}); + Level.WARNING, "Exception caught while extracting span context; returning null.", e); } return null; diff --git a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java index c4900f6d92a..b5a8fb153c0 100644 --- a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java +++ b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java @@ -501,8 +501,7 @@ private AutoConfiguredOpenTelemetrySdk buildImpl() { logger.fine("Closing " + closeable.getClass().getName()); closeable.close(); } catch (IOException ex) { - logger.warning( - "Error closing " + closeable.getClass().getName() + ": " + ex.getMessage()); + logger.log(Level.WARNING, "Error closing " + closeable.getClass().getName(), ex); } } if (e instanceof ConfigurationException) { diff --git a/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java b/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java index 8ed8c578693..c2bd7221408 100644 --- a/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java +++ b/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java @@ -703,7 +703,7 @@ void configurationError_ClosesResources() { verify(tracerProvider).close(); verify(meterProvider).close(); - logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider: Error!"); + logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider"); } @Test diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java index 6d351386e1d..2d38f24aec3 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java @@ -287,8 +287,7 @@ static R createAndMaybeCleanup( logger.fine("Closing " + closeable.getClass().getName()); closeable.close(); } catch (IOException ex) { - logger.warning( - "Error closing " + closeable.getClass().getName() + ": " + ex.getMessage()); + logger.log(Level.WARNING, "Error closing " + closeable.getClass().getName(), ex); } } if (e instanceof DeclarativeConfigException) { diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java index 75202072524..ca4d581ac3b 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java @@ -128,14 +128,9 @@ private void onResponse(GrpcResponse grpcResponse) { private static void onError(Throwable e) { logger.log( - Level.SEVERE, - "Failed to execute " - + TYPE - + "s. The request could not be executed. Error message: " - + e.getMessage(), - e); + Level.SEVERE, "Failed to execute " + TYPE + "s. The request could not be executed.", e); if (logger.isLoggable(Level.FINEST)) { - logger.log(Level.FINEST, "Failed to execute " + TYPE + "s. Details follow: " + e); + logger.log(Level.FINEST, "Failed to execute " + TYPE + "s. Details follow:", e); } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java index 15f0f5ea44d..8fb69109b13 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java @@ -94,7 +94,7 @@ public DoubleHistogramBuilder setExplicitBucketBoundariesAdvice(List buc Objects.requireNonNull(bucketBoundaries, "bucketBoundaries must not be null"); ExplicitBucketHistogramUtils.validateBucketBoundaries(bucketBoundaries); } catch (IllegalArgumentException | NullPointerException e) { - logger.warning("Error setting explicit bucket boundaries advice: " + e.getMessage()); + logger.log(Level.WARNING, "Error setting explicit bucket boundaries advice", e); return this; } builder.setExplicitBucketBoundaries(bucketBoundaries); diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java index 4f935d0d091..72f737a895c 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongHistogram.java @@ -99,7 +99,7 @@ public LongHistogramBuilder setExplicitBucketBoundariesAdvice(List bucketB boundaries = bucketBoundaries.stream().map(Long::doubleValue).collect(Collectors.toList()); ExplicitBucketHistogramUtils.validateBucketBoundaries(boundaries); } catch (IllegalArgumentException | NullPointerException e) { - logger.warning("Error setting explicit bucket boundaries advice: " + e.getMessage()); + logger.log(Level.WARNING, "Error setting explicit bucket boundaries advice", e); return this; } builder.setExplicitBucketBoundaries(boundaries); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java index 765d583e520..c35864e1cef 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java @@ -266,7 +266,7 @@ private static Stream histogramsWithInvalidAdvice() { .build(); return histogram::record; }, - "Error setting explicit bucket boundaries advice: Bucket boundaries must be in increasing order: 10.0 >= 9.0"), + "Error setting explicit bucket boundaries advice"), Arguments.of( (Function>) meterProvider -> { @@ -279,7 +279,7 @@ private static Stream histogramsWithInvalidAdvice() { .build(); return histogram::record; }, - "Error setting explicit bucket boundaries advice: Bucket boundaries must be in increasing order: 10.0 >= 9.0"), + "Error setting explicit bucket boundaries advice"), Arguments.of( (Function>) meterProvider -> { @@ -291,6 +291,6 @@ private static Stream histogramsWithInvalidAdvice() { .build(); return histogram::record; }, - "Error setting explicit bucket boundaries advice: bucketBoundaries must not be null")); + "Error setting explicit bucket boundaries advice")); } }