Skip to content

Commit

Permalink
Update OTel snapshot to 723 for recordException (#786)
Browse files Browse the repository at this point in the history
* Update OTel snapshot to 723 for recordException

* Fix various servlet issues

Co-authored-by: Trask Stalnaker <[email protected]>
  • Loading branch information
Anuraag Agrawal and trask authored Jul 27, 2020
1 parent 4cbfef8 commit 688733a
Show file tree
Hide file tree
Showing 24 changed files with 158 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ public void setStatus(final Status status) {
delegate.setStatus(status);
}

@Override
public void recordException(Throwable throwable) {
delegate.recordException(throwable);
}

@Override
public void updateName(final String name) {
delegate.updateName(name);
Expand Down
14 changes: 8 additions & 6 deletions gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ configurations.all {
ext {
versions = [
// Check https://oss.jfrog.org/libs-snapshot/io/opentelemetry/ for latest snapshot version.
opentelemetry: '0.7.0-20200721.021000-20',
opentelemetry: '0.7.0-20200723.154929-27',
// Snapshot versions can often get split into two suffixes
opentelemetryOther: '0.7.0-20200723.154929-26',

slf4j : "1.7.30",
guava : "20.0", // Last version to support Java 7
Expand All @@ -26,13 +28,13 @@ ext {
deps = [
// OpenTelemetry
opentelemetryApi : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-api', version: versions.opentelemetry),
opentelemetryApiAutoAnnotations: dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-extension-auto-annotations', version: versions.opentelemetry),
opentelemetryTraceProps : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-extension-trace-propagators', version: versions.opentelemetry),
opentelemetryApiAutoAnnotations: dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-extension-auto-annotations', version: versions.opentelemetryOther),
opentelemetryTraceProps : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-extension-trace-propagators', version: versions.opentelemetryOther),
opentelemetrySdk : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-sdk', version: versions.opentelemetry),
opentelemetrySdkAutoConfig : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-sdk-extension-auto-config', version: versions.opentelemetry),
opentelemetrySdkAutoConfig : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-sdk-extension-auto-config', version: versions.opentelemetryOther),
opentelemetryJaeger : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-exporters-jaeger', version: versions.opentelemetry),
opentelemetryOtlp : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-exporters-otlp', version: versions.opentelemetry),
opentelemetryZipkin : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-exporters-zipkin', version: versions.opentelemetry),
opentelemetryOtlp : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-exporters-otlp', version: versions.opentelemetryOther),
opentelemetryZipkin : dependencies.create(group: 'io.opentelemetry', name: 'opentelemetry-exporters-zipkin', version: versions.opentelemetryOther),

// General
slf4j : "org.slf4j:slf4j-api:${versions.slf4j}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,6 @@ public void setPrincipal(Span span, HttpServletRequest request) {
}

public void setContentLength(Span span, int length) {
SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.set(span, String.valueOf(length));
SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.set(span, length);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> {
"${SemanticAttributes.HTTP_METHOD.key()}" method
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" endpoint.status
// exception bodies are not yet recorded
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { "${responseContentLength ?: 0}" || endpoint == EXCEPTION }
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == responseContentLength || /* async */ it == null }
"servlet.context" ""
"servlet.path" ""
if (endpoint.errored) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import io.opentelemetry.trace.attributes.SemanticAttributes

import java.util.concurrent.TimeoutException

import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import static io.opentelemetry.trace.Span.Kind.INTERNAL
Expand Down Expand Up @@ -117,7 +116,6 @@ class FinatraServerTest extends HttpServerTest<HttpServer> {
"${SemanticAttributes.HTTP_METHOD.key()}" method
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" endpoint.status
// exception bodies are not yet recorded
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { "${responseContentLength ?: 0}" || endpoint == EXCEPTION }
if (endpoint.query) {
"$MoreAttributes.HTTP_QUERY" endpoint.query
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import static io.opentelemetry.auto.instrumentation.jetty.JettyHttpServerTracer.TRACER;

import io.grpc.Context;
import io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletRequest;
import io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse;
import io.opentelemetry.auto.instrumentation.servlet.v3_0.TagSettingAsyncListener;
import io.opentelemetry.context.Scope;
import io.opentelemetry.trace.Span;
Expand All @@ -34,18 +36,24 @@ public class JettyHandlerAdvice {
public static void onEnter(
@Advice.Origin final Method method,
@Advice.This final Object source,
@Advice.Argument(2) final HttpServletRequest httpServletRequest,
@Advice.Argument(value = 2, readOnly = false) HttpServletRequest request,
@Advice.Argument(value = 3, readOnly = false) HttpServletResponse response,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope) {

Context attachedContext = TRACER.getServerContext(httpServletRequest);
Context attachedContext = TRACER.getServerContext(request);
if (attachedContext != null) {
// We are inside nested handler, don't create new span
return;
}

span = TRACER.startSpan(httpServletRequest, httpServletRequest, method);
scope = TRACER.startScope(span, httpServletRequest);
span = TRACER.startSpan(request, request, method);
scope = TRACER.startScope(span, request);

if (!(response instanceof CountingHttpServletResponse)) {
response = new CountingHttpServletResponse(response);
request = new CountingHttpServletRequest(request, response);
}
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
Expand Down Expand Up @@ -86,6 +94,7 @@ public static void stopSpan(

// Check again in case the request finished before adding the listener.
if (!request.isAsyncStarted() && responseHandled.compareAndSet(false, true)) {
JettyHttpServerTracer.contentLengthHelper(span, response);
TRACER.end(span, response.getStatus());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public String[] helperClassNames() {
"io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.Servlet3HttpServerTracer",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.TagSettingAsyncListener",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.TagSettingAsyncListener",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletRequest",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse$CountingServletOutputStream",
"io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse$CountingPrintWriter",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@

package io.opentelemetry.auto.instrumentation.jetty;

import io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse;
import io.opentelemetry.auto.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
import io.opentelemetry.trace.Span;
import javax.servlet.ServletResponse;

public class JettyHttpServerTracer extends Servlet3HttpServerTracer {
public static final JettyHttpServerTracer TRACER = new JettyHttpServerTracer();

public static void contentLengthHelper(Span span, ServletResponse response) {
if (response instanceof CountingHttpServletResponse) {
TRACER.setContentLength(span, ((CountingHttpServletResponse) response).getContentLength());
}
}

@Override
protected String getInstrumentationName() {
return "io.opentelemetry.auto.jetty-8.0";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.opentelemetry.auto.instrumentation.api.MoreAttributes
import io.opentelemetry.auto.test.asserts.TraceAssert
import io.opentelemetry.auto.test.base.HttpServerTest
import io.opentelemetry.trace.attributes.SemanticAttributes
import spock.lang.Shared

import javax.servlet.DispatcherType
import javax.servlet.ServletException
Expand All @@ -38,7 +39,8 @@ import static io.opentelemetry.trace.Span.Kind.SERVER

class JettyHandlerTest extends HttpServerTest<Server> {

static errorHandler = new ErrorHandler() {
@Shared
ErrorHandler errorHandler = new ErrorHandler() {
@Override
protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message) throws IOException {
Throwable th = (Throwable) request.getAttribute("javax.servlet.error.exception")
Expand All @@ -49,6 +51,9 @@ class JettyHandlerTest extends HttpServerTest<Server> {
}
}

@Shared
TestHandler testHandler = new TestHandler()

@Override
Server startServer(int port) {
def server = new Server(port)
Expand All @@ -59,7 +64,7 @@ class JettyHandlerTest extends HttpServerTest<Server> {
}

AbstractHandler handler() {
TestHandler.INSTANCE
testHandler
}

@Override
Expand Down Expand Up @@ -102,8 +107,6 @@ class JettyHandlerTest extends HttpServerTest<Server> {
}

static class TestHandler extends AbstractHandler {
static final TestHandler INSTANCE = new TestHandler()

@Override
void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
if (baseRequest.dispatcherType != DispatcherType.ERROR) {
Expand Down Expand Up @@ -134,7 +137,7 @@ class JettyHandlerTest extends HttpServerTest<Server> {
"${SemanticAttributes.HTTP_METHOD.key()}" method
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" endpoint.status
// exception bodies are not yet recorded
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { "${responseContentLength ?: 0}" || endpoint == EXCEPTION }
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == responseContentLength || endpoint == EXCEPTION }
"servlet.path" ''
if (endpoint.errored) {
"error.msg" { it == null || it == EXCEPTION.body }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$jspFileName"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/$jspFileName"
}
Expand Down Expand Up @@ -175,7 +175,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/getQuery.jsp?$queryString"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/getQuery.jsp"
}
Expand Down Expand Up @@ -233,7 +233,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/post.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "POST"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/post.jsp"
}
Expand Down Expand Up @@ -288,7 +288,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$jspFileName"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/$jspFileName"
"error.type" { String tagExceptionType ->
Expand Down Expand Up @@ -362,7 +362,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/includes/includeHtml.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/includes/includeHtml.jsp"
}
Expand Down Expand Up @@ -416,7 +416,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/includes/includeMulti.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/includes/includeMulti.jsp"
}
Expand Down Expand Up @@ -508,7 +508,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$jspFileName"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/$jspFileName"
errorAttributes(JasperException, String)
Expand Down Expand Up @@ -564,7 +564,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$staticFile"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/$staticFile"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$forwardFromFileName"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/$forwardFromFileName"
}
Expand Down Expand Up @@ -191,7 +191,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToHtml.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/forwards/forwardToHtml.jsp"
}
Expand Down Expand Up @@ -245,7 +245,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToIncludeMulti.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/forwards/forwardToIncludeMulti.jsp"
}
Expand Down Expand Up @@ -359,7 +359,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToJspForward.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/forwards/forwardToJspForward.jsp"
}
Expand Down Expand Up @@ -453,7 +453,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToCompileError.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/forwards/forwardToCompileError.jsp"
errorAttributes(JasperException, String)
Expand Down Expand Up @@ -520,7 +520,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
"${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToNonExistent.jsp"
"${SemanticAttributes.HTTP_METHOD.key()}" "GET"
"${SemanticAttributes.HTTP_STATUS_CODE.key()}" 404
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" String
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.context" "/$jspWebappContext"
"servlet.path" "/forwards/forwardToNonExistent.jsp"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ public void setStatus(final Status status) {
}
}

@Override
public void recordException(Throwable throwable) {
shadedSpan.recordException(throwable);
}

@Override
public void updateName(final String name) {
shadedSpan.updateName(name);
Expand Down
Loading

0 comments on commit 688733a

Please sign in to comment.