From 4a2517a7d34c986f80157528caab832707582404 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sat, 19 Oct 2019 11:57:47 -0700 Subject: [PATCH] Update servlet-2 to new agent api --- .../AbstractServlet2Instrumentation.java | 1 - .../HttpServletRequestExtractAdapter.java | 90 +++---------------- .../servlet2/Servlet2Advice.java | 52 +++++------ .../servlet2/Servlet2Decorator.java | 4 +- 4 files changed, 34 insertions(+), 113 deletions(-) diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java index e1ece26995..00307bbe68 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/AbstractServlet2Instrumentation.java @@ -26,7 +26,6 @@ public abstract class AbstractServlet2Instrumentation extends Instrumenter.Defau "datadog.trace.agent.decorator.HttpServerDecorator", packageName + ".Servlet2Decorator", packageName + ".HttpServletRequestExtractAdapter", - packageName + ".HttpServletRequestExtractAdapter$MultivaluedMapFlatIterator", packageName + ".StatusSavingHttpServletResponseWrapper", }; } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServletRequestExtractAdapter.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServletRequestExtractAdapter.java index cb91d5d37b..3c5d59dd6a 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServletRequestExtractAdapter.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/HttpServletRequestExtractAdapter.java @@ -1,14 +1,7 @@ package datadog.trace.instrumentation.servlet2; -import io.opentracing.propagation.TextMap; -import java.util.AbstractMap; -import java.util.ArrayList; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; +import datadog.trace.instrumentation.api.AgentPropagation; +import java.util.Collections; import javax.servlet.http.HttpServletRequest; /** @@ -17,80 +10,19 @@ import javax.servlet.http.HttpServletRequest; * @author Pavol Loffay */ // FIXME: This code is duplicated in several places. Extract to a common dependency. -public class HttpServletRequestExtractAdapter implements TextMap { +public class HttpServletRequestExtractAdapter + implements AgentPropagation.Getter { - private final Map> headers; + public static final HttpServletRequestExtractAdapter GETTER = + new HttpServletRequestExtractAdapter(); - public HttpServletRequestExtractAdapter(final HttpServletRequest httpServletRequest) { - headers = servletHeadersToMultiMap(httpServletRequest); + @Override + public Iterable keys(final HttpServletRequest carrier) { + return Collections.list(carrier.getHeaderNames()); } @Override - public Iterator> iterator() { - return new MultivaluedMapFlatIterator<>(headers.entrySet()); - } - - @Override - public void put(final String key, final String value) { - throw new UnsupportedOperationException("This class should be used only with Tracer.inject()!"); - } - - protected Map> servletHeadersToMultiMap( - final HttpServletRequest httpServletRequest) { - final Map> headersResult = new HashMap<>(); - - final Enumeration headerNamesIt = httpServletRequest.getHeaderNames(); - while (headerNamesIt.hasMoreElements()) { - final String headerName = headerNamesIt.nextElement().toString(); - - final Enumeration valuesIt = httpServletRequest.getHeaders(headerName); - final List valuesList = new ArrayList<>(1); - while (valuesIt.hasMoreElements()) { - valuesList.add(valuesIt.nextElement().toString()); - } - - headersResult.put(headerName, valuesList); - } - - return headersResult; - } - - public static final class MultivaluedMapFlatIterator implements Iterator> { - - private final Iterator>> mapIterator; - private Map.Entry> mapEntry; - private Iterator listIterator; - - public MultivaluedMapFlatIterator(final Set>> multiValuesEntrySet) { - mapIterator = multiValuesEntrySet.iterator(); - } - - @Override - public boolean hasNext() { - if (listIterator != null && listIterator.hasNext()) { - return true; - } - - return mapIterator.hasNext(); - } - - @Override - public Map.Entry next() { - if (mapEntry == null || (!listIterator.hasNext() && mapIterator.hasNext())) { - mapEntry = mapIterator.next(); - listIterator = mapEntry.getValue().iterator(); - } - - if (listIterator.hasNext()) { - return new AbstractMap.SimpleImmutableEntry<>(mapEntry.getKey(), listIterator.next()); - } else { - return new AbstractMap.SimpleImmutableEntry<>(mapEntry.getKey(), null); - } - } - - @Override - public void remove() { - throw new UnsupportedOperationException(); - } + public String get(final HttpServletRequest carrier, final String key) { + return carrier.getHeader(key); } } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 23cdd25894..6166f73de7 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -1,16 +1,18 @@ package datadog.trace.instrumentation.servlet2; import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.api.AgentTracer.propagate; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.servlet2.HttpServletRequestExtractAdapter.GETTER; import static datadog.trace.instrumentation.servlet2.Servlet2Decorator.DECORATE; import datadog.trace.api.DDTags; -import datadog.trace.context.TraceScope; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.propagation.Format; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import datadog.trace.instrumentation.api.AgentSpan.Context; import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; import java.security.Principal; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -22,7 +24,7 @@ import net.bytebuddy.implementation.bytecode.assign.Assigner; public class Servlet2Advice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan( + public static AgentScope onEnter( @Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest req, @Advice.Argument(value = 1, readOnly = false, typing = Assigner.Typing.DYNAMIC) @@ -38,30 +40,20 @@ public class Servlet2Advice { } final HttpServletRequest httpServletRequest = (HttpServletRequest) req; - final SpanContext extractedContext = - GlobalTracer.get() - .extract( - Format.Builtin.HTTP_HEADERS, - new HttpServletRequestExtractAdapter(httpServletRequest)); + final Context extractedContext = propagate().extract(httpServletRequest, GETTER); - final Scope scope = - GlobalTracer.get() - .buildSpan("servlet.request") - .ignoreActiveSpan() - .asChildOf(extractedContext) - .withTag("span.origin.type", servlet.getClass().getName()) - .startActive(true); + final AgentSpan span = + startSpan("servlet.request", extractedContext) + .setTag("span.origin.type", servlet.getClass().getName()); - final Span span = scope.span(); DECORATE.afterStart(span); DECORATE.onConnection(span, httpServletRequest); DECORATE.onRequest(span, httpServletRequest); - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(true); - } - req.setAttribute(DD_SPAN_ATTRIBUTE, span); + + final AgentScope scope = activateSpan(span, true); + scope.setAsyncPropagation(true); return scope; } @@ -69,10 +61,10 @@ public class Servlet2Advice { public static void stopSpan( @Advice.Argument(0) final ServletRequest request, @Advice.Argument(1) final ServletResponse response, - @Advice.Enter final Scope scope, + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { // Set user.principal regardless of who created this span. - final Span currentSpan = GlobalTracer.get().activeSpan(); + final AgentSpan currentSpan = activeSpan(); if (currentSpan != null) { if (request instanceof HttpServletRequest) { final Principal principal = ((HttpServletRequest) request).getUserPrincipal(); @@ -83,22 +75,20 @@ public class Servlet2Advice { } if (scope != null) { - final Span span = scope.span(); + final AgentSpan span = scope.span(); DECORATE.onResponse(span, response); if (throwable != null) { if (response instanceof StatusSavingHttpServletResponseWrapper && ((StatusSavingHttpServletResponseWrapper) response).status == HttpServletResponse.SC_OK) { // exception was thrown but status code wasn't set - Tags.HTTP_STATUS.set(span, 500); + span.setTag(Tags.HTTP_STATUS.getKey(), 500); } DECORATE.onError(span, throwable); } DECORATE.beforeFinish(span); - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(false); - } + scope.setAsyncPropagation(false); scope.close(); } } diff --git a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java index 50c6998339..6b3378679b 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java +++ b/dd-java-agent/instrumentation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.servlet2; import datadog.trace.agent.decorator.HttpServerDecorator; -import io.opentracing.Span; +import datadog.trace.instrumentation.api.AgentSpan; import java.net.URI; import java.net.URISyntaxException; import javax.servlet.ServletResponse; @@ -58,7 +58,7 @@ public class Servlet2Decorator } @Override - public Span onRequest(final Span span, final HttpServletRequest request) { + public AgentSpan onRequest(final AgentSpan span, final HttpServletRequest request) { assert span != null; if (request != null) { span.setTag("servlet.context", request.getContextPath());