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

Update to use the OT Java 0.31.0-RC1 API #27

Merged
merged 6 commits into from
Oct 9, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion opentracing-web-servlet-filter/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<parent>
<groupId>io.opentracing.contrib</groupId>
<artifactId>opentracing-web-servlet-filter-parent</artifactId>
<version>0.0.10-SNAPSHOT</version>
<version>0.1.0-RC1-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import io.opentracing.BaseSpan;
import io.opentracing.Span;
import io.opentracing.tag.Tags;

/**
Expand All @@ -31,7 +31,7 @@ public interface ServletFilterSpanDecorator {
* @param httpServletRequest request
* @param span span to decorate
*/
void onRequest(HttpServletRequest httpServletRequest, BaseSpan<?> span);
void onRequest(HttpServletRequest httpServletRequest, Span span);

/**
* Decorate span after {@link javax.servlet.Filter#doFilter(ServletRequest, ServletResponse, FilterChain)}. When it
Expand All @@ -41,7 +41,7 @@ public interface ServletFilterSpanDecorator {
* @param httpServletResponse response
* @param span span to decorate
*/
void onResponse(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, BaseSpan<?> span);
void onResponse(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Span span);

/**
* Decorate span when an exception is thrown during processing in
Expand All @@ -53,7 +53,7 @@ public interface ServletFilterSpanDecorator {
* @param span span to decorate
*/
void onError(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
Throwable exception, BaseSpan<?> span);
Throwable exception, Span span);

/**
* Decorate span on asynchronous request timeout. It is called in
Expand All @@ -65,7 +65,7 @@ void onError(HttpServletRequest httpServletRequest, HttpServletResponse httpServ
* @param span span to decorate
*/
void onTimeout(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
long timeout, BaseSpan<?> span);
long timeout, Span span);

