Skip to content

Commit

Permalink
4.x: MP Opentracing TCK fix (#5599)
Browse files Browse the repository at this point in the history
* TCK OpenTracing
* Fix absolute path handling in Routing and base path handling in JAX-RS
* Only send trailer headers when expected by the client.
  • Loading branch information
tomas-langer committed Dec 6, 2022
1 parent 7c9381e commit b47ae71
Show file tree
Hide file tree
Showing 17 changed files with 620 additions and 186 deletions.
5 changes: 5 additions & 0 deletions common/http/src/main/java/io/helidon/common/http/Http.java
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,11 @@ public static final class HeaderValues {
*/
public static final HeaderValue CACHE_NORMAL = Header.createCached(Header.CACHE_CONTROL, "no-transform");

/**
* TE header set to {@code trailers}, used to enable trailer headers.
*/
public static final HeaderValue TE_TRAILERS = Header.createCached(Header.TE, "trailers");

private HeaderValues() {
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import io.helidon.nima.webserver.http.ServerRequest;
import io.helidon.nima.webserver.http.ServerResponse;
import io.helidon.nima.webserver.http1.Http1Route;
import io.helidon.nima.webserver.tracing.TracingSupport;
import io.helidon.nima.webserver.tracing.TracingFeature;
import io.helidon.tracing.Span;
import io.helidon.tracing.Tracer;
import io.helidon.tracing.TracerBuilder;
Expand Down Expand Up @@ -51,7 +51,7 @@ public static void main(String[] args) {
.port(8080)
.host("127.0.0.1")
.routing(router -> router
.update(TracingSupport.create(tracer)::register)
.addFeature(TracingFeature.create(tracer))
.route(Http1Route.route(GET, "/versionspecific", new TracedHandler(tracer, "HTTP/1.1 route")))
.route(Http2Route.route(GET, "/versionspecific", new TracedHandler(tracer, "HTTP/2 route")))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,6 @@ public void routing(HttpRules rules) {
rules.any(this::handle);
}

private void handle(ServerRequest req, ServerResponse res) {
Contexts.runInContext(req.context(), () -> doHandle(req.context(), req, res));
}

@Override
public void beforeStart() {
appHandler.onStartup(container);
Expand Down Expand Up @@ -187,14 +183,20 @@ private static URI baseUri(ServerRequest req) {
return URI.create(uri);
}

private void handle(ServerRequest req, ServerResponse res) {
Contexts.runInContext(req.context(), () -> doHandle(req.context(), req, res));
}

private void doHandle(Context ctx, ServerRequest req, ServerResponse res) {
URI baseUri = baseUri(req);
URI requestUri;

String rawPath = req.path().rawPath();
rawPath = rawPath.startsWith("/") ? rawPath.substring(1) : rawPath;
if (req.query().isEmpty()) {
requestUri = baseUri.resolve(req.path().rawPath());
requestUri = baseUri.resolve(rawPath);
} else {
requestUri = baseUri.resolve(req.path().rawPath() + "?" + req.query().rawValue());
requestUri = baseUri.resolve(rawPath + "?" + req.query().rawValue());
}

ContainerRequest requestContext = new ContainerRequest(baseUri,
Expand Down Expand Up @@ -347,14 +349,16 @@ public void setSuspendTimeout(long l, TimeUnit timeUnit) throws IllegalStateExce

@Override
public void commit() {
if (outputStream != null) {
try {
try {
if (outputStream == null) {
res.outputStream().close();
} else {
outputStream.close();
cdl.countDown();
} catch (IOException e) {
cdl.countDown();
throw new UncheckedIOException(e);
}
cdl.countDown();
} catch (IOException e) {
cdl.countDown();
throw new UncheckedIOException(e);
}
}

Expand Down
4 changes: 1 addition & 3 deletions microprofile/tests/tck/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@
<module>tck-jwt-auth</module>
-->
<module>tck-openapi</module>
<!-- TODO:Níma - fails for now, requires analysis (duplicate request scope in Jersey, probably related)
to injection managers -->
<!-- <module>tck-opentracing</module>-->
<module>tck-opentracing</module>
<module>tck-rest-client</module>
<module>tck-reactive-operators</module>
<!-- TODO:Níma - fails for now, requires analysis -->
Expand Down
32 changes: 32 additions & 0 deletions microprofile/tests/tck/tck-opentracing/logging.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#
# Copyright (c) 2022 Oracle and/or its affiliates.
#
# 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.
#

# Example Logging Configuration File
# For more information see $JAVA_HOME/jre/lib/logging.properties

# Send messages to the console
handlers=io.helidon.logging.jul.HelidonConsoleHandler

# HelidonConsoleHandler uses a SimpleFormatter subclass that replaces "!thread!" with the current thread
java.util.logging.SimpleFormatter.format=%1$tY.%1$tm.%1$td %1$tH:%1$tM:%1$tS %4$s %3$s !thread!: %5$s%6$s%n

# Global logging level. Can be overridden by specific loggers
.level=WARNING

io.helidon.level=INFO



4 changes: 4 additions & 0 deletions microprofile/tracing/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
<artifactId>jakarta.enterprise.cdi-api</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>io.helidon.nima.webserver</groupId>
<artifactId>helidon-nima-webserver-tracing</artifactId>
</dependency>
<dependency>
<groupId>io.helidon.tracing</groupId>
<artifactId>helidon-tracing-opentracing</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public MpTracingContextFilter(@Context Provider<ServerRequest> request) {
public void filter(ContainerRequestContext requestContext) {
ServerRequest serverRequest = this.request.get();

Tracer tracer = Tracer.global();
Tracer tracer = serverRequest.context().get(Tracer.class).orElseGet(Tracer::global);
Optional<SpanContext> parentSpan = Span.current().map(Span::context);

boolean clientEnabled = config.getOptionalValue("tracing.client.enabled", Boolean.class).orElse(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import io.helidon.config.Config;
import io.helidon.microprofile.server.JaxRsApplication;
import io.helidon.microprofile.server.JaxRsCdiExtension;
import io.helidon.microprofile.server.ServerCdiExtension;
import io.helidon.nima.webserver.tracing.TracingFeature;
import io.helidon.tracing.TracerBuilder;

import io.opentelemetry.opentracingshim.OpenTracingShim;
Expand Down Expand Up @@ -57,6 +59,7 @@ private void observeBeforeBeanDiscovery(@Observes BeforeBeanDiscovery bbd) {
private void prepareTracer(@Observes @Priority(PLATFORM_BEFORE + 1) @Initialized(ApplicationScoped.class) Object event,
BeanManager bm) {
JaxRsCdiExtension jaxrs = bm.getExtension(JaxRsCdiExtension.class);
ServerCdiExtension server = bm.getExtension(ServerCdiExtension.class);

Config config = ((Config) ConfigProvider.getConfig()).get("tracing");

Expand Down Expand Up @@ -105,9 +108,8 @@ private void prepareTracer(@Observes @Priority(PLATFORM_BEFORE + 1) @Initialized



// TODO Helidon Níma
// server.serverRoutingBuilder()
// .register(WebTracingConfig.create(config));
server.serverRoutingBuilder()
.addFeature(TracingFeature.create(helidonTracer, config));

jaxRsApps
.forEach(app -> app.resourceConfig().register(MpTracingFilter.class));
Expand Down
1 change: 1 addition & 0 deletions microprofile/tracing/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
requires io.helidon.common;
requires io.helidon.nima.webserver;
requires io.helidon.jersey.common;
requires io.helidon.nima.webserver.tracing;
requires transitive io.helidon.tracing;
requires transitive io.helidon.tracing.jersey;
requires io.helidon.tracing.tracerresolver;
Expand Down
4 changes: 4 additions & 0 deletions nima/webserver/tracing/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
<groupId>io.helidon.tracing</groupId>
<artifactId>helidon-tracing</artifactId>
</dependency>
<dependency>
<groupId>io.helidon.tracing</groupId>
<artifactId>helidon-tracing-config</artifactId>
</dependency>
<dependency>
<groupId>io.helidon.nima.testing.junit5</groupId>
<artifactId>helidon-nima-testing-junit5-webserver</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/*
* Copyright (c) 2019, 2022 Oracle and/or its affiliates.
*
* 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.
*/
package io.helidon.nima.webserver.tracing;

import java.util.Collection;
import java.util.LinkedList;
import java.util.List;

import io.helidon.common.http.Http;
import io.helidon.common.http.PathMatcher;
import io.helidon.common.http.PathMatchers;
import io.helidon.common.uri.UriPath;
import io.helidon.config.Config;
import io.helidon.tracing.config.TracingConfig;

/**
* Traced system configuration for web server for a specific path.
*/
public interface PathTracingConfig {
/**
* Create a new traced path configuration from {@link io.helidon.config.Config}.
*
* @param config config of a path
* @return traced path configuration
*/
static PathTracingConfig create(Config config) {
return builder().config(config).build();
}

/**
* Create a new builder to configure traced path configuration.
*
* @return a new builder instance
*/
static Builder builder() {
return new Builder();
}

/**
* Method used by Helidon to check if this tracing is valid for the specified method and path.
*
* @param method HTTP method
* @param path invoked path
* @return {@code true} if matched
*/
boolean matches(Http.Method method, UriPath path);

/**
* Associated configuration of tracing valid for the configured path and (possibly) methods.
*
* @return traced system configuration
*/
TracingConfig tracedConfig();

/**
* Fluent API builder for {@link io.helidon.nima.webserver.tracing.PathTracingConfig}.
*/
final class Builder implements io.helidon.common.Builder<Builder, PathTracingConfig> {
private final List<String> methods = new LinkedList<>();
private String path;
private TracingConfig tracedConfig;

private Builder() {
}

@Override
public PathTracingConfig build() {
// immutable
final Collection<Http.Method> finalMethods = methods.stream()
.map(Http.Method::create)
.toList();
final TracingConfig finalTracingConfig = tracedConfig;

PathMatcher pathMatcher = PathMatchers.create(path);
Http.MethodPredicate methodPredicate = Http.Method.predicate(finalMethods);

return new PathTracingConfig() {
@Override
public boolean matches(Http.Method method, UriPath path) {
return methodPredicate.test(method) && pathMatcher.match(path).accepted();
}

@Override
public TracingConfig tracedConfig() {
return finalTracingConfig;
}

@Override
public String toString() {
return path + "(" + finalMethods + "): " + finalTracingConfig;
}
};
}

/**
* Update this builder from provided {@link io.helidon.config.Config}.
*
* @param config config to update this builder from
* @return updated builder instance
*/
public Builder config(Config config) {
path(config.get("path").asString().get());
List<String> methods = config.get("methods").asList(String.class).orElse(null);
if (null != methods) {
methods(methods);
}
tracingConfig(TracingConfig.create(config));

return this;
}

/**
* Path to register the traced configuration on.
*
* @param path path as understood by {@link io.helidon.nima.webserver.http.HttpRouting.Builder} of web server
* @return updated builder instance
*/
public Builder path(String path) {
this.path = path;
return this;
}

/**
* HTTP methods to restrict registration of this configuration on web server.
*
* @param methods list of methods to use, empty means all methods
* @return updated builder instance
*/
public Builder methods(List<String> methods) {
this.methods.clear();
this.methods.addAll(methods);
return this;
}

/**
* Add a new HTTP method to restrict this configuration for.
*
* @param method method to add to the list of supported methods
* @return updated builder instance
*/
public Builder addMethod(String method) {
this.methods.add(method);
return this;
}

/**
* Configuration of a traced system to use on this path and possibly method(s).
*
* @param tracedConfig configuration of components, spans and span logs
* @return updated builder instance
*/
public Builder tracingConfig(TracingConfig tracedConfig) {
this.tracedConfig = tracedConfig;
return this;
}
}
}

0 comments on commit b47ae71

Please sign in to comment.