Stop setting error tag and update tests.

This commit is contained in:
Tyler Benson 2020-02-18 10:11:56 -08:00
parent 73fb7aa2b6
commit 97efa307d3
26 changed files with 26 additions and 58 deletions

View File

@ -113,7 +113,6 @@ class LagomTest extends AgentTestRunner {
"$Tags.HTTP_URL" "ws://localhost:${server.port()}/error"
"$Tags.HTTP_METHOD" "GET"
"$Tags.HTTP_STATUS" 500
"$Tags.ERROR" true
defaultTags()
}
}

View File

@ -83,7 +83,6 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest {
tags {
"$Tags.COMPONENT" "akka-http-client"
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.ERROR" true
errorTags(NullPointerException)
defaultTags()
}

View File

@ -57,7 +57,6 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<Object>
"$Tags.HTTP_METHOD" method
"$Tags.HTTP_STATUS" endpoint.status
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -118,7 +118,6 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> {
"$Tags.HTTP_STATUS" endpoint.status
"span.origin.type" ServletHandler.CachedChain.name
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -51,6 +51,12 @@ class FinatraServerTest extends HttpServerTest<HttpServer> {
return true
}
@Override
boolean testNotFound() {
// Resource name is set to "GET /notFound"
false
}
@Override
void stopServer(HttpServer httpServer) {
Await.ready(httpServer.close(), TIMEOUT)
@ -81,9 +87,6 @@ class FinatraServerTest extends HttpServerTest<HttpServer> {
// Finatra doesn't propagate the stack trace or exception to the instrumentation
// so the normal errorTags() method can't be used
if (errorEndpoint) {
"$Tags.ERROR" true
}
defaultTags()
}
}

View File

@ -22,14 +22,6 @@ class FinatraController extends Controller {
})
}
any(NOT_FOUND.getPath) { request: Request =>
controller(NOT_FOUND, new Closure[Response](null) {
override def call(): Response = {
response.notFound(NOT_FOUND.getBody)
}
})
}
any(QUERY_PARAM.getPath) { request: Request =>
controller(QUERY_PARAM, new Closure[Response](null) {
override def call(): Response = {

View File

@ -112,7 +112,6 @@ class GlassFishServerTest extends HttpServerTest<GlassFish> {
"servlet.path" endpoint.path
"span.origin.type" { it.startsWith("TestServlets\$") || it == DefaultServlet.name }
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -71,7 +71,6 @@ abstract class AbstractGoogleHttpClientTest extends HttpClientTest {
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
"$Tags.ERROR" true
"$DDTags.ERROR_MSG" "Server Error"
defaultTags()
}

View File

@ -162,8 +162,6 @@ class GrpcTest extends AgentTestRunner {
"status.description" description
if (status.cause != null) {
errorTags status.cause.class, status.cause.message
} else {
tag "error", true
}
defaultTags(true)
}
@ -196,7 +194,6 @@ class GrpcTest extends AgentTestRunner {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"status.code" "${status.code.name()}"
"status.description" description
tag "error", true
defaultTags()
}
}
@ -281,7 +278,6 @@ class GrpcTest extends AgentTestRunner {
"$Tags.COMPONENT" "grpc-client"
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"status.code" "UNKNOWN"
tag "error", true
defaultTags()
}
}

View File

@ -19,7 +19,6 @@ import com.sun.jersey.api.client.ClientHandler;
import com.sun.jersey.api.client.ClientRequest;
import com.sun.jersey.api.client.ClientResponse;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.DDTags;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import java.util.Map;
@ -72,9 +71,7 @@ public final class JaxRsClientV1Instrumentation extends Instrumenter.Default {
// WARNING: this might be a chain...so we only have to trace the first in the chain.
final boolean isRootClientHandler = null == request.getProperties().get(DD_SPAN_ATTRIBUTE);
if (isRootClientHandler) {
final AgentSpan span =
startSpan("jax-rs.client.call")
.setTag(DDTags.RESOURCE_NAME, request.getMethod() + " jax-rs.client.call");
final AgentSpan span = startSpan("jax-rs.client.call");
DECORATE.afterStart(span);
DECORATE.onRequest(span, request);
request.getProperties().put(DD_SPAN_ATTRIBUTE, span);

View File

@ -6,7 +6,6 @@ import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
import static datadog.trace.instrumentation.jaxrs.InjectAdapter.SETTER;
import static datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator.DECORATE;
import datadog.trace.api.DDTags;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import javax.annotation.Priority;
@ -24,9 +23,7 @@ public class ClientTracingFilter implements ClientRequestFilter, ClientResponseF
@Override
public void filter(final ClientRequestContext requestContext) {
final AgentSpan span =
startSpan("jax-rs.client.call")
.setTag(DDTags.RESOURCE_NAME, requestContext.getMethod() + " jax-rs.client.call");
final AgentSpan span = startSpan("jax-rs.client.call");
try (final AgentScope scope = activateSpan(span, false)) {
DECORATE.afterStart(span);
DECORATE.onRequest(span, requestContext);

View File

@ -66,6 +66,12 @@ class JettyHandlerTest extends HttpServerTest<Server> {
return "jetty.request"
}
@Override
boolean testNotFound() {
// resource name is set to "GET JettyHandlerTest$TestHandler"
false
}
@Override
boolean testExceptionBody() {
false
@ -139,7 +145,6 @@ class JettyHandlerTest extends HttpServerTest<Server> {
"$Tags.HTTP_STATUS" endpoint.status
"span.origin.type" handlerName
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -320,7 +320,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"servlet.path" "/$jspFileName"
"error" true
"error.type" { String tagExceptionType ->
return tagExceptionType == exceptionClass.getName() || tagExceptionType.contains(exceptionClass.getSimpleName())
}
@ -342,7 +341,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.origin.type" jspClassName
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
"error" true
"error.type" { String tagExceptionType ->
return tagExceptionType == exceptionClass.getName() || tagExceptionType.contains(exceptionClass.getSimpleName())
}

View File

@ -100,9 +100,7 @@ class PlayServerTest extends HttpServerTest<Server> {
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
if (endpoint == ERROR) {
"$Tags.ERROR" true
} else if (endpoint == EXCEPTION) {
if (endpoint == EXCEPTION) {
errorTags(Exception, EXCEPTION.body)
}
if (endpoint.query) {

View File

@ -102,9 +102,7 @@ class PlayServerTest extends HttpServerTest<Server> {
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
if (endpoint == ERROR) {
"$Tags.ERROR" true
} else if (endpoint == EXCEPTION) {
if (endpoint == EXCEPTION) {
errorTags(Exception, EXCEPTION.body)
}
if (endpoint.query) {
@ -135,7 +133,6 @@ class PlayServerTest extends HttpServerTest<Server> {
"$Tags.HTTP_URL" "${endpoint.resolve(address)}"
"$Tags.HTTP_METHOD" method
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -106,6 +106,12 @@ class RatpackHttpServerTest extends HttpServerTest<EmbeddedApp> {
true
}
@Override
boolean testNotFound() {
// resource name is set by instrumentation, so not changed to 404
false
}
@Override
void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {
@ -122,9 +128,7 @@ class RatpackHttpServerTest extends HttpServerTest<EmbeddedApp> {
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
if (endpoint == ERROR) {
"$Tags.ERROR" true
} else if (endpoint == EXCEPTION) {
if (endpoint == EXCEPTION) {
errorTags(Exception, EXCEPTION.body)
}
if (endpoint.query) {

View File

@ -107,7 +107,6 @@ class JettyServlet2Test extends HttpServerTest<Server> {
"servlet.path" endpoint.path
"span.origin.type" TestServlet2.Sync.name
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -96,7 +96,6 @@ abstract class AbstractServlet3Test<SERVER, CONTEXT> extends HttpServerTest<SERV
"servlet.path" { it == endpoint.path || it == "/dispatch$endpoint.path" }
"span.origin.type" { it == servlet.name || it == ApplicationFilterChain.name }
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -266,7 +266,6 @@ abstract class JettyDispatchTest extends JettyServlet3Test {
it == TestServlet3.DispatchImmediate.name || it == TestServlet3.DispatchAsync.name || it == ApplicationFilterChain.name
}
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -421,7 +421,6 @@ abstract class TomcatDispatchTest extends TomcatServlet3Test {
it == TestServlet3.DispatchImmediate.name || it == TestServlet3.DispatchAsync.name || it == ApplicationFilterChain.name
}
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -210,7 +210,7 @@ class SpringWebfluxTest extends AgentTestRunner {
assertTraces(1) {
trace(0, 2) {
span(0) {
resourceName "404"
resourceName "GET /**"
operationName "netty.request"
spanType DDSpanTypes.HTTP_SERVER
parent()
@ -328,7 +328,6 @@ class SpringWebfluxTest extends AgentTestRunner {
"$Tags.HTTP_URL" url
"$Tags.HTTP_METHOD" "GET"
"$Tags.HTTP_STATUS" 500
"error" true
defaultTags()
}
}

View File

@ -136,7 +136,6 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
"span.origin.type" ApplicationFilterChain.name
"servlet.path" endpoint.path
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }

View File

@ -435,7 +435,6 @@ class TwilioClientTest extends AgentTestRunner {
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
"$Tags.ERROR" true
defaultTags()
}
}
@ -589,7 +588,6 @@ class TwilioClientTest extends AgentTestRunner {
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
"$Tags.ERROR" true
defaultTags()
}
}

View File

@ -63,7 +63,6 @@ class TagsAssert {
}
def errorTags(Class<Throwable> errorType, message) {
tag("error", true)
tag("error.type", errorType.name)
tag("error.stack", String)

View File

@ -538,9 +538,6 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
"$Tags.HTTP_URL" "${endpoint.resolve(address)}"
"$Tags.HTTP_METHOD" method
"$Tags.HTTP_STATUS" endpoint.status
if (endpoint.errored) {
"$Tags.ERROR" endpoint.errored
}
if (endpoint.query) {
"$DDTags.HTTP_QUERY" endpoint.query
}

View File

@ -6,7 +6,7 @@ import io.opentracing.tag.Tags;
public class ErrorFlag extends AbstractDecorator {
public ErrorFlag() {
super();
this.setMatchingTag(Tags.ERROR.getKey());
setMatchingTag(Tags.ERROR.getKey());
}
@Override
@ -17,7 +17,6 @@ public class ErrorFlag extends AbstractDecorator {
} catch (final Throwable t) {
// DO NOTHING
}
// TODO: Do we really want an error tag if the error flag is already set?
return true;
return false;
}
}