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

Jump to bottom

Stop collecting http status code on play span by default #2804

Merged
merged 2 commits into from
Jun 4, 2021
Merged

Stop collecting http status code on play span by default #2804

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
c93c789
Stop collecting http status code on play span by default
tylerbenson Jun 4, 2021
461a97e
Missed a spot where status was being set.
tylerbenson Jun 4, 2021
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
5 changes: 4 additions & 1 deletion ...trumentation/play-2.3/src/main/java8/datadog/trace/instrumentation/play23/PlayAdvice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static datadog.trace.instrumentation.play23.PlayHeaders.GETTER;
import static datadog.trace.instrumentation.play23.PlayHttpServerDecorator.DECORATE;
import static datadog.trace.instrumentation.play23.PlayHttpServerDecorator.PLAY_REQUEST;
import static datadog.trace.instrumentation.play23.PlayHttpServerDecorator.REPORT_HTTP_STATUS;

import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
Expand Down Expand Up @@ -55,7 +56,9 @@ public static void stopTraceOnResponse(
((Action) thisAction).executionContext());
} else {
DECORATE.onError(playControllerSpan, throwable);
playControllerSpan.setHttpStatusCode(500);
if (REPORT_HTTP_STATUS) {
playControllerSpan.setHttpStatusCode(500);
}
DECORATE.beforeFinish(playControllerSpan);
playControllerSpan.finish();
}
Expand Down
6 changes: 5 additions & 1 deletion ...play-2.3/src/main/java8/datadog/trace/instrumentation/play23/PlayHttpServerDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.instrumentation.decorator.RouteHandlerDecorator.ROUTE_HANDLER_DECORATOR;

import datadog.trace.api.Config;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
Expand All @@ -14,6 +15,7 @@
import scala.Option;

