Skip to content
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

Changing the log levels in error scenarios #6832

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Changing the log levels in error scenarios #6832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cb24eb
Changing the log levels in error scenarios
vidyadharamurthy Oct 29, 2024
793d6ad
Merge branch 'open-telemetry:main' into loglevel
vidyadharamurthy Oct 29, 2024
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: 4 additions & 3 deletions opentracing-shim/src/main/java/io/opentelemetry/opentracingshim/TracerShim.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,13 @@ public SpanBuilder buildSpan(String operationName) {
@Override
public <C> void inject(SpanContext context, Format<C> format, C carrier) {
if (context == null) {
logger.log(Level.INFO, "Cannot inject a null span context.");
logger.log(Level.WARNING, "Cannot inject a null span context.");
return;
}

SpanContextShim contextShim = ShimUtil.getContextShim(context);
if (contextShim == null) {
logger.log(Level.WARNING, "Cannot inject a null span context shim.");
return;
}

Expand All @@ -107,7 +108,7 @@ public <C> SpanContext extract(Format<C> format, C carrier) {
}
} catch (RuntimeException e) {
logger.log(
Level.INFO,
Level.WARNING,
"Exception caught while extracting span context; returning null. "
+ "Exception: [{0}] Message: [{1}]",
new String[] {e.getClass().getName(), e.getMessage()});
Expand All @@ -127,7 +128,7 @@ public void close() {
try {
((Closeable) provider).close();
} catch (RuntimeException | IOException e) {
logger.log(Level.INFO, "Exception caught while closing TracerProvider.", e);
logger.log(Level.WARNING, "Exception caught while closing TracerProvider.", e);
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/TracerShimTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ void extract_onlyBaggage() {
assertThat(spanContextShim.getBaggage()).isEqualTo(baggage);
}

@Test
void extract_null_spanctx() {
SpanContext spanContext = tracerShim.extract(Format.Builtin.TEXT_MAP, null);
assertThat(spanContext).isNull();
Copy link
Member

Choose a reason for hiding this comment

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

Given that this PR only changes the log level, is this test trying to add missing coverage for existing behavior? Or verify logs are emitted at the intended level?

If the latter, I suggest we skip this. We do have test tooling to verify log messages (for example), but asserting the log level seems rather specific.

}

@Test
void close_OpenTelemetrySdk() {
SdkTracerProvider sdkProvider = mock(SdkTracerProvider.class);
Expand Down
Loading