[SCB-422] prometheus switch to new mechanism #639
Conversation
wujimin
commented
Apr 8, 2018
wujimin
commented
wujimin
force-pushed
the
metrics-prometheus
branch
from
fced737
to
702e18d
Compare
| try { | ||
| InetSocketAddress socketAddress = getSocketAddress(address); | ||
| register(); | ||
| this.httpServer = new HTTPServer(socketAddress, CollectorRegistry.defaultRegistry, true); |
There was a problem hiding this comment.
For future customization, I think we 'd provide way to export to servicecomb rest framework. And contribute the code to https://github.com/prometheus/client_java. If we can create a task to tracking this?
There was a problem hiding this comment.
By MetricsRestPublisher in previous PR, we already support metrics endpoint: /metrics
and created a issue for contribute: https://issues.apache.org/jira/browse/SCB-466
| @@ -43,7 +42,10 @@ | |||
| <groupId>org.apache.servicecomb</groupId> | |||
| <artifactId>metrics-core</artifactId> | |||
| </dependency> | |||
|
|
|||
| <dependency> | |||
There was a problem hiding this comment.
Shouldn't the scope set to 'test' here?
There was a problem hiding this comment.
why test?
all servicecomb measurement come from metrics-core.
There was a problem hiding this comment.
I mean 'foundation-test-scaffolding'
There was a problem hiding this comment.
'foundation-test-scaffolding' already in 'test' scope, no need to set again.
|
Some files don't have proper final newline. |
| import com.netflix.spectator.api.ManualClock; | ||
| import com.netflix.spectator.api.Registry; | ||
| import com.netflix.spectator.api.SpectatorUtils; | ||
| import com.sun.net.httpserver.HttpServer; |
There was a problem hiding this comment.
Do we still need to use com.sun package ?
There was a problem hiding this comment.
prometheus depend on com.sun.net.httpserver.HttpServer
| Counter counter = registry.counter("count.name", "tag1", "tag1v", "tag2", "tag2v"); | ||
| counter.increment(); | ||
|
|
||
| HTTPServer httpServer = (HTTPServer) FieldUtils.readField(publisher, "httpServer", true); |
There was a problem hiding this comment.
Can we find another way to find the server published address?
There was a problem hiding this comment.
we can use port 0 to try listen, and got the real port, and then close it
use the found real port to init prometheus.
but so complex, no need to do this, just a test case. i think reflection is enough.
