the collector register should probably happen in an init
in any case, I don't really know why this blows up in tests but is fine in production other maybe it's a dependency issue then or something else is different between test and production
What happens if you don't use the exporter trait and do what it's doing manually? E.g....
class Main extends HttpServer {
init {
PrometheusMetricsCollector().register()
}
override val modules: Seq[TwitterModule] = Seq(DbConnectionProviderModule)
override def configureHttp(router: HttpRouter): Unit = {
router
.filter[LoggingMDCFilter[Request, Response]]
.filter[CommonFilters]
.filter[CorsFilter]
.exceptionMapper[EntityNotFoundExceptionMapper]
.exceptionMapper[MappingExistsMapper]
.exceptionMapper[OverCapacityExceptionMapper]
.exceptionMapper[DuplicateOccurrenceExceptionMapper]
.add[OccurrenceController]
.add[ZoneController]
}
premain {
addAdminRoute(PrometheusExporter.metricsRoute)
}
override protected def warmup(): Unit = {
handle[Warmup]
}
override protected def setup(): Unit = {
// Run migrations
val conf: Config = ConfigFactory.load()
val dbConfig: Config = conf.getConfig("gregor.db")
Utils
.buildMigrateConf(dbConfig)
.locations("com.armoredthings.gregor.database.migration")
.load()
.migrate()
super.setup()
}
}
Ok found an example where it was a requirement in testing to "drop all" the metrics from the sink: https://github.com/twitter/finatra/blob/1b34404068f7f87e062dcdd0d3d2af36bbe04727/examples/hello-world-heroku/src/test/scala/com/twitter/hello/heroku/HelloWorldFeatureTest.scala#L14. Not sure if there is an equivalent here.
@cacoco looked into this a little bit more.
1) The error is directly coming from Prometheus.register() that is inside the trait.
2) The lifecycle approach doesn't work either, however is there a way to override the lifecycle hooks? If I was able to do that, I can init prometheus on premain in the application but override it in the EmbeddedHttpServer. Wasn't able to locate a way of doing this tho
3) On that note
import com.twitter.finagle.metrics._seems to be deprecated. I was able to locate MetricsStatsReceiver in
com.twitter.finagle.stats.MetricsStatsReceiverbut it doesn't seem to expose those .metrics. Is there a corresponding version of this in the most recent finagle ?
@sinanspd you don’t override lifecycle methods in the embedded http sever it is a wrapper over a server under test. You do it in the instance your testing. The lifecycle approach is more correct than the example given and should have the same affect but with the benefit of actually being able to use flags if necessary since their values are parsed in between init and Premain. Lastly the example was someone else integration and not Finagle and Finagle doesn’t expose the same thing for reasons.
Looking at the error you need a way to clear metrics but I don’t know how much help we’re going to be since we’re not Prometheus experts. I saw you flagged a ticket for them feel free to add me to it
gotcha. I was trying to see if we can get to the bottom of this on the finatra side rather than prometheus. Thanks for the insight. Something like this works as far as I can see,
premain {
s.get match {
case None =>
PrometheusMetricsCollector().register()
addAdminRoute(Route.isolate(Route(
path = "/metrics",
handler = new PrometheusMetricsExporterService(),
alias = "Prometheus Metrics",
group = Some(Grouping.Metrics),
includeInIndex = true
)
))
case Some("prod") =>
PrometheusMetricsCollector().register()
addAdminRoute(Route.isolate(Route(
path = "/metrics",
handler = new PrometheusMetricsExporterService(),
alias = "Prometheus Metrics",
group = Some(Grouping.Metrics),
includeInIndex = true
)
))
case Some(r) => r
}
}
I think the problem is them using the default registry, I will try to write up a PR to specify registries in the library and see if they would be willing to merge that
thanks for all the help
Referencing bug: twitter/finatra#514
Migrated from finatra-19.7.0 to 19.11.0, now the Controller will not resolve endpoints with parameters, like /api/hello/:status/
just to set expectations appropriately. this is probably related to us changing our routing backend, thanks for the reproduction case!
And this should be later
@cacoco Understood. My comment to the ticket is more specific to the issue I'm referring to (https://github.com/twitter/finatra/issues/514#issuecomment-559194608), which is the param not being picked up. Thought it'd be more beneficial to piggy back off the related ticket.
I am dubious of this being an issue as we have plenty of tests around that case and many internal usages so am wondering if there is something else awry. What version were you upgrading from since you confirmed you saw the behavior in 19.7.0? That is what is the version that worked for you?
Hmm, you're doing this inside of a prefix, perhaps that is adding some wrinkle. We'll have to diagnose.
It would be great if you didn't piggyback on this ticket as it looks like you are experiencing something different and it would be great if you opened a separate ticket.
Was using 19.7.0 for several months, the routing was working as expected; I am also utilizing the prefixes. I just upgraded to 19.11.0 on @jyanJing 's recommendation to get the streaming updates:
Hi @mehulmistryavlino , unfortunately there is no resource management for AsyncStream, afaik, there is no interface to disconnect an ongoing AsyncStream. This is why we have rolled out a new version of streaming, which is backed by Reader instead of AsyncStream. We recommend you to switch to the newer version to solve this particular problem. It is very user friendly, you can still keep the AsyncStream but use the
Reader.fromAsyncStreaminterface to create a Reader, then pass the Reader to create aStreamingResponse, and you can usereader.discard()to close streaming connection through the Reader. Here is the user guide: https://docbird.twitter.biz/finatra/user-guide/http/streaming.html. Please let me know if you have any questions!
Now all of the endpoints containing params are not being found.
I'll open a separate ticket as well
Also again I am surprised that no params work in your prefixed routes as you can search the tests for these cases...so adding as much context to the issue as possible will be helpful.
Hello. I wonder what's the standard shutdown operations of finatra, because I'm getting
NoClassDefFoundError on c/t/util/Promise$ stack trace if I double Ctrl-c in sbt run and this won't happen if I shutdown from admin console on 9990 port. Is this normal behavior? My main object extends HttpServer. Do I have to write something on onExit{} in my class or something?
Thank you, @sinanspd. I don't have any user implemented async process or DB connections. I think I should check my code, starting from build.sbt. What should it look like that minimum build.sbt setup for
examples/http-server? The repo's build.sbt seems too big for my simple project...
Thank you, again. Since I want to follow the test in practice of finatra, I want to also add test dependencies as well, in order to run those test.
Mine is:
Mine is:
name := "inu"
version := "0.1"
scalaVersion := "2.12.8"
libraryDependencies ++= Seq(
"com.twitter" %% "finatra-http" % "19.10.0",
"com.twitter" %% "finatra-http" % "19.10.0" % Test classifier "tests",
"com.twitter" %% "inject-server" % "19.10.0" % Test classifier "tests",
"com.twitter" %% "inject-app" % "19.10.0" % Test classifier "tests",
"com.twitter" %% "inject-core" % "19.10.0" % Test classifier "tests",
"com.twitter" %% "inject-modules" % "19.10.0" % Test classifier "tests",
"org.scalatest" %% "scalatest" % "3.0.0" % Test,
"com.google.inject" % "guice" % "4.2.2",
"com.google.inject.extensions" % "guice-testlib" % "4.2.2" % Test classifier "tests",
"ch.qos.logback" % "logback-classic" % "1.1.7",
)
Hi there. We currently have our Server extend KafkaStreamsTwitterServer and mix in HttpServer. I have noticed that if any of our kafka streams methods are long-running (even if they aren't blocking) that HTTP request are not being serviced. I have tried wrapping the underlying functionality in a FuturePool but still seeing the same behavior. Is there a way to ensure that HTTP requests are serviced by separate threads than the kafka streams use?
FuturePool should ensure this. You are likely orphaning the work, however. If you submit it to a pool then do nothing with the resultant Future, then your main thread continues and can exit before the work you put in the future pool has finished. You generally want to map that future to your main request
I have tried in different test on different machine, I found actually there are several patterns of stack traces about
Promise, Hotspot and so on when I use C-c C-c in sbt especially after been kept the app up for quite a while (like 3 hours). However, these never come out with admin console shutdown. I decided move forward about my project rather than identifying the cause. I will raise issues once it comes even with admin console shutdown. Thank you for your help.
@jdcohen220 you get a future back when you submit something to a FuturePool, you need to do something with it. I can’t quite tell exactly what you’re trying to do in your case so it’s hard to give great guidance. As @sinanspd pointed out, it’s a bit odd to mix these concerns in this way though I could see you wanting to expose an Http API over some Kakfa work. If it’s the case where the http request is triggering the kafkae work, then it could be as simple as mapping the Future returned from submitting the kafka work to the pool to an HTTP response.
futurepool.submit(work).map(_ => http 201 response)
that would be the return of your http callback in your controller for instance
@kazuf3 running an app from the sbt console will likley always be odd and highly dependent on the sbt configuration you’re using (e.g., are you forking in run, etc). generally we recommend starting your server simply by running the resultant jar. the server can be gracefully shutdown by hitting the /admin/quitquitquit endpoint. killing the process (either manually or through sbt) would not be graceful and all bets are off then as to what errors you may get as it is dependent on what the server is doing when killed.
you can take a look at the older examples for info: https://github.com/twitter/finatra/tree/finatra-2.2.0/examples/hello-world