/**
* Adds standard tags to span. {@link Tags#HTTP_URL}, {@link Tags#HTTP_STATUS}, {@link Tags#HTTP_METHOD} and
Expand All @@ -75,7 +75,7 @@ void onTimeout(HttpServletRequest httpServletRequest, HttpServletResponse httpSe
*/
ServletFilterSpanDecorator STANDARD_TAGS = new ServletFilterSpanDecorator() {
@Override
public void onRequest(HttpServletRequest httpServletRequest, BaseSpan<?> span) {
public void onRequest(HttpServletRequest httpServletRequest, Span span) {
Tags.COMPONENT.set(span, "java-web-servlet");

Tags.HTTP_METHOD.set(span, httpServletRequest.getMethod());
Expand All @@ -85,13 +85,13 @@ public void onRequest(HttpServletRequest httpServletRequest, BaseSpan<?> span) {

@Override
public void onResponse(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
BaseSpan<?> span) {
Span span) {
Tags.HTTP_STATUS.set(span, httpServletResponse.getStatus());
}

@Override
public void onError(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
Throwable exception, BaseSpan<?> span) {
Throwable exception, Span span) {
Tags.ERROR.set(span, Boolean.TRUE);
span.log(logsForException(exception));

Expand All @@ -103,7 +103,7 @@ public void onError(HttpServletRequest httpServletRequest, HttpServletResponse h

@Override
public void onTimeout(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
long timeout, BaseSpan<?> span) {
long timeout, Span span) {
Tags.ERROR.set(span, Boolean.TRUE);

Map<String, Object> timeoutLogs = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import io.opentracing.ActiveSpan;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;
Expand Down Expand Up @@ -151,63 +151,62 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
SpanContext extractedContext = tracer.extract(Format.Builtin.HTTP_HEADERS,
new HttpServletRequestExtractAdapter(httpRequest));

final ActiveSpan span = tracer.buildSpan(httpRequest.getMethod())
final Scope scope = tracer.buildSpan(httpRequest.getMethod())
.asChildOf(extractedContext)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.startActive();
.startActive(false);

httpRequest.setAttribute(SERVER_SPAN_CONTEXT, span.context());
httpRequest.setAttribute(SERVER_SPAN_CONTEXT, scope.span().context());

for (ServletFilterSpanDecorator spanDecorator: spanDecorators) {
spanDecorator.onRequest(httpRequest, span);
spanDecorator.onRequest(httpRequest, scope.span());
}

try {
chain.doFilter(servletRequest, servletResponse);
if (!httpRequest.isAsyncStarted()) {
for (ServletFilterSpanDecorator spanDecorator : spanDecorators) {
spanDecorator.onResponse(httpRequest, httpResponse, span);
spanDecorator.onResponse(httpRequest, httpResponse, scope.span());
}
}
// catch all exceptions (e.g. RuntimeException, ServletException...)
} catch (Throwable ex) {
for (ServletFilterSpanDecorator spanDecorator : spanDecorators) {
spanDecorator.onError(httpRequest, httpResponse, ex, span);
spanDecorator.onError(httpRequest, httpResponse, ex, scope.span());
}
throw ex;
} finally {
if (httpRequest.isAsyncStarted()) {
final ActiveSpan.Continuation cont = span.capture();
// what if async is already finished? This would not be called
httpRequest.getAsyncContext()
.addListener(new AsyncListener() {
@Override
public void onComplete(AsyncEvent event) throws IOException {
try (ActiveSpan activeSpan = cont.activate()) {
try (Scope asyncScope = tracer.scopeManager().activate(scope.span())) {
for (ServletFilterSpanDecorator spanDecorator: spanDecorators) {
spanDecorator.onResponse((HttpServletRequest) event.getSuppliedRequest(),
(HttpServletResponse) event.getSuppliedResponse(), span);
(HttpServletResponse) event.getSuppliedResponse(), asyncScope.span());
}
}
}

@Override
public void onTimeout(AsyncEvent event) throws IOException {
try (ActiveSpan activeSpan = cont.activate()) {
try (Scope asyncScope = tracer.scopeManager().activate(scope.span())) {
for (ServletFilterSpanDecorator spanDecorator: spanDecorators) {
spanDecorator.onTimeout((HttpServletRequest) event.getSuppliedRequest(),
(HttpServletResponse) event.getSuppliedResponse(),
event.getAsyncContext().getTimeout(), span);
event.getAsyncContext().getTimeout(), asyncScope.span());
}
}
}

@Override
public void onError(AsyncEvent event) throws IOException {
try (ActiveSpan activeSpan = cont.activate()) {
try (Scope asyncScope = tracer.scopeManager().activate(scope.span())) {
for (ServletFilterSpanDecorator spanDecorator: spanDecorators) {
spanDecorator.onError((HttpServletRequest) event.getSuppliedRequest(),
(HttpServletResponse) event.getSuppliedResponse(), event.getThrowable(), span);
(HttpServletResponse) event.getSuppliedResponse(), event.getThrowable(), asyncScope.span());
}
}
}
Expand All @@ -216,8 +215,13 @@ public void onError(AsyncEvent event) throws IOException {
public void onStartAsync(AsyncEvent event) throws IOException {
}
});
} else {
// If not async, then need to explicitly finish the span associated with the scope.
// This is necessary, as we don't know whether this request is being handled
// asynchronously until after the scope has already been started.
scope.span().finish();
}
span.deactivate();
scope.close();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@
import org.junit.Before;
import org.mockito.Mockito;

import io.opentracing.ActiveSpan;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.mock.MockTracer;
import io.opentracing.util.GlobalTracer;
import io.opentracing.util.ThreadLocalActiveSpanSource;
import io.opentracing.util.ThreadLocalScopeManager;

/**
* @author Pavol Loffay
Expand All @@ -47,7 +48,7 @@ public abstract class AbstractJettyTest {

@Before
public void beforeTest() throws Exception {
mockTracer = Mockito.spy(new MockTracer(new ThreadLocalActiveSpanSource(), MockTracer.Propagator.TEXT_MAP));
mockTracer = Mockito.spy(new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP));

ServletContextHandler servletContext = new ServletContextHandler();
servletContext.setContextPath(contextPath);
Expand Down Expand Up @@ -129,7 +130,7 @@ public CurrentSpanServlet(Tracer tracer) {
public void doGet(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {

tracer.activeSpan().setTag("CurrentSpan", true);
tracer.scopeManager().active().span().setTag("CurrentSpan", true);
}
}

Expand Down Expand Up @@ -161,13 +162,13 @@ public void doGet(HttpServletRequest request, HttpServletResponse response)
final AsyncContext asyncContext = request.startAsync(request, response);

// TODO: This could be avoided by using an OpenTracing aware Runnable (when available)
final ActiveSpan.Continuation cont = tracer.activeSpan().capture();
final Span cont = tracer.scopeManager().active().span();

asyncContext.start(new Runnable() {
@Override
public void run() {
HttpServletResponse asyncResponse = (HttpServletResponse) asyncContext.getResponse();
try (ActiveSpan activeSpan = cont.activate()) {
try (Scope activeScope = tracer.scopeManager().activate(cont, false)) {
try {
Thread.sleep(ASYNC_SLEEP_TIME_MS);
asyncResponse.setStatus(204);
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<groupId>io.opentracing.contrib</groupId>
<artifactId>opentracing-web-servlet-filter-parent</artifactId>
<version>0.0.10-SNAPSHOT</version>
<version>0.1.0-RC1-SNAPSHOT</version>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought it might be worth bumping the minor version as there are breaking changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good, perhaps we could merge it to v0.1.0-RC1 branch. (We have to include the modified publis.sh)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it just a straight copy of the publish.sh from opentracing-java?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will raise a PR against the master, the current version of the script has to be modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok - although I've just added it to this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

opentracing/opentracing-java#198

This change is missing in the script. Please add it once it's merged. Or we can add it as a separate PR to the master.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This can also be left as it is. The version will be changed by release process.

<packaging>pom</packaging>

<name>${project.groupId}:${project.artifactId}</name>
Expand Down Expand Up @@ -39,7 +39,7 @@
<maven.compiler.target>1.7</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

<version.io.opentracing>0.30.0</version.io.opentracing>
<version.io.opentracing>0.31.0-RC1</version.io.opentracing>
<version.javax.servlet-javax.servlet-api>3.0.1</version.javax.servlet-javax.servlet-api>
<version.junit>4.12</version.junit>
<version.org.mockito-mockito-all>1.10.19</version.org.mockito-mockito-all>
Expand Down
24 changes: 19 additions & 5 deletions travis/publish.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
#
# Copyright 2016-2017 The OpenTracing Authors
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
# in compliance with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed under the License
# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
# or implied. See the License for the specific language governing permissions and limitations under
# the License.
#

set -euo pipefail
set -x

Expand Down Expand Up @@ -46,20 +60,20 @@ check_travis_branch_equals_travis_tag() {

check_release_tag() {
tag="${TRAVIS_TAG}"
if [[ "$tag" =~ ^[[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+$ ]]; then
if [[ "$tag" =~ ^[[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+(\.RC[[:digit:]]+)?$ ]]; then
echo "Build started by version tag $tag. During the release process tags like this"
echo "are created by the 'release' Maven plugin. Nothing to do here."
exit 0
elif [[ ! "$tag" =~ ^release-[[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+$ ]]; then
echo "You must specify a tag of the format 'release-0.0.0' to release this project."
elif [[ ! "$tag" =~ ^release-[[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+(\.RC[[:digit:]]+)?$ ]]; then
echo "You must specify a tag of the format 'release-0.0.0' or 'release-0.0.0.RC0' to release this project."
echo "The provided tag ${tag} doesn't match that. Aborting."
exit 1
fi
}

is_release_commit() {
project_version=$(./mvnw help:evaluate -N -Dexpression=project.version|grep -v '\[')
if [[ "$project_version" =~ ^[[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+$ ]]; then
project_version=$(./mvnw help:evaluate -N -Dexpression=project.version|sed -n '/^[0-9]/p')
if [[ "$project_version" =~ ^[[:digit:]]+\.[[:digit:]]+\.[[:digit:]]+(\.RC[[:digit:]]+)?$ ]]; then
echo "Build started by release commit $project_version. Will synchronize to maven central."
return 0
else
Expand Down