Change default operation name and make it overridable

[dimitrijer]

Operation name for handler spans contains only method name, like GET or POST. This makes trace analysis quite difficult. I changed default operation name to include the path as well: tracing handler on /somePath route will now generate a span with operation name HTTP GET /somePath. I also made it overridable from children classes.

Also changed attribute map key from severMap -> serverMap.

[pavolloffay]

@dimitrijer hi, thanks for the contribution.

Using HTTP paths for operation names isn't a good default value. There is a problem with wildcards e.g. for endpoint /users/:id there would two spans with operation names users/1 and users/2 representing the same operation. For more see https://github.com/opentracing/specification/blob/master/specification.md#tracer

The best solution is to set the operation name to wildcard or http verb and wildcard, however I am not sure if we can get that from the routing context.

This instrumentation provides also span decorator which allows you to change operation name to any value.

[dimitrijer]

Hi, @pavolloffay!

Thanks for quick review. I can see why including raw path in operation name is a bad idea. I guess I'll provide wildcard paths as operation names via decorators then.

Is there any danger of span being finished before my decorator changes the operation name (before decorator's onRequest is executed)?

[pavolloffay]

If you can manage to implement a decorator or even directly set the operation name to wildcard in tracing handler that would be awesome!

Is there any danger of span being finished before my decorator changes the operation name (before decorator's onRequest is executed)?

The span shouldn't be finished when calling https://github.com/opentracing-contrib/java-vertx-web/blob/master/opentracing-vertx-web/src/main/java/io/opentracing/contrib/vertx/ext/web/WebSpanDecorator.java#L24

[dimitrijer]

Also, do you think it would be a good idea to pass RoutingContext to decorators instead of routingContext.request()?

[pavolloffay]

There is a little danger when passing routing context e.g. one could call next.

[dimitrijer]

Yeah, that makes sense.

I tried extracting wildcard route with routingContext.currentRoute().getPath(). The problem is that at the time we set context path, either directly or with decorators, no route is matched yet, meaning that routingContext.currentRoute().getPath() will return null. It works fine when actual route is matched, that is, call to that method returns wildcard path when invoked from handler body.

Hm, I'll just make some hacky solution for my use case at this time. If I figure out how to do it properly, I'll let you know here.

Again, thanks for the patience (first time doing OpenTracing here) and thanks for being so responsive.