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.

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

Use Span.recordException for logging throwable #813

Merged
merged 8 commits into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,11 @@

import io.grpc.Context;
import io.opentelemetry.auto.config.Config;
import io.opentelemetry.auto.instrumentation.api.MoreAttributes;
import io.opentelemetry.context.propagation.HttpTextFormat;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.Status;
import io.opentelemetry.trace.attributes.SemanticAttributes;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Method;
import java.net.InetAddress;
import java.net.InetSocketAddress;
Expand Down Expand Up @@ -114,12 +111,7 @@ public static void setPeer(final Span span, String peerName, String peerIp) {
}

public static void addThrowable(final Span span, final Throwable throwable) {
span.setAttribute(MoreAttributes.ERROR_MSG, throwable.getMessage());
span.setAttribute(MoreAttributes.ERROR_TYPE, throwable.getClass().getName());

StringWriter errorString = new StringWriter();
throwable.printStackTrace(new PrintWriter(errorString));
span.setAttribute(MoreAttributes.ERROR_STACK, errorString.toString());
span.recordException(throwable);
Copy link
Member

Choose a reason for hiding this comment

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

🎉

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,9 @@

import io.grpc.Context;
import io.opentelemetry.OpenTelemetry;
import io.opentelemetry.auto.instrumentation.api.MoreAttributes;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.Status;
import io.opentelemetry.trace.Tracer;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Method;
import java.util.concurrent.ExecutionException;

Expand Down Expand Up @@ -127,12 +124,7 @@ protected Throwable unwrapThrowable(Throwable throwable) {
}

public void addThrowable(final Span span, final Throwable throwable) {
span.setAttribute(MoreAttributes.ERROR_MSG, throwable.getMessage());
span.setAttribute(MoreAttributes.ERROR_TYPE, throwable.getClass().getName());

StringWriter errorString = new StringWriter();
throwable.printStackTrace(new PrintWriter(errorString));
span.setAttribute(MoreAttributes.ERROR_STACK, errorString.toString());
span.recordException(throwable);
}

public static void setPeer(final Span span, String peerName, String peerIp) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,4 @@ public class MoreAttributes {
public static final String HTTP_FRAGMENT = "http.fragment.string";

public static final String USER_NAME = "user.principal";

public static final String ERROR_MSG = "error.msg"; // string representing the error message
public static final String ERROR_TYPE = "error.type"; // string representing the type of the error
public static final String ERROR_STACK = "error.stack"; // human readable version of the stack
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ class BaseDecoratorTest extends AgentSpecification {
then:
if (error) {
1 * span.setStatus(Status.UNKNOWN)
1 * span.setAttribute(MoreAttributes.ERROR_TYPE, error.getClass().getName())
1 * span.setAttribute(MoreAttributes.ERROR_STACK, _)
1 * span.setAttribute(MoreAttributes.ERROR_MSG, null)
1 * span.recordException(error)
}
0 * _

Expand All @@ -113,9 +111,7 @@ class BaseDecoratorTest extends AgentSpecification {
then:
1 * span.setStatus(status)
if (error) {
1 * span.setAttribute(MoreAttributes.ERROR_TYPE, error.getClass().getName())
1 * span.setAttribute(MoreAttributes.ERROR_STACK, _)
1 * span.setAttribute(MoreAttributes.ERROR_MSG, null)
1 * span.recordException(error)
}
0 * _

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,8 @@ class ReactorCoreTest extends InstrumentationTestRunner {
span(0) {
operationName "trace-parent"
errored true
errorEvent(RuntimeException, EXCEPTION_MESSAGE)
parent()
attributes {
errorAttributes(RuntimeException, EXCEPTION_MESSAGE)
}
}

// It's important that we don't attach errors at the Reactor level so that we don't
Expand Down Expand Up @@ -169,10 +167,8 @@ class ReactorCoreTest extends InstrumentationTestRunner {
span(0) {
operationName "trace-parent"
errored true
errorEvent(RuntimeException, EXCEPTION_MESSAGE)
parent()
attributes {
errorAttributes(RuntimeException, EXCEPTION_MESSAGE)
}
}

// It's important that we don't attach errors at the Reactor level so that we don't
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest {
operationName HttpClientDecorator.DEFAULT_SPAN_NAME
spanKind CLIENT
errored true
attributes {
errorAttributes(NullPointerException)
}
errorEvent(NullPointerException)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ class TraceAnnotationsTest extends AgentTestRunner {
span(0) {
operationName "SayTracedHello.sayERROR"
errored true
attributes {
errorAttributes(error.class)
}
errorEvent(error.class)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ class AWS1ClientTest extends AgentTestRunner {
operationName "$service.$operation"
spanKind CLIENT
errored true
errorEvent SdkClientException, ~/Unable to execute HTTP request/
parent()
attributes {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:${UNUSABLE_PORT}/"
Expand All @@ -238,7 +239,6 @@ class AWS1ClientTest extends AgentTestRunner {
for (def addedTag : additionalAttributes) {
"$addedTag.key" "$addedTag.value"
}
errorAttributes SdkClientException, ~/Unable to execute HTTP request/
}
}
}
Expand Down Expand Up @@ -271,6 +271,7 @@ class AWS1ClientTest extends AgentTestRunner {
operationName "S3.HeadBucket"
spanKind CLIENT
errored true
errorEvent RuntimeException, "bad handler"
parent()
attributes {
"${SemanticAttributes.HTTP_URL.key()}" "https://s3.amazonaws.com/"
Expand All @@ -280,7 +281,6 @@ class AWS1ClientTest extends AgentTestRunner {
"aws.endpoint" "https://s3.amazonaws.com"
"aws.operation" "HeadBucketRequest"
"aws.agent" "java-aws-sdk"
errorAttributes RuntimeException, "bad handler"
}
}
}
Expand Down Expand Up @@ -314,6 +314,11 @@ class AWS1ClientTest extends AgentTestRunner {
operationName "S3.GetObject"
spanKind CLIENT
errored true
try {
errorEvent AmazonClientException, ~/Unable to execute HTTP request/
} catch (AssertionError e) {
errorEvent SdkClientException, "Unable to execute HTTP request: Request did not complete before the request timeout configuration."
}
parent()
attributes {
"${SemanticAttributes.HTTP_URL.key()}" "$server.address/"
Expand All @@ -325,11 +330,6 @@ class AWS1ClientTest extends AgentTestRunner {
"aws.operation" "GetObjectRequest"
"aws.agent" "java-aws-sdk"
"aws.bucket.name" "someBucket"
try {
errorAttributes AmazonClientException, ~/Unable to execute HTTP request/
} catch (AssertionError e) {
errorAttributes SdkClientException, "Unable to execute HTTP request: Request did not complete before the request timeout configuration."
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ class AWS0ClientTest extends AgentTestRunner {
operationName "$service.$operation"
spanKind CLIENT
errored true
errorEvent AmazonClientException, ~/Unable to execute HTTP request/
parent()
attributes {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:${UNUSABLE_PORT}/"
Expand All @@ -183,7 +184,6 @@ class AWS0ClientTest extends AgentTestRunner {
for (def addedTag : additionalAttributes) {
"$addedTag.key" "$addedTag.value"
}
errorAttributes AmazonClientException, ~/Unable to execute HTTP request/
}
}
}
Expand Down Expand Up @@ -216,6 +216,7 @@ class AWS0ClientTest extends AgentTestRunner {
operationName "S3.GetObject"
spanKind CLIENT
errored true
errorEvent RuntimeException, "bad handler"
parent()
attributes {
"${SemanticAttributes.HTTP_URL.key()}" "https://s3.amazonaws.com/"
Expand All @@ -226,7 +227,6 @@ class AWS0ClientTest extends AgentTestRunner {
"aws.operation" "GetObjectRequest"
"aws.agent" "java-aws-sdk"
"aws.bucket.name" "someBucket"
errorAttributes RuntimeException, "bad handler"
}
}
}
Expand Down Expand Up @@ -260,6 +260,7 @@ class AWS0ClientTest extends AgentTestRunner {
operationName "S3.GetObject"
spanKind CLIENT
errored true
errorEvent AmazonClientException, ~/Unable to execute HTTP request/
parent()
attributes {
"${SemanticAttributes.HTTP_URL.key()}" "$server.address/"
Expand All @@ -271,7 +272,6 @@ class AWS0ClientTest extends AgentTestRunner {
"aws.operation" "GetObjectRequest"
"aws.agent" "java-aws-sdk"
"aws.bucket.name" "someBucket"
errorAttributes AmazonClientException, ~/Unable to execute HTTP request/
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification {
operationName "S3.GetObject"
spanKind CLIENT
errored true
errorEvent SdkClientException, "Unable to execute HTTP request: Read timed out"
parent()
attributes {
"${SemanticAttributes.NET_PEER_NAME.key()}" "localhost"
Expand All @@ -298,7 +299,6 @@ abstract class AbstractAws2ClientTest extends InstrumentationSpecification {
"aws.operation" "GetObject"
"aws.agent" "java-aws-sdk"
"aws.bucket.name" "somebucket"
errorAttributes SdkClientException, "Unable to execute HTTP request: Read timed out"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,10 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> {
operationName "${this.testResource().simpleName}.${endpoint.name().toLowerCase()}"
spanKind INTERNAL
errored endpoint == EXCEPTION
childOf((SpanData) parent)
attributes {
if (endpoint == EXCEPTION) {
errorAttributes(Exception, EXCEPTION.body)
}
if (endpoint == EXCEPTION) {
errorEvent(Exception, EXCEPTION.body)
}
childOf((SpanData) parent)
}
}

Expand All @@ -126,11 +124,6 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> {
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == responseContentLength || /* async */ it == null }
"servlet.context" ""
"servlet.path" ""
if (endpoint.errored) {
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }
}
Comment on lines -129 to -133
Copy link
Member

Choose a reason for hiding this comment

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

no replacement for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think so - the exception is recorded in the handler span so isn't on this one. Not sure if this is an issue with the instrumentation or expected though.

Copy link
Member

Choose a reason for hiding this comment

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

ah, makes sense, i think this is expected

if (endpoint.query) {
"$MoreAttributes.HTTP_QUERY" endpoint.query
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner {
operationName "GetAction"
spanKind CLIENT
errored true
errorEvent IndexNotFoundException, "no such index"
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "GetAction"
"elasticsearch.request" "GetRequest"
"elasticsearch.request.indices" indexName
errorAttributes IndexNotFoundException, "no such index"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,12 @@ class Elasticsearch2TransportClientTest extends AgentTestRunner {
operationName "GetAction"
spanKind CLIENT
errored true
errorEvent RemoteTransportException, String
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "GetAction"
"elasticsearch.request" "GetRequest"
"elasticsearch.request.indices" indexName
errorAttributes RemoteTransportException, String
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ class Elasticsearch2SpringTemplateTest extends AgentTestRunner {
operationName "RefreshAction"
spanKind CLIENT
errored true
errorEvent IndexNotFoundException, "no such index"
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "RefreshAction"
"elasticsearch.request" "RefreshRequest"
"elasticsearch.request.indices" indexName
errorAttributes IndexNotFoundException, "no such index"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ class Elasticsearch2NodeClientTest extends AgentTestRunner {
operationName "GetAction"
spanKind CLIENT
errored true
errorEvent IndexNotFoundException, "no such index"
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "GetAction"
"elasticsearch.request" "GetRequest"
"elasticsearch.request.indices" indexName
errorAttributes IndexNotFoundException, "no such index"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ class Elasticsearch2TransportClientTest extends AgentTestRunner {
operationName "GetAction"
spanKind CLIENT
errored true
errorEvent RemoteTransportException, String
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "GetAction"
"elasticsearch.request" "GetRequest"
"elasticsearch.request.indices" indexName
errorAttributes RemoteTransportException, String
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ class Elasticsearch2SpringTemplateTest extends AgentTestRunner {
operationName "RefreshAction"
spanKind CLIENT
errored true
errorEvent IndexNotFoundException, "no such index"
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "RefreshAction"
"elasticsearch.request" "RefreshRequest"
"elasticsearch.request.indices" indexName
errorAttributes IndexNotFoundException, "no such index"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ class Elasticsearch5NodeClientTest extends AgentTestRunner {
span(0) {
operationName "GetAction"
errored true
errorEvent IndexNotFoundException, "no such index"
spanKind CLIENT
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "GetAction"
"elasticsearch.request" "GetRequest"
"elasticsearch.request.indices" indexName
errorAttributes IndexNotFoundException, "no such index"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,12 @@ class Elasticsearch5TransportClientTest extends AgentTestRunner {
operationName "GetAction"
spanKind CLIENT
errored true
errorEvent RemoteTransportException, String
attributes {
"${SemanticAttributes.DB_TYPE.key()}" "elasticsearch"
"elasticsearch.action" "GetAction"
"elasticsearch.request" "GetRequest"
"elasticsearch.request.indices" indexName
errorAttributes RemoteTransportException, String
}
}
}
Expand Down
Loading