Repository navigation
refactor: add metrics endpoint (DEV-1555) - #2331
Conversation
|
✅ Linked to Story DEV-1555 · Add prometheus metrics endpoint |
Codecov ReportBase: 86.68% // Head: 85.85% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #2331 +/- ##
==========================================
- Coverage 86.68% 85.85% -0.83%
==========================================
Files 250 238 -12
Lines 28252 27924 -328
==========================================
- Hits 24490 23975 -515
- Misses 3762 3949 +187
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
|
||
| override def run = for { | ||
| f1 <- PrometheusServer.make.forkDaemon | ||
| f2 <- (AppServer.live *> ZIO.never).forkDaemon |
There was a problem hiding this comment.
Is it possible to leave AppServer.live.launch instead of AppServer.live *> ZIO.never?
There was a problem hiding this comment.
.launch is a method on ZLayer. It does exactly the same.
BalduinLandolt
left a comment
BalduinLandolt
left a comment
There was a problem hiding this comment.
Looks good code-wise, I would like to have seen some documentation on it
|
|
||
| object PrometheusServer { | ||
|
|
||
| private val nThreads = 5 |
There was a problem hiding this comment.
why is this not coming from the config?
There was a problem hiding this comment.
I'm not sure what it does exactly. I still need to figure that out first. Maybe we don't need it.
mpro7
left a comment
mpro7
left a comment
There was a problem hiding this comment.
Looks good. Feel free to apply my suggestion. Shouldn't PrometheusServer has some unit tests written?
| f1 <- PrometheusServer.make.forkDaemon | ||
| f2 <- (AppServer.live *> ZIO.never).forkDaemon | ||
| _ <- f1.join | ||
| _ <- f2.join |
There was a problem hiding this comment.
| f1 <- PrometheusServer.make.forkDaemon | |
| f2 <- (AppServer.live *> ZIO.never).forkDaemon | |
| _ <- f1.join | |
| _ <- f2.join | |
| _ <- PrometheusServer.make.forkDaemon.join | |
| _ <- (AppServer.live *> ZIO.never).forkDaemon.join |
How about join in the same lines?
|
As discussed. I would prefer to have all zio http server definitions in the core package side by side. I do think it will make sense to split the application and instrumentation http endpoint to be on separate port, hence the new Server should be named Please also move the health route to the instrumentation server. |
…ch-swiss/dsp-api into wip/DEV-1555-add-prometheus-endpoint
subotic
changed the title
Docs added. |
done. |
subotic
changed the title
5 participants
Pull Request Checklist
Task Description/Number
Issue Number: DEV-1555
Basic Requirements
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Does this PR change client-test-data?