public class PlayHttpServerDecorator extends HttpServerDecorator<Request, Request, Result> {
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
devinsba marked this conversation as resolved.
Show resolved Hide resolved
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
public static final PlayHttpServerDecorator DECORATE = new PlayHttpServerDecorator();
Expand Down Expand Up @@ -74,7 +76,9 @@ public AgentSpan onRequest(

@Override
public AgentSpan onError(final AgentSpan span, Throwable throwable) {
span.setHttpStatusCode(500);
if (REPORT_HTTP_STATUS) {
span.setHttpStatusCode(500);
devinsba marked this conversation as resolved.
Show resolved Hide resolved
}
if (throwable != null
// This can be moved to instanceof check when using Java 8.
&& throwable.getClass().getName().equals("java.util.concurrent.CompletionException")
Expand Down
5 changes: 4 additions & 1 deletion ...play-2.3/src/main/java8/datadog/trace/instrumentation/play23/RequestCompleteCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
import static datadog.trace.instrumentation.play23.PlayHttpServerDecorator.DECORATE;
import static datadog.trace.instrumentation.play23.PlayHttpServerDecorator.REPORT_HTTP_STATUS;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.context.TraceScope;
Expand All @@ -25,7 +26,9 @@ public Object apply(final Try<Result> result) {
if (result.isFailure()) {
DECORATE.onError(span, result.failed().get());
} else {
DECORATE.onResponse(span, result.get());
if (REPORT_HTTP_STATUS) {
DECORATE.onResponse(span, result.get());
}
}
DECORATE.beforeFinish(span);
final TraceScope scope = activeScope();
Expand Down
4 changes: 1 addition & 3 deletions dd-java-agent/instrumentation/play-2.3/src/test/groovy/server/PlayServerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import datadog.trace.instrumentation.netty38.server.NettyHttpServerDecorator
import datadog.trace.instrumentation.play23.PlayHttpServerDecorator
import play.api.test.TestServer

import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
Expand Down Expand Up @@ -44,15 +43,14 @@ class PlayServerTest extends HttpServerTest<TestServer> {
serviceName expectedServiceName()
operationName "play.request"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint == ERROR || endpoint == EXCEPTION
errored endpoint == EXCEPTION
childOfPrevious()
tags {
"$Tags.COMPONENT" PlayHttpServerDecorator.DECORATE.component()
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
"$Tags.PEER_HOST_IPV4" { it == (endpoint == FORWARDED ? endpoint.body : "127.0.0.1") }
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
// BUG
// "$Tags.HTTP_ROUTE" String
if (endpoint == EXCEPTION) {
Expand Down
5 changes: 4 additions & 1 deletion ...trumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static datadog.trace.instrumentation.play24.PlayHeaders.GETTER;
import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.DECORATE;
import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.PLAY_REQUEST;
import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.REPORT_HTTP_STATUS;

import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
Expand Down Expand Up @@ -69,7 +70,9 @@ public static void stopTraceOnResponse(
((Action) thisAction).executionContext());
} else {
DECORATE.onError(playControllerSpan, throwable);
playControllerSpan.setHttpStatusCode(500);
if (REPORT_HTTP_STATUS) {
playControllerSpan.setHttpStatusCode(500);
}
DECORATE.beforeFinish(playControllerSpan);
playControllerSpan.finish();
}
Expand Down
6 changes: 5 additions & 1 deletion ...play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.instrumentation.decorator.RouteHandlerDecorator.ROUTE_HANDLER_DECORATOR;

import datadog.trace.api.Config;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
Expand All @@ -13,6 +14,7 @@
import scala.Option;

public class PlayHttpServerDecorator extends HttpServerDecorator<Request, Request, Result> {
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
public static final PlayHttpServerDecorator DECORATE = new PlayHttpServerDecorator();
Expand Down Expand Up @@ -73,7 +75,9 @@ public AgentSpan onRequest(

@Override
public AgentSpan onError(final AgentSpan span, Throwable throwable) {
span.setHttpStatusCode(500);
if (REPORT_HTTP_STATUS) {
span.setHttpStatusCode(500);
}
if (throwable != null
// This can be moved to instanceof check when using Java 8.
&& throwable.getClass().getName().equals("java.util.concurrent.CompletionException")
Expand Down
5 changes: 4 additions & 1 deletion ...play-2.4/src/main/java8/datadog/trace/instrumentation/play24/RequestCompleteCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.DECORATE;
import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.REPORT_HTTP_STATUS;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.context.TraceScope;
Expand All @@ -24,7 +25,9 @@ public Object apply(final Try<Result> result) {
if (result.isFailure()) {
DECORATE.onError(span, result.failed().get());
} else {
DECORATE.onResponse(span, result.get());
if (REPORT_HTTP_STATUS) {
DECORATE.onResponse(span, result.get());
}
}
DECORATE.beforeFinish(span);
final TraceScope scope = activeScope();
Expand Down
3 changes: 1 addition & 2 deletions dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,14 @@ class PlayServerTest extends HttpServerTest<Server> {
serviceName expectedServiceName()
operationName "play.request"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint == ERROR || endpoint == EXCEPTION
errored endpoint == EXCEPTION
childOfPrevious()
tags {
"$Tags.COMPONENT" PlayHttpServerDecorator.DECORATE.component()
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
"$Tags.PEER_HOST_IPV4" { it == (endpoint == FORWARDED ? endpoint.body : "127.0.0.1") }
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
// BUG
// "$Tags.HTTP_ROUTE" String
if (endpoint == EXCEPTION) {
Expand Down
6 changes: 5 additions & 1 deletion ...play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.instrumentation.decorator.RouteHandlerDecorator.ROUTE_HANDLER_DECORATOR;

import datadog.trace.api.Config;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
Expand All @@ -20,6 +21,7 @@
import scala.Option;

public class PlayHttpServerDecorator extends HttpServerDecorator<Request, Request, Result> {
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
public static final PlayHttpServerDecorator DECORATE = new PlayHttpServerDecorator();
Expand Down Expand Up @@ -116,7 +118,9 @@ public AgentSpan onRequest(

@Override
public AgentSpan onError(final AgentSpan span, Throwable throwable) {
span.setHttpStatusCode(500);
if (REPORT_HTTP_STATUS) {
span.setHttpStatusCode(500);
}
if (throwable instanceof CompletionException && throwable.getCause() != null) {
throwable = throwable.getCause();
}
Expand Down
5 changes: 4 additions & 1 deletion ...play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
import static datadog.trace.instrumentation.play26.PlayHttpServerDecorator.DECORATE;
import static datadog.trace.instrumentation.play26.PlayHttpServerDecorator.REPORT_HTTP_STATUS;

import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.context.TraceScope;
Expand All @@ -26,7 +27,9 @@ public Object apply(final Try<Result> result) {
if (result.isFailure()) {
DECORATE.onError(span, result.failed().get());
} else {
DECORATE.onResponse(span, result.get());
if (REPORT_HTTP_STATUS) {
DECORATE.onResponse(span, result.get());
}
}
DECORATE.beforeFinish(span);
final TraceScope scope = activeScope();
Expand Down
3 changes: 1 addition & 2 deletions dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,14 @@ class PlayServerTest extends HttpServerTest<Server> {
serviceName expectedServiceName()
operationName "play.request"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint == ERROR || endpoint == EXCEPTION
errored endpoint == EXCEPTION
childOfPrevious()
tags {
"$Tags.COMPONENT" PlayHttpServerDecorator.DECORATE.component()
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
"$Tags.PEER_HOST_IPV4" { it == (endpoint == FORWARDED ? endpoint.body : "127.0.0.1") }
"$Tags.HTTP_URL" String
"$Tags.HTTP_METHOD" String
"$Tags.HTTP_STATUS" Integer
// BUG
// "$Tags.HTTP_ROUTE" String
if (endpoint == EXCEPTION) {
Expand Down
2 changes: 2 additions & 0 deletions dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public final class TraceInstrumentationConfig {

public static final String OSGI_SEARCH_DEPTH = "osgi.search.depth";

public static final String PLAY_REPORT_HTTP_STATUS = "trace.play.report-http-status";

public static final String SERVLET_PRINCIPAL_ENABLED = "trace.servlet.principal.enabled";
public static final String SERVLET_ASYNC_TIMEOUT_ERROR = "trace.servlet.async-timeout.error";

Expand Down
10 changes: 10 additions & 0 deletions internal-api/src/main/java/datadog/trace/api/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
import static datadog.trace.api.config.TraceInstrumentationConfig.LOGS_INJECTION_ENABLED;
import static datadog.trace.api.config.TraceInstrumentationConfig.LOGS_MDC_TAGS_INJECTION_ENABLED;
import static datadog.trace.api.config.TraceInstrumentationConfig.OSGI_SEARCH_DEPTH;
import static datadog.trace.api.config.TraceInstrumentationConfig.PLAY_REPORT_HTTP_STATUS;
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_USE_LOADCLASS;
import static datadog.trace.api.config.TraceInstrumentationConfig.RUNTIME_CONTEXT_FIELD_INJECTION;
import static datadog.trace.api.config.TraceInstrumentationConfig.SERIALVERSIONUID_FIELD_INJECTION;
Expand Down Expand Up @@ -362,6 +363,9 @@ public class Config {

private final int osgiSearchDepth;

// TODO: remove at a future point.
private final boolean playReportHttpStatus;

private final boolean servletPrincipalEnabled;
private final boolean servletAsyncTimeoutError;

Expand Down Expand Up @@ -753,6 +757,8 @@ && isJavaVersionAtLeast(8)

osgiSearchDepth = configProvider.getInteger(OSGI_SEARCH_DEPTH, 1);

playReportHttpStatus = configProvider.getBoolean(PLAY_REPORT_HTTP_STATUS, false);

servletPrincipalEnabled = configProvider.getBoolean(SERVLET_PRINCIPAL_ENABLED, false);

servletAsyncTimeoutError = configProvider.getBoolean(SERVLET_ASYNC_TIMEOUT_ERROR, true);
Expand Down Expand Up @@ -1149,6 +1155,10 @@ public int getOsgiSearchDepth() {
return osgiSearchDepth;
}

public boolean getPlayReportHttpStatus() {
return playReportHttpStatus;
}

public boolean isServletPrincipalEnabled() {
return servletPrincipalEnabled;
}
Expand Down