Armeria: BraveService should be public non final

[liemle3893]

Armeria version: 0.92.0

Problems:

• There're times we want to exclude some path. Eg: /healthz, /metrics, etc...
• I am new to Armeria so I don't know is there any thing like dynamic decorator (I guest not)

Work around:

• Create com.linecorp.armeria.server.brave inside my code and rewrite BraveService

Solution:

• Open BraveService to public
• Has some kind of optional filter/predicate for each Service. (I want to exclude access log too) This is not what I really want, currently Decorator is doing good in this part

[trustin]

That's a good point. As a workaround, you could define a decorator that wraps a BraveService:

public class MyDecorator extends SimpleDecoratingHttpService {

    private final BraveService braveService;

    public MyDecorator(Service<HttpService, HttpService> delegate) {
        super(delegate);
        braveService = BraveService.newDecorator(...).apply(delegate);
    }

    @Override
    public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) {
        if (req.path().startsWith("/healthz")) {
            return delegate().serve(ctx, req);
        } else {
            return braveService.serve(ctx, req);
        }
    }
}

However, I'm not sure if it's a good idea to open BraveService, because we prefer composition over inheritance in many cases. What do you think about providing a way to exclude a certain path pattern from a route instead? i.e. It'd be nice if we have an API like the following:

ServerBuilder sb = Server.builder();
sb.routeDecorator()
  .pathUnder("/")
  .exceptUnder("/healthz")
  .exceptUnder("/metrics")
  .except("/foo/:bar")
  ...
  .build(myDecorator);

/cc @ikhoon

[anuraaga]

Being able to express negative path expressions seems useful (though a little concerned about performance). For this use case, though, I think it's better to use Brave's native functionality. You can customize the sampling policy of HttpTracing to ignore paths. Actually zipkin-server itself does just that

https://github.com/openzipkin/zipkin/blob/master/zipkin-server/src/main/java/zipkin2/server/internal/brave/ZipkinSelfTracingConfiguration.java#L120

[ikhoon]

What do you think about providing a way to exclude a certain path pattern from a route instead?

Excepting specific paths from decorating the path looks useful.👍 But in this case, I lean toward customizing HttpTracing suggested by @anuraaga 😀

I'm not sure if it's a good idea to open BraveService, because we prefer composition over inheritance in many cases.

I agree with your opinion. I prefer composition or configuration for extending or customizing a feature to inheritance.

[liemle3893]

Thank you for your quick relies.

So, for the Zipkin/Tracing I think I better custom HttpTracing. Thank @anuraaga.
And other use cases, I may use composition pattern like @trustin just mention above.
Thank you.