From 8cd295331f0c5c0287a9da6fef782da89facc991 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Thu, 1 Dec 2022 14:47:40 +0100 Subject: [PATCH 1/4] TCK OpenTracing --- microprofile/tests/tck/pom.xml | 4 +-- .../tck/tck-opentracing/logging.properties | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 microprofile/tests/tck/tck-opentracing/logging.properties diff --git a/microprofile/tests/tck/pom.xml b/microprofile/tests/tck/pom.xml index 55adbe508bb..d1096ec6e9a 100644 --- a/microprofile/tests/tck/pom.xml +++ b/microprofile/tests/tck/pom.xml @@ -43,9 +43,7 @@ tck-jwt-auth --> tck-openapi - - + tck-opentracing tck-rest-client tck-reactive-operators diff --git a/microprofile/tests/tck/tck-opentracing/logging.properties b/microprofile/tests/tck/tck-opentracing/logging.properties new file mode 100644 index 00000000000..bf874ecb8c4 --- /dev/null +++ b/microprofile/tests/tck/tck-opentracing/logging.properties @@ -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 + + + From 6333e7f3bd8a6141b16d266aae5e1b25389ebbdf Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Thu, 1 Dec 2022 17:13:51 +0100 Subject: [PATCH 2/4] Fix absolute path handling in Routing and base path handling in JAX-RS --- .../java/io/helidon/microprofile/server/JaxRsService.java | 6 ++++-- .../java/io/helidon/nima/webserver/http/HttpRouting.java | 3 +-- .../java/io/helidon/nima/webserver/http/RouteCrawler.java | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java index 51a3f5be141..87b1e75b950 100644 --- a/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java @@ -191,10 +191,12 @@ 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, diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java index f0f8cf38087..335ec1a202b 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/HttpRouting.java @@ -45,11 +45,10 @@ public final class HttpRouting implements Routing { private final Filters filters; private final ServiceRoute rootRoute; - private final ErrorHandlers errorHandlers; private final List features; private HttpRouting(Builder builder) { - this.errorHandlers = ErrorHandlers.create(builder.errorHandlers); + ErrorHandlers errorHandlers = ErrorHandlers.create(builder.errorHandlers); this.filters = Filters.create(errorHandlers, List.copyOf(builder.filters)); this.rootRoute = builder.rootRules.build(); this.features = List.copyOf(builder.features); diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/RouteCrawler.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/RouteCrawler.java index 45ef287eee6..91691d55a7a 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/RouteCrawler.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http/RouteCrawler.java @@ -190,7 +190,7 @@ public Parameters pathParameters() { @Override public RoutedPath absolute() { - return new CrawlerRoutedPath(path, templateParams); + return new CrawlerRoutedPath(path.absolute(), templateParams); } } } From bf63c0d085ed93d67cfe8394f29e3e0dc58b896a Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Fri, 2 Dec 2022 15:12:44 +0100 Subject: [PATCH 3/4] Tracing - squashable 1 --- .../examples/nima/tracing/TracingMain.java | 4 +- microprofile/tracing/pom.xml | 4 + .../tracing/TracingCdiExtension.java | 8 +- .../tracing/src/main/java/module-info.java | 1 + nima/webserver/tracing/pom.xml | 4 + .../webserver/tracing/PathTracingConfig.java | 170 ++++++++ ...racingSupport.java => TracingFeature.java} | 25 +- .../webserver/tracing/WebTracingConfig.java | 389 ++++++++++++++++++ .../tracing/src/main/java/module-info.java | 1 + 9 files changed, 587 insertions(+), 19 deletions(-) create mode 100644 nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java rename nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/{TracingSupport.java => TracingFeature.java} (89%) create mode 100644 nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java diff --git a/examples/nima/tracing/src/main/java/io/helidon/examples/nima/tracing/TracingMain.java b/examples/nima/tracing/src/main/java/io/helidon/examples/nima/tracing/TracingMain.java index ca4b0842d10..d3129c9d1c6 100644 --- a/examples/nima/tracing/src/main/java/io/helidon/examples/nima/tracing/TracingMain.java +++ b/examples/nima/tracing/src/main/java/io/helidon/examples/nima/tracing/TracingMain.java @@ -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; @@ -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"))) ) diff --git a/microprofile/tracing/pom.xml b/microprofile/tracing/pom.xml index 3e935fe139c..ab2659c48eb 100644 --- a/microprofile/tracing/pom.xml +++ b/microprofile/tracing/pom.xml @@ -40,6 +40,10 @@ jakarta.enterprise.cdi-api provided + + io.helidon.nima.webserver + helidon-nima-webserver-tracing + io.helidon.tracing helidon-tracing-opentracing diff --git a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java index b260403c1c1..126cd8d2ebd 100644 --- a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java +++ b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java @@ -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.WebTracingConfig; import io.helidon.tracing.TracerBuilder; import io.opentelemetry.opentracingshim.OpenTracingShim; @@ -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"); @@ -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() + .register(WebTracingConfig.create(config)); jaxRsApps .forEach(app -> app.resourceConfig().register(MpTracingFilter.class)); diff --git a/microprofile/tracing/src/main/java/module-info.java b/microprofile/tracing/src/main/java/module-info.java index 1ffdaec060a..1cf6d6b19b7 100644 --- a/microprofile/tracing/src/main/java/module-info.java +++ b/microprofile/tracing/src/main/java/module-info.java @@ -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; diff --git a/nima/webserver/tracing/pom.xml b/nima/webserver/tracing/pom.xml index 6f4643a9c8c..56c939c0a8e 100644 --- a/nima/webserver/tracing/pom.xml +++ b/nima/webserver/tracing/pom.xml @@ -41,6 +41,10 @@ io.helidon.tracing helidon-tracing + + io.helidon.tracing + helidon-tracing-config + io.helidon.nima.testing.junit5 helidon-nima-testing-junit5-webserver diff --git a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java new file mode 100644 index 00000000000..d3731fd1896 --- /dev/null +++ b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java @@ -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.LinkedList; +import java.util.List; + +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(); + } + + /** + * Path this configuration should configure. + * @return path on the web server + * @see io.helidon.nima.webserver.http.HttpRouting.Builder#any(String, io.helidon.nima.webserver.http.Handler) + */ + String path(); + + /** + * Method(s) this configuration should be valid for. This can be used to restrict the configuration + * only to specific HTTP methods (such as {@code GET} or {@code POST}). + * + * @return list of methods, if empty, this configuration is valid for any method + */ + List methods(); + + /** + * 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 { + private final List methods = new LinkedList<>(); + private String path; + private TracingConfig tracedConfig; + + private Builder() { + } + + @Override + public PathTracingConfig build() { + // immutable + final String finalPath = path; + final List finalMethods = new LinkedList<>(methods); + final TracingConfig finalTracingConfig = tracedConfig; + + return new PathTracingConfig() { + @Override + public String path() { + return finalPath; + } + + @Override + public List methods() { + return finalMethods; + } + + @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 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 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; + } + } +} diff --git a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingSupport.java b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java similarity index 89% rename from nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingSupport.java rename to nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java index b68cc2ac31d..9fdb5414008 100644 --- a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingSupport.java +++ b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java @@ -24,6 +24,7 @@ import io.helidon.common.http.HttpPrologue; import io.helidon.nima.webserver.http.Filter; import io.helidon.nima.webserver.http.FilterChain; +import io.helidon.nima.webserver.http.HttpFeature; import io.helidon.nima.webserver.http.HttpRouting; import io.helidon.nima.webserver.http.RoutingRequest; import io.helidon.nima.webserver.http.RoutingResponse; @@ -35,18 +36,18 @@ import io.helidon.tracing.Tracer; /** - * Open Telemetry tracing support. + * Tracing support for Níma WebServer. */ -public class TracingSupport { +public class TracingFeature implements HttpFeature { private final boolean enabled; private final Filter filter; - private TracingSupport(Filter filter) { + private TracingFeature(Filter filter) { this.enabled = true; this.filter = filter; } - private TracingSupport() { + private TracingFeature() { this.enabled = false; this.filter = null; } @@ -57,22 +58,18 @@ private TracingSupport() { * @param tracer tracer * @return new tracing support */ - public static TracingSupport create(Tracer tracer) { + public static TracingFeature create(Tracer tracer) { if (tracer.enabled()) { - return new TracingSupport(new TracingFilter(tracer)); + return new TracingFeature(new TracingFilter(tracer)); } else { - return new TracingSupport(); + return new TracingFeature(); } } - /** - * Register tracing support filter with the HTTP routing. - * - * @param builder routing builder - */ - public void register(HttpRouting.Builder builder) { + @Override + public void setup(HttpRouting.Builder routing) { if (enabled) { - builder.addFilter(filter); + routing.addFilter(filter); } } diff --git a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java new file mode 100644 index 00000000000..8b0c198adb9 --- /dev/null +++ b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java @@ -0,0 +1,389 @@ +/* + * 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.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; + +import io.helidon.common.context.Context; +import io.helidon.common.http.Http; +import io.helidon.common.http.PathMatchers; +import io.helidon.config.Config; +import io.helidon.nima.webserver.WebServer; +import io.helidon.nima.webserver.http.Handler; +import io.helidon.nima.webserver.http.HttpRules; +import io.helidon.nima.webserver.http.HttpService; +import io.helidon.nima.webserver.http.ServerRequest; +import io.helidon.nima.webserver.http.ServerResponse; +import io.helidon.tracing.HeaderProvider; +import io.helidon.tracing.Span; +import io.helidon.tracing.SpanContext; +import io.helidon.tracing.Tag; +import io.helidon.tracing.Tracer; +import io.helidon.tracing.config.SpanTracingConfig; +import io.helidon.tracing.config.TracingConfig; +import io.helidon.tracing.config.TracingConfigUtil; + +/** + * Tracing configuration for webserver. + * Tracing configuration has two components - an overall (application wide) {@link io.helidon.tracing.config.TracingConfig} + * and a path specific {@link io.helidon.nima.webserver.tracing.PathTracingConfig}. + */ +public abstract class WebTracingConfig implements HttpService { + /** + * No side effects. + */ + protected WebTracingConfig() { + } + + /** + * Create a tracing configuration that is enabled for all paths and spans (that are enabled by default). + * + * @return tracing configuration to register with + * {@link + * io.helidon.nima.webserver.http.HttpRouting.Builder#register(java.util.function.Supplier[])} + */ + public static WebTracingConfig create(Tracer tracer) { + return create(tracer, TracingConfig.ENABLED); + } + + /** + * Create a new tracing support base on {@link io.helidon.tracing.config.TracingConfig}. + * + * @param configuration traced system configuration + * @return a new tracing support to register with web server routing + */ + public static WebTracingConfig create(Tracer tracer, TracingConfig configuration) { + return builder().tracer(tracer).envConfig(configuration).build(); + } + + /** + * Create a new tracing support base on {@link io.helidon.config.Config}. + * + * @param config to base this support on + * @return a new tracing support to register with web server routing + */ + public static WebTracingConfig create(Config config) { + return builder().config(config).build(); + } + + /** + * A fluent API builder to create tracing support. + * + * @return a new builder instance + */ + public static Builder builder() { + return new Builder(); + } + + @Override + public void routing(HttpRules rules) { + pathConfigs().forEach(path -> { + List methods = path.methods() + .stream() + .map(Http.Method::create) + .collect(Collectors.toList()); + + TracingConfig wrappedPath = path.tracedConfig(); + if (methods.isEmpty()) { + rules.any(path.path(), new TracingConfigHandler(wrappedPath)); + } else { + rules.route(Http.Method.predicate(methods), + PathMatchers.create(path.path()), + new TracingConfigHandler(wrappedPath)); + } + }); + // and now register the tracing of requests + rules.any(new RequestSpanHandler(tracer())); + } + + abstract Tracer tracer(); + + /** + * Tracing configuration. + * This is the configuration set up for the whole server. There can also be a path specific configuration available + * through {@link #pathConfigs()}. + * + * @return tracing configuration for all components + */ + abstract TracingConfig envConfig(); + + /** + * Path specific tracing configurations. + * + * @return tracing configuration per path (and HTTP methods) + */ + abstract Iterable pathConfigs(); + + /** + * A fluent API builder for {@link io.helidon.nima.webserver.tracing.WebTracingConfig}. + */ + public static class Builder implements io.helidon.common.Builder { + private final List pathTracingConfigs = new LinkedList<>(); + private TracingConfig tracedConfig = TracingConfig.ENABLED; + private Tracer tracer; + + /** + * OpenTracing spec states that certain MP paths need to be disabled by default. + * Note that if a user changes the default location of any of these using + * web-context's, then they would need to provide these exclusions manually. + * + * The default path configs below are overridable via configuration. For example, + * health could be enabled by setting {@code tracing.paths.0.path=/health} and + * {@code tracing.paths.0.enabled=true}. + */ + Builder() { + addPathConfig(PathTracingConfig.builder() + .path("/metrics") + .tracingConfig(TracingConfig.DISABLED) + .build()); + // Simplified matching for base, vendor and application paths + addPathConfig(PathTracingConfig.builder() + .path("/metrics/{+}") + .tracingConfig(TracingConfig.DISABLED) + .build()); + addPathConfig(PathTracingConfig.builder() + .path("/health") + .tracingConfig(TracingConfig.DISABLED) + .build()); + addPathConfig(PathTracingConfig.builder() + .path("/health/{+}") + .tracingConfig(TracingConfig.DISABLED) + .build()); + addPathConfig(PathTracingConfig.builder() + .path("/openapi") + .tracingConfig(TracingConfig.DISABLED) + .build()); + } + + @Override + public WebTracingConfig build() { + final TracingConfig envConfig = this.tracedConfig; + final List pathConfigs = new LinkedList<>(this.pathTracingConfigs); + final Tracer tracer = this.tracer; + + return new WebTracingConfig() { + @Override + Tracer tracer() { + return tracer; + } + + @Override + public TracingConfig envConfig() { + return envConfig; + } + + @Override + public Iterable pathConfigs() { + return pathConfigs; + } + }; + } + + /** + * Add a path specific configuration of tracing. + * + * @param pathTracingConfig configuration of tracing for a specific path + * @return updated builder instance + */ + public Builder addPathConfig(PathTracingConfig pathTracingConfig) { + this.pathTracingConfigs.add(pathTracingConfig); + return this; + } + + /** + * Use the provided configuration as a default for any request. + * + * @param tracingConfig default web server tracing configuration + * @return updated builder instance + */ + public Builder envConfig(TracingConfig tracingConfig) { + this.tracedConfig = tracingConfig; + return this; + } + + /** + * Update builder from {@link io.helidon.config.Config}. + * + * @param config config to read default configuration and path specific configuration from + * @return updated builder instance + */ + public Builder config(Config config) { + // read the overall configuration + envConfig(TracingConfig.create(config)); + + // and then the paths + Config allPaths = config.get("paths"); + allPaths.asNodeList().ifPresent(this::addPaths); + return this; + } + + private void addPaths(List configs) { + configs.stream() + .map(PathTracingConfig::create) + .forEach(this::addPathConfig); + } + } + + // this class exists so tracing of handler in webserver shows nice class name and not a lambda + private static final class TracingConfigHandler implements Handler { + private final TracingConfig pathSpecific; + + private TracingConfigHandler(TracingConfig pathSpecific) { + this.pathSpecific = pathSpecific; + } + + @Override + public void handle(ServerRequest req, ServerResponse res) { + Optional existing = req.context().get(TracingConfig.class); + if (existing.isPresent()) { + req.context().register(TracingConfig.merge(existing.get(), pathSpecific)); + } else { + req.context().register(pathSpecific); + } + res.next(); + } + } + + static final class RequestSpanHandler implements Handler { + private static final String TRACING_SPAN_HTTP_REQUEST = "HTTP Request"; + private final AtomicBoolean checkedIfShouldTrace = new AtomicBoolean(); + private final Tracer tracer; + private volatile boolean shouldTrace = true; + + RequestSpanHandler(Tracer tracer) { + this.tracer = tracer; + } + + @Override + public void handle(ServerRequest req, ServerResponse res) { + if (shouldTrace && checkedIfShouldTrace.compareAndSet(false, true)) { + shouldTrace = tracer().enabled(); + } + + if (shouldTrace) { + doAccept(req, res); + } + + res.next(); + } + + private void doAccept(ServerRequest req, ServerResponse res) { + // must run in context + Context context = req.context(); + + SpanTracingConfig spanConfig = TracingConfigUtil + .spanConfig(WebServer.TRACING_COMPONENT, TRACING_SPAN_HTTP_REQUEST, context); + + SpanContext inboundSpanContext = tracer.extract(new TracingHeaderProvider(req.headers().toMap())) + .orElse(null); + + if (inboundSpanContext != null) { + // register as parent span + context.register(inboundSpanContext); + context.register(TracingConfig.class, inboundSpanContext); + } + + if (!spanConfig.enabled()) { + return; + } + + String spanName = spanConfig.newName().orElse(TRACING_SPAN_HTTP_REQUEST); + if (spanName.indexOf('%') > -1) { + spanName = String.format(spanName, req.method().text(), req.path(), req.query()); + } + // tracing is enabled, so we replace the parent span with web server parent span + Span.Builder spanBuilder = tracer.spanBuilder(spanName) + .kind(Span.Kind.SERVER) + .tag(Tag.COMPONENT.create("helidon-reactive-webserver")) + .tag(Tag.HTTP_METHOD.create(req.method().text())) + .tag(Tag.HTTP_URL.create(req.uri().toString())) + .tag(Tag.HTTP_VERSION.create(req.version().value())); + + if (inboundSpanContext != null) { + spanBuilder.parent(inboundSpanContext); + } + + // cannot use startActive, as it conflicts with the thread model we use + Span span = spanBuilder.start(); + + context.register(span.context()); + context.register(TracingConfig.class, span.context()); + + res.whenSent() + .thenRun(() -> { + Http.Status httpStatus = res.status(); + if (httpStatus != null) { + int statusCode = httpStatus.code(); + span.tag(Tag.HTTP_STATUS.create(statusCode)); + + if (statusCode >= 400) { + span.status(Span.Status.ERROR); + + span.addEvent("error", Map.of("message", "Response HTTP status: " + statusCode, + "error.kind", statusCode < 500 ? "ClientError" : "ServerError")); + } + } + span.end(); + }) + .exceptionally(t -> { + span.end(t); + return null; + }); + } + + private static class TracingHeaderProvider implements HeaderProvider { + + private final Map> headers; + + TracingHeaderProvider(Map> headers) { + this.headers = headers; + } + + @Override + public Iterable keys() { + return headers.keySet(); + } + + @Override + public Optional get(String key) { + List strings = headers.get(key); + if (strings == null) { + return Optional.empty(); + } + return Optional.of(strings.get(0)); + } + + @Override + public Iterable getAll(String key) { + List strings = headers.get(key); + if (strings == null) { + return List.of(); + } + return strings; + } + + @Override + public boolean contains(String key) { + return headers.containsKey(key); + } + } + } +} diff --git a/nima/webserver/tracing/src/main/java/module-info.java b/nima/webserver/tracing/src/main/java/module-info.java index b6c050dc892..a09b7d08595 100644 --- a/nima/webserver/tracing/src/main/java/module-info.java +++ b/nima/webserver/tracing/src/main/java/module-info.java @@ -21,6 +21,7 @@ requires io.helidon.common.http; requires io.helidon.nima.webserver; requires io.helidon.tracing; + requires io.helidon.tracing.config; exports io.helidon.nima.webserver.tracing; } \ No newline at end of file From f53a1f5334e730004911d9cb522092d337b75840 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Tue, 6 Dec 2022 12:37:27 +0100 Subject: [PATCH 4/4] MP Tracing TCK fix. Only send trailer headers when expected by the client. --- .../java/io/helidon/common/http/Http.java | 5 + .../microprofile/server/JaxRsService.java | 22 +- .../tracing/MpTracingContextFilter.java | 2 +- .../tracing/TracingCdiExtension.java | 4 +- .../webserver/tracing/PathTracingConfig.java | 40 +- .../webserver/tracing/TracingFeature.java | 313 +++++++++++--- .../webserver/tracing/WebTracingConfig.java | 389 ------------------ .../webserver/http1/Http1ServerResponse.java | 24 +- 8 files changed, 317 insertions(+), 482 deletions(-) delete mode 100644 nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java diff --git a/common/http/src/main/java/io/helidon/common/http/Http.java b/common/http/src/main/java/io/helidon/common/http/Http.java index 2ff7d4ee150..58fb1578168 100644 --- a/common/http/src/main/java/io/helidon/common/http/Http.java +++ b/common/http/src/main/java/io/helidon/common/http/Http.java @@ -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() { } } diff --git a/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java b/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java index 87b1e75b950..527d44fcb62 100644 --- a/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java +++ b/microprofile/server/src/main/java/io/helidon/microprofile/server/JaxRsService.java @@ -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); @@ -187,6 +183,10 @@ 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; @@ -349,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); } } diff --git a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingContextFilter.java b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingContextFilter.java index b3afaa669f4..1b72795703b 100644 --- a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingContextFilter.java +++ b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingContextFilter.java @@ -65,7 +65,7 @@ public MpTracingContextFilter(@Context Provider 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 parentSpan = Span.current().map(Span::context); boolean clientEnabled = config.getOptionalValue("tracing.client.enabled", Boolean.class).orElse(true); diff --git a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java index 126cd8d2ebd..9d9a8bb98f9 100644 --- a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java +++ b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/TracingCdiExtension.java @@ -23,7 +23,7 @@ import io.helidon.microprofile.server.JaxRsApplication; import io.helidon.microprofile.server.JaxRsCdiExtension; import io.helidon.microprofile.server.ServerCdiExtension; -import io.helidon.nima.webserver.tracing.WebTracingConfig; +import io.helidon.nima.webserver.tracing.TracingFeature; import io.helidon.tracing.TracerBuilder; import io.opentelemetry.opentracingshim.OpenTracingShim; @@ -109,7 +109,7 @@ private void prepareTracer(@Observes @Priority(PLATFORM_BEFORE + 1) @Initialized server.serverRoutingBuilder() - .register(WebTracingConfig.create(config)); + .addFeature(TracingFeature.create(helidonTracer, config)); jaxRsApps .forEach(app -> app.resourceConfig().register(MpTracingFilter.class)); diff --git a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java index d3731fd1896..4effe1e6ea5 100644 --- a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java +++ b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/PathTracingConfig.java @@ -15,9 +15,14 @@ */ 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; @@ -27,6 +32,7 @@ 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 */ @@ -44,19 +50,13 @@ static Builder builder() { } /** - * Path this configuration should configure. - * @return path on the web server - * @see io.helidon.nima.webserver.http.HttpRouting.Builder#any(String, io.helidon.nima.webserver.http.Handler) - */ - String path(); - - /** - * Method(s) this configuration should be valid for. This can be used to restrict the configuration - * only to specific HTTP methods (such as {@code GET} or {@code POST}). + * Method used by Helidon to check if this tracing is valid for the specified method and path. * - * @return list of methods, if empty, this configuration is valid for any method + * @param method HTTP method + * @param path invoked path + * @return {@code true} if matched */ - List methods(); + boolean matches(Http.Method method, UriPath path); /** * Associated configuration of tracing valid for the configured path and (possibly) methods. @@ -79,19 +79,18 @@ private Builder() { @Override public PathTracingConfig build() { // immutable - final String finalPath = path; - final List finalMethods = new LinkedList<>(methods); + final Collection finalMethods = methods.stream() + .map(Http.Method::create) + .toList(); final TracingConfig finalTracingConfig = tracedConfig; - return new PathTracingConfig() { - @Override - public String path() { - return finalPath; - } + PathMatcher pathMatcher = PathMatchers.create(path); + Http.MethodPredicate methodPredicate = Http.Method.predicate(finalMethods); + return new PathTracingConfig() { @Override - public List methods() { - return finalMethods; + public boolean matches(Http.Method method, UriPath path) { + return methodPredicate.test(method) && pathMatcher.match(path).accepted(); } @Override @@ -136,6 +135,7 @@ public Builder path(String path) { /** * 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 */ diff --git a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java index 9fdb5414008..ba88ea41696 100644 --- a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java +++ b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/TracingFeature.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 Oracle and/or its affiliates. + * 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. @@ -18,132 +18,345 @@ import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Optional; +import io.helidon.common.context.Context; +import io.helidon.common.context.Contexts; import io.helidon.common.http.Http; import io.helidon.common.http.HttpPrologue; +import io.helidon.config.Config; import io.helidon.nima.webserver.http.Filter; import io.helidon.nima.webserver.http.FilterChain; import io.helidon.nima.webserver.http.HttpFeature; import io.helidon.nima.webserver.http.HttpRouting; import io.helidon.nima.webserver.http.RoutingRequest; import io.helidon.nima.webserver.http.RoutingResponse; +import io.helidon.nima.webserver.http.ServerRequest; import io.helidon.tracing.HeaderProvider; import io.helidon.tracing.Scope; import io.helidon.tracing.Span; import io.helidon.tracing.SpanContext; import io.helidon.tracing.Tag; import io.helidon.tracing.Tracer; +import io.helidon.tracing.config.SpanTracingConfig; +import io.helidon.tracing.config.TracingConfig; /** - * Tracing support for Níma WebServer. + * Tracing configuration for webserver. + * Tracing configuration has two components - an overall (application wide) {@link io.helidon.tracing.config.TracingConfig} + * and a path specific {@link io.helidon.nima.webserver.tracing.PathTracingConfig}. */ public class TracingFeature implements HttpFeature { private final boolean enabled; - private final Filter filter; + private final Tracer tracer; + private final TracingConfig envConfig; + private final List pathConfigs; - private TracingFeature(Filter filter) { - this.enabled = true; - this.filter = filter; + /** + * No side effects. + */ + private TracingFeature(Builder builder) { + this.enabled = builder.enabled; + this.tracer = builder.tracer; + this.envConfig = builder.tracedConfig; + this.pathConfigs = List.copyOf(builder.pathTracingConfigs); } - private TracingFeature() { - this.enabled = false; - this.filter = null; + /** + * Create a tracing configuration that is enabled for all paths and spans (that are enabled by default). + * + * @param tracer tracer to use for tracing spans created by this feature + * @return tracing configuration to register with + * {@link + * io.helidon.nima.webserver.http.HttpRouting.Builder#register(java.util.function.Supplier[])} + */ + public static TracingFeature create(Tracer tracer) { + return create(tracer, TracingConfig.ENABLED); } /** - * Create new support from an tracer instance. + * Create a new tracing support base on {@link io.helidon.tracing.config.TracingConfig}. * - * @param tracer tracer - * @return new tracing support + * @param tracer tracer to use for tracing spans created by this feature + * @param configuration traced system configuration + * @return a new tracing support to register with web server routing */ - public static TracingFeature create(Tracer tracer) { - if (tracer.enabled()) { - return new TracingFeature(new TracingFilter(tracer)); - } else { - return new TracingFeature(); - } + public static TracingFeature create(Tracer tracer, TracingConfig configuration) { + return builder().tracer(tracer).envConfig(configuration).build(); + } + + /** + * Create a new tracing support base on {@link io.helidon.config.Config}. + * + * @param tracer tracer to use for tracing spans created by this feature + * @param config to base this support on + * @return a new tracing support to register with web server routing + */ + public static TracingFeature create(Tracer tracer, Config config) { + return builder().tracer(tracer).config(config).build(); + } + + /** + * A fluent API builder to create tracing support. + * + * @return a new builder instance + */ + public static Builder builder() { + return new Builder(); } @Override public void setup(HttpRouting.Builder routing) { if (enabled) { - routing.addFilter(filter); + // and now register the tracing of requests + routing.addFilter(new TracingFilter(tracer, envConfig, pathConfigs)); + } + } + + /** + * A fluent API builder for {@link TracingFeature}. + */ + public static class Builder implements io.helidon.common.Builder { + private final List pathTracingConfigs = new LinkedList<>(); + + private TracingConfig tracedConfig = TracingConfig.ENABLED; + private Tracer tracer; + private boolean enabled = true; + + /** + * OpenTracing spec states that certain MP paths need to be disabled by default. + * Note that if a user changes the default location of any of these using + * web-context's, then they would need to provide these exclusions manually. + * + * The default path configs below are overridable via configuration. For example, + * health could be enabled by setting {@code tracing.paths.0.path=/health} and + * {@code tracing.paths.0.enabled=true}. + */ + Builder() { + addPathConfig(PathTracingConfig.builder() + .path("/metrics/*") + .tracingConfig(TracingConfig.DISABLED) + .build()); + addPathConfig(PathTracingConfig.builder() + .path("/health/*") + .tracingConfig(TracingConfig.DISABLED) + .build()); + addPathConfig(PathTracingConfig.builder() + .path("/openapi/*") + .tracingConfig(TracingConfig.DISABLED) + .build()); + } + + @Override + public TracingFeature build() { + if (tracer == null) { + throw new IllegalArgumentException("Tracing feature must be configured with a tracer"); + } + return new TracingFeature(this); + } + + /** + * Add a path specific configuration of tracing. + * + * @param pathTracingConfig configuration of tracing for a specific path + * @return updated builder instance + */ + public Builder addPathConfig(PathTracingConfig pathTracingConfig) { + this.pathTracingConfigs.add(pathTracingConfig); + return this; + } + + /** + * Use the provided configuration as a default for any request. + * + * @param tracingConfig default web server tracing configuration + * @return updated builder instance + */ + public Builder envConfig(TracingConfig tracingConfig) { + this.tracedConfig = tracingConfig; + return this; + } + + /** + * Update builder from {@link io.helidon.config.Config}. + * + * @param config config to read default configuration and path specific configuration from + * @return updated builder instance + */ + public Builder config(Config config) { + // read the overall configuration + envConfig(TracingConfig.create(config)); + + // and then the paths + Config allPaths = config.get("paths"); + allPaths.asNodeList().ifPresent(this::addPaths); + enabled(tracedConfig.enabled()); + return this; + } + + /** + * Explicitly enable/disable tracing feature. + * + * @param enabled if set to {@code false}, this feature will be disabled and tracing filter will never be registered + * @return updated builder + */ + public Builder enabled(boolean enabled) { + this.enabled = enabled; + return this; + } + + /** + * Tracer to use to extract inbound span context. + * + * @param tracer tracer to use + * @return updated builder + */ + public Builder tracer(Tracer tracer) { + this.tracer = tracer; + return this; + } + + private void addPaths(List configs) { + configs.stream() + .map(PathTracingConfig::create) + .forEach(this::addPathConfig); } } private static class TracingFilter implements Filter { + private static final String TRACING_SPAN_HTTP_REQUEST = "HTTP Request"; private final Tracer tracer; + private final TracingConfig envConfig; + private final List pathConfigs; - private TracingFilter(Tracer tracer) { + TracingFilter(Tracer tracer, TracingConfig envConfig, List pathConfigs) { this.tracer = tracer; + this.envConfig = envConfig; + this.pathConfigs = pathConfigs; } @Override public void filter(FilterChain chain, RoutingRequest req, RoutingResponse res) { - Optional context = tracer.extract(new NimaHeaderProvider(req)); + // context of the request - we register configuration and parent spans to it + Context context = req.context(); + TracingConfig resolved = configureTracingConfig(req, context); + + /* + Extract inbound span context, this will act as a parent of the new webserver span + */ + Optional inboundSpanContext = tracer.extract(new NimaHeaderProvider(req)); + + /* + Find configuration of the web server span (can customize name, disable etc.) + */ + SpanTracingConfig spanConfig = resolved.spanConfig("web-server", TRACING_SPAN_HTTP_REQUEST); + if (!spanConfig.enabled()) { + // nope, do not start this span, but still register parent span context for components further down + if (inboundSpanContext.isPresent()) { + context.register(inboundSpanContext.get()); + context.register(TracingConfig.class, inboundSpanContext.get()); + } + Contexts.runInContext(context, chain::proceed); + return; + } + /* + Create web server span + */ HttpPrologue prologue = req.prologue(); - Span span = tracer.spanBuilder(prologue.method().text() - + " " + req.path().rawPath() - + " " + prologue.protocol() - + "/" + prologue.protocolVersion()) + String spanName = spanConfig.newName().orElse(TRACING_SPAN_HTTP_REQUEST); + if (spanName.indexOf('%') > -1) { + spanName = String.format(spanName, prologue.method().text(), req.path().rawPath(), req.query().rawValue()); + } + // tracing is enabled, so we replace the parent span with web server parent span + Span span = tracer.spanBuilder(spanName) .kind(Span.Kind.SERVER) - .update(it -> context.ifPresent(it::parent)) + .update(it -> inboundSpanContext.ifPresent(it::parent)) .start(); + context.register(span.context()); + context.register(TracingConfig.class, span.context()); + try (Scope ignored = span.activate()) { + span.tag(Tag.COMPONENT.create("helidon-nima-webserver")); span.tag(Tag.HTTP_METHOD.create(prologue.method().text())); span.tag(Tag.HTTP_URL.create(prologue.protocol() + "://" + req.authority() + "/" + prologue.uriPath().path())); span.tag(Tag.HTTP_VERSION.create(prologue.protocolVersion())); - chain.proceed(); - } finally { + Contexts.runInContext(context, chain::proceed); + Http.Status status = res.status(); span.tag(Tag.HTTP_STATUS.create(status.code())); if (status.code() >= 400) { span.status(Span.Status.ERROR); + span.addEvent("error", Map.of("message", "Response HTTP status: " + status, + "error.kind", status.code() < 500 ? "ClientError" : "ServerError")); } else { span.status(Span.Status.OK); } span.end(); + } catch (Exception e) { + span.end(e); + throw e; } } - private static class NimaHeaderProvider implements HeaderProvider { - private final RoutingRequest request; + private TracingConfig configureTracingConfig(RoutingRequest req, Context context) { + TracingConfig discovered = null; - private NimaHeaderProvider(RoutingRequest request) { - this.request = request; - } - - @Override - public Iterable keys() { - List result = new LinkedList<>(); - for (Http.HeaderValue header : request.headers()) { - result.add(header.headerName().lowerCase()); + for (PathTracingConfig pathConfig : pathConfigs) { + if (pathConfig.matches(req.prologue().method(), req.prologue().uriPath())) { + if (discovered == null) { + discovered = pathConfig.tracedConfig(); + } else { + discovered = TracingConfig.merge(discovered, pathConfig.tracedConfig()); + } } - return result; } - @Override - public Optional get(String key) { - return request.headers().first(Http.Header.create(key)); + if (discovered == null) { + context.register(envConfig); + return envConfig; + } else { + context.register(discovered); + return discovered; } + } + } - @Override - public Iterable getAll(String key) { - return request.headers().all(Http.Header.create(key), List::of); - } + private static class NimaHeaderProvider implements HeaderProvider { + private final ServerRequest request; - @Override - public boolean contains(String key) { - return request.headers().contains(Http.Header.create(key)); + private NimaHeaderProvider(ServerRequest request) { + this.request = request; + } + + @Override + public Iterable keys() { + List result = new LinkedList<>(); + for (Http.HeaderValue header : request.headers()) { + result.add(header.headerName().lowerCase()); } + return result; + } + + @Override + public Optional get(String key) { + return request.headers().first(Http.Header.create(key)); + } + + @Override + public Iterable getAll(String key) { + return request.headers().all(Http.Header.create(key), List::of); + } + + @Override + public boolean contains(String key) { + return request.headers().contains(Http.Header.create(key)); } } } diff --git a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java b/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java deleted file mode 100644 index 8b0c198adb9..00000000000 --- a/nima/webserver/tracing/src/main/java/io/helidon/nima/webserver/tracing/WebTracingConfig.java +++ /dev/null @@ -1,389 +0,0 @@ -/* - * 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.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; - -import io.helidon.common.context.Context; -import io.helidon.common.http.Http; -import io.helidon.common.http.PathMatchers; -import io.helidon.config.Config; -import io.helidon.nima.webserver.WebServer; -import io.helidon.nima.webserver.http.Handler; -import io.helidon.nima.webserver.http.HttpRules; -import io.helidon.nima.webserver.http.HttpService; -import io.helidon.nima.webserver.http.ServerRequest; -import io.helidon.nima.webserver.http.ServerResponse; -import io.helidon.tracing.HeaderProvider; -import io.helidon.tracing.Span; -import io.helidon.tracing.SpanContext; -import io.helidon.tracing.Tag; -import io.helidon.tracing.Tracer; -import io.helidon.tracing.config.SpanTracingConfig; -import io.helidon.tracing.config.TracingConfig; -import io.helidon.tracing.config.TracingConfigUtil; - -/** - * Tracing configuration for webserver. - * Tracing configuration has two components - an overall (application wide) {@link io.helidon.tracing.config.TracingConfig} - * and a path specific {@link io.helidon.nima.webserver.tracing.PathTracingConfig}. - */ -public abstract class WebTracingConfig implements HttpService { - /** - * No side effects. - */ - protected WebTracingConfig() { - } - - /** - * Create a tracing configuration that is enabled for all paths and spans (that are enabled by default). - * - * @return tracing configuration to register with - * {@link - * io.helidon.nima.webserver.http.HttpRouting.Builder#register(java.util.function.Supplier[])} - */ - public static WebTracingConfig create(Tracer tracer) { - return create(tracer, TracingConfig.ENABLED); - } - - /** - * Create a new tracing support base on {@link io.helidon.tracing.config.TracingConfig}. - * - * @param configuration traced system configuration - * @return a new tracing support to register with web server routing - */ - public static WebTracingConfig create(Tracer tracer, TracingConfig configuration) { - return builder().tracer(tracer).envConfig(configuration).build(); - } - - /** - * Create a new tracing support base on {@link io.helidon.config.Config}. - * - * @param config to base this support on - * @return a new tracing support to register with web server routing - */ - public static WebTracingConfig create(Config config) { - return builder().config(config).build(); - } - - /** - * A fluent API builder to create tracing support. - * - * @return a new builder instance - */ - public static Builder builder() { - return new Builder(); - } - - @Override - public void routing(HttpRules rules) { - pathConfigs().forEach(path -> { - List methods = path.methods() - .stream() - .map(Http.Method::create) - .collect(Collectors.toList()); - - TracingConfig wrappedPath = path.tracedConfig(); - if (methods.isEmpty()) { - rules.any(path.path(), new TracingConfigHandler(wrappedPath)); - } else { - rules.route(Http.Method.predicate(methods), - PathMatchers.create(path.path()), - new TracingConfigHandler(wrappedPath)); - } - }); - // and now register the tracing of requests - rules.any(new RequestSpanHandler(tracer())); - } - - abstract Tracer tracer(); - - /** - * Tracing configuration. - * This is the configuration set up for the whole server. There can also be a path specific configuration available - * through {@link #pathConfigs()}. - * - * @return tracing configuration for all components - */ - abstract TracingConfig envConfig(); - - /** - * Path specific tracing configurations. - * - * @return tracing configuration per path (and HTTP methods) - */ - abstract Iterable pathConfigs(); - - /** - * A fluent API builder for {@link io.helidon.nima.webserver.tracing.WebTracingConfig}. - */ - public static class Builder implements io.helidon.common.Builder { - private final List pathTracingConfigs = new LinkedList<>(); - private TracingConfig tracedConfig = TracingConfig.ENABLED; - private Tracer tracer; - - /** - * OpenTracing spec states that certain MP paths need to be disabled by default. - * Note that if a user changes the default location of any of these using - * web-context's, then they would need to provide these exclusions manually. - * - * The default path configs below are overridable via configuration. For example, - * health could be enabled by setting {@code tracing.paths.0.path=/health} and - * {@code tracing.paths.0.enabled=true}. - */ - Builder() { - addPathConfig(PathTracingConfig.builder() - .path("/metrics") - .tracingConfig(TracingConfig.DISABLED) - .build()); - // Simplified matching for base, vendor and application paths - addPathConfig(PathTracingConfig.builder() - .path("/metrics/{+}") - .tracingConfig(TracingConfig.DISABLED) - .build()); - addPathConfig(PathTracingConfig.builder() - .path("/health") - .tracingConfig(TracingConfig.DISABLED) - .build()); - addPathConfig(PathTracingConfig.builder() - .path("/health/{+}") - .tracingConfig(TracingConfig.DISABLED) - .build()); - addPathConfig(PathTracingConfig.builder() - .path("/openapi") - .tracingConfig(TracingConfig.DISABLED) - .build()); - } - - @Override - public WebTracingConfig build() { - final TracingConfig envConfig = this.tracedConfig; - final List pathConfigs = new LinkedList<>(this.pathTracingConfigs); - final Tracer tracer = this.tracer; - - return new WebTracingConfig() { - @Override - Tracer tracer() { - return tracer; - } - - @Override - public TracingConfig envConfig() { - return envConfig; - } - - @Override - public Iterable pathConfigs() { - return pathConfigs; - } - }; - } - - /** - * Add a path specific configuration of tracing. - * - * @param pathTracingConfig configuration of tracing for a specific path - * @return updated builder instance - */ - public Builder addPathConfig(PathTracingConfig pathTracingConfig) { - this.pathTracingConfigs.add(pathTracingConfig); - return this; - } - - /** - * Use the provided configuration as a default for any request. - * - * @param tracingConfig default web server tracing configuration - * @return updated builder instance - */ - public Builder envConfig(TracingConfig tracingConfig) { - this.tracedConfig = tracingConfig; - return this; - } - - /** - * Update builder from {@link io.helidon.config.Config}. - * - * @param config config to read default configuration and path specific configuration from - * @return updated builder instance - */ - public Builder config(Config config) { - // read the overall configuration - envConfig(TracingConfig.create(config)); - - // and then the paths - Config allPaths = config.get("paths"); - allPaths.asNodeList().ifPresent(this::addPaths); - return this; - } - - private void addPaths(List configs) { - configs.stream() - .map(PathTracingConfig::create) - .forEach(this::addPathConfig); - } - } - - // this class exists so tracing of handler in webserver shows nice class name and not a lambda - private static final class TracingConfigHandler implements Handler { - private final TracingConfig pathSpecific; - - private TracingConfigHandler(TracingConfig pathSpecific) { - this.pathSpecific = pathSpecific; - } - - @Override - public void handle(ServerRequest req, ServerResponse res) { - Optional existing = req.context().get(TracingConfig.class); - if (existing.isPresent()) { - req.context().register(TracingConfig.merge(existing.get(), pathSpecific)); - } else { - req.context().register(pathSpecific); - } - res.next(); - } - } - - static final class RequestSpanHandler implements Handler { - private static final String TRACING_SPAN_HTTP_REQUEST = "HTTP Request"; - private final AtomicBoolean checkedIfShouldTrace = new AtomicBoolean(); - private final Tracer tracer; - private volatile boolean shouldTrace = true; - - RequestSpanHandler(Tracer tracer) { - this.tracer = tracer; - } - - @Override - public void handle(ServerRequest req, ServerResponse res) { - if (shouldTrace && checkedIfShouldTrace.compareAndSet(false, true)) { - shouldTrace = tracer().enabled(); - } - - if (shouldTrace) { - doAccept(req, res); - } - - res.next(); - } - - private void doAccept(ServerRequest req, ServerResponse res) { - // must run in context - Context context = req.context(); - - SpanTracingConfig spanConfig = TracingConfigUtil - .spanConfig(WebServer.TRACING_COMPONENT, TRACING_SPAN_HTTP_REQUEST, context); - - SpanContext inboundSpanContext = tracer.extract(new TracingHeaderProvider(req.headers().toMap())) - .orElse(null); - - if (inboundSpanContext != null) { - // register as parent span - context.register(inboundSpanContext); - context.register(TracingConfig.class, inboundSpanContext); - } - - if (!spanConfig.enabled()) { - return; - } - - String spanName = spanConfig.newName().orElse(TRACING_SPAN_HTTP_REQUEST); - if (spanName.indexOf('%') > -1) { - spanName = String.format(spanName, req.method().text(), req.path(), req.query()); - } - // tracing is enabled, so we replace the parent span with web server parent span - Span.Builder spanBuilder = tracer.spanBuilder(spanName) - .kind(Span.Kind.SERVER) - .tag(Tag.COMPONENT.create("helidon-reactive-webserver")) - .tag(Tag.HTTP_METHOD.create(req.method().text())) - .tag(Tag.HTTP_URL.create(req.uri().toString())) - .tag(Tag.HTTP_VERSION.create(req.version().value())); - - if (inboundSpanContext != null) { - spanBuilder.parent(inboundSpanContext); - } - - // cannot use startActive, as it conflicts with the thread model we use - Span span = spanBuilder.start(); - - context.register(span.context()); - context.register(TracingConfig.class, span.context()); - - res.whenSent() - .thenRun(() -> { - Http.Status httpStatus = res.status(); - if (httpStatus != null) { - int statusCode = httpStatus.code(); - span.tag(Tag.HTTP_STATUS.create(statusCode)); - - if (statusCode >= 400) { - span.status(Span.Status.ERROR); - - span.addEvent("error", Map.of("message", "Response HTTP status: " + statusCode, - "error.kind", statusCode < 500 ? "ClientError" : "ServerError")); - } - } - span.end(); - }) - .exceptionally(t -> { - span.end(t); - return null; - }); - } - - private static class TracingHeaderProvider implements HeaderProvider { - - private final Map> headers; - - TracingHeaderProvider(Map> headers) { - this.headers = headers; - } - - @Override - public Iterable keys() { - return headers.keySet(); - } - - @Override - public Optional get(String key) { - List strings = headers.get(key); - if (strings == null) { - return Optional.empty(); - } - return Optional.of(strings.get(0)); - } - - @Override - public Iterable getAll(String key) { - List strings = headers.get(key); - if (strings == null) { - return List.of(); - } - return strings; - } - - @Override - public boolean contains(String key) { - return headers.containsKey(key); - } - } - } -} diff --git a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1ServerResponse.java b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1ServerResponse.java index 62d180be4f4..0a58a02f23b 100644 --- a/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1ServerResponse.java +++ b/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1ServerResponse.java @@ -312,14 +312,16 @@ public void close() { } if (isChunked || forcedChunked) { - // not optimized, we need to write trailers - trailers.set(STREAM_STATUS_NAME, String.valueOf(status.get().code())); - trailers.set(STREAM_RESULT_NAME, streamResult.get()); - BufferData buffer = BufferData.growing(128); - writeHeaders(trailers, buffer); - buffer.write('\r'); // "\r\n" - empty line after headers - buffer.write('\n'); - dataWriter.write(buffer); + if (request.headers().contains(HeaderValues.TE_TRAILERS)) { + // not optimized, trailers enabled: we need to write trailers + trailers.set(STREAM_STATUS_NAME, String.valueOf(status.get().code())); + trailers.set(STREAM_RESULT_NAME, streamResult.get()); + BufferData buffer = BufferData.growing(128); + writeHeaders(trailers, buffer); + buffer.write('\r'); // "\r\n" - empty line after headers + buffer.write('\n'); + dataWriter.write(buffer); + } } responseCloseRunnable.run(); @@ -365,8 +367,10 @@ private void write(BufferData buffer) throws IOException { } if (firstByte) { - // proper stream with multiple buffers, write status amd headers - headers.add(STREAM_TRAILERS); + if (request.headers().contains(HeaderValues.TE_TRAILERS)) { + // proper stream with multiple buffers, write status amd headers + headers.add(STREAM_TRAILERS); + } sendHeadersAndPrepare(); firstByte = false; BufferData combined = BufferData.create(firstBuffer, buffer);