palantir / conjure-java

Conjure generator for Java clients and servers
Apache License 2.0
27 stars 43 forks source link

Extension point for endpoint specific handler behaviour in conjure-undertow #527

Open robert3005 opened 5 years ago

robert3005 commented 5 years ago

What happened?

I had forked generated undertow handlers to add metrics dependent on request body.

Currently you can modify your service by wrapping your service definition or wrapping the endpoint. However, there's no way to create a plugin that can use both of them without forking generated undertow handlers.

The concrete case was creating a server response time metric that has tag values that are request body specific. This is a simpler version of the code modifications I had to make to generated service endpoints. Naturally this gets pretty cumbersome with many endpoints.

public final class SomeServiceEndpoints implements UndertowService {
    private final DatasetSizeThresholds sizeThresholds;
    private final MetricsSink metrics;
    private final SomeService delegate;

    private SomeServiceEndpoints(
            DatasetSizeThresholds sizeThresholds, MetricsSink metrics, SomeService delegate) {
        this.sizeThresholds = sizeThresholds;
        this.metrics = metrics;
        this.delegate = delegate;
    }

    public static UndertowService of(
            DatasetSizeThresholds sizeThresholds,
            MetricsSink metrics,
            SomeService delegate) {
        return new SomeServiceEndpoints(sizeThresholds, metrics, delegate);
    }

    @Override
    public List<Endpoint> endpoints(UndertowRuntime runtime) {
        return Collections.unmodifiableList(
                Arrays.asList(
                        new GetPreviewEndpoint(runtime, delegate, sizeThresholds, metrics)));
    }

    private static final class GetPreviewEndpoint
            implements HttpHandler, Endpoint, ReturnValueWriter<BinaryResponseBody> {
        private final UndertowRuntime runtime;
        private final SomeService delegate;
        private final Deserializer<PreviewDataRequest> deserializer;
        private final DatasetSizeThresholds sizeThresholds;
        private final MetricsSink metrics;

        GetPreviewEndpoint(
                UndertowRuntime runtime,
                SomeService delegate,
                DatasetSizeThresholds sizeThresholds,
                MetricsSink metrics) {
            this.runtime = runtime;
            this.delegate = delegate;
            this.deserializer =
                    runtime.bodySerDe().deserializer(new TypeMarker<PreviewDataRequest>() {});
            this.sizeThresholds = sizeThresholds;
            this.metrics = metrics;
        }

        @Override
        public void handleRequest(HttpServerExchange exchange) throws IOException {
            AuthHeader authHeader = runtime.auth().header(exchange);
            PreviewDataRequest previewRequest = deserializer.deserialize(exchange);
            exchange.addExchangeCompleteListener(buildExchangeCompletionListener(
                    metrics,
                    () -> sizeThresholds.getDatasetSize(
                            authHeader,
                            previewRequest)
                            .sizeInBytes()));
            ListenableFuture<BinaryResponseBody> result =
                    delegate.getPreview(authHeader, previewRequest);
            runtime.async().register(result, this, exchange);
        }

        @Override
        public void write(BinaryResponseBody result, HttpServerExchange exchange)
                throws IOException {
            runtime.bodySerDe().serialize(result, exchange);
        }

        @Override
        public HttpString method() {
            return Methods.POST;
        }

        @Override
        public String template() {
            return "/someservice/arrow";
        }

        @Override
        public String serviceName() {
            return "SomeService";
        }

        @Override
        public String name() {
            return "getPreview";
        }

        @Override
        public HttpHandler handler() {
            return this;
        }
    }

    private static void recordMetrics(
            MetricsSink metrics, HttpServerExchange completeExchange, long inputDatasetSize) {
        metrics.status(completeExchange.getStatusCode(), inputDatasetSize);
        metrics.durationNanoseconds(System.nanoTime() - completeExchange.getRequestStartTime(), inputDatasetSize);
    }

    private static ExchangeCompletionListener buildExchangeCompletionListener(
            MetricsSink metrics, Supplier<Optional<Long>> sizeProvider) {
        return (exchange, nextListener) -> {
            try {
                sizeProvider.get().ifPresent(datasetSize -> recordMetrics(metrics, exchange, datasetSize));
            } finally {
                nextListener.proceed();
            }
        };
    }
}

What did you want to happen?

It should be possible to write a an extension to generated handlers that can access both request objects and the undertow exchange.

markelliot commented 5 years ago

Maybe related to #416?

In this instance, is the lone thing you need from the exchange the status code?

robert3005 commented 5 years ago

I need to be able to register an ExchangeCompletionListener

robert3005 commented 5 years ago

Sorry, the complicated bit of this logic is being able to get access to request body. Since if I were to do it before the default handler then it would fail deserializing the input.

markelliot commented 5 years ago

Specifically to get the size of the incoming request or something else?

robert3005 commented 5 years ago

DatasetSizeThresholds makes an external call to fetch size of the data that the request is about.

carterkozak commented 5 years ago

I'm not sure I understand why this cannot be implemented using the async ListenableFuture instead. We can count bytes written to a BinaryResponseBody and report success or failure using a future callback without reaching into the framework implementation. There may be something I'm missing though.

robert3005 commented 5 years ago

I am not interested in the byte size of the body but getting the body and using it to make a call to external service and then adding that to an exchange completion listener. I think I could wrap the async listenablefuture but I wanted to mimic the metric we used internally which uses the exchange completion time