ZopaPublic / ktor-opentracing

Ktor features for OpenTracing instrumentation.
MIT License
37 stars 8 forks source link

Possible problem with `StatusPages` feature and error responses ? #37

Open legzo opened 3 years ago

legzo commented 3 years ago

Hi,

I'm using version 0.3.6 and everything works correctly for responses that are not in error. However, when I have an exception on my backend, no root span is created by the OpenTracingServer feature. It seems to be linked with the use of the StatusPages feature.

For ex. if I have the StatusPages feature installed with this configuration :

install(StatusPages) {
    exception<Throwable> { cause ->
        call.respond(HttpStatusCode.InternalServerError)
    }
}

and throw an exception in a get mapping :

get("test") {
    throw IllegalArgumentException()
}

-> then no span is created.

And if I uninstall the StatusPages feature and modify my mapping to do :

get("test") {
    call.respond(HttpStatusCode.InternalServerError)
}

-> then a span is created as expected.

I think it is linked to phases but I know very little on the topic so....

Thanks

legzo commented 3 years ago

Ok, I took some time to try to understand what is going on. I started fresh from a new projet with the ktor official bootstrap, adding only the StatusPages plugin. What caught my attention is that the StatusPages is installed inside the routing { } closure and not outside. In my projet it is outside.

I then tried both configurations and the /exception endpoint logs a span only if the install(StatusPages) in inside routing { }. If outside, only the / and /error log some spans.

Working configuration :

    routing {

        install(StatusPages) {
            exception<AuthorizationException> { cause ->
                call.respond(HttpStatusCode.Forbidden)
            }
        }

        get("/") {
            call.respondText("Hello World!")
        }

        get("/error") {
            call.respond(HttpStatusCode.InternalServerError)
        }

        get("/exception") {
            throw AuthorizationException()
        }
    }

Not working one :

    install(StatusPages) {     
        exception<AuthorizationException> { cause ->
            call.respond(HttpStatusCode.Forbidden)
        }
    }

    routing {

        get("/") {
            call.respondText("Hello World!")
        }

        get("/error") {
            call.respond(HttpStatusCode.InternalServerError)
        }

        get("/exception") {
            throw AuthorizationException()
        }
    }

I don't know how the internals of ktor work. Maybe it is expected to work this way...

It is strange because in the docs we can find the StatusPages outside the routing and in the generated bootstrap projet from https://start.ktor.io/ is installing StatusPages inside the routing... ¯\_(ツ)_/¯