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

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

Open
robert3005 opened this issue Sep 2, 2019 · 7 comments

Comments

@robert3005
Copy link
Contributor

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
Copy link
Contributor

Maybe related to #416?

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

@robert3005
Copy link
Contributor Author

I need to be able to register an ExchangeCompletionListener

@robert3005
Copy link
Contributor Author

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
Copy link
Contributor

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

@robert3005
Copy link
Contributor Author

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

@carterkozak
Copy link
Contributor

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants