cmd/scollector collector: local listener #1435
Conversation
|
I'm curious what role this plays for you. Have you seen |
|
It's to simplify deployment of application producing OpenTSDB metrics. The application sends its data to a local scollector (which we need anyway to collect system metric) which adds our usual default tags relating to the host. I see it as a http equivalent to tcollector's udp_bridge.py. |
|
Very interesting. What language is your app in? |
|
Some in Java, some in Go. |
|
In most deployments I have seen, apps will send metrics directly to bosun. In go you can load the collect package directly, or https://bitbucket.org/oneoffcode/go-metrics-bosun/src. I haven't seen a java library, but one would be really useful. Not saying we won't accept this, but making sure to look at alternative approaches. |
|
The benefit of this imo is that the application doesn't need to know anything about its deployment. No need to find out what's its hostname, cluster, product, etc. It's all added by scollector which is in charge of topology related tags. As for producing libraries looks like we had the same idea on the Go side (https://github.com/mathpl/go-tsdmetrics). For Java our library is tightly coupled with our internal framework currently so we can't open source it easily for now. |
| "bosun.org/opentsdb" | ||
| ) | ||
|
|
||
| type StreamCollector struct { |
There was a problem hiding this comment.
I'm not sure the stream collector is really needed here. Why not just have the listener relay data points to the collect package directly. That channel is made here: https://github.com/bosun-monitor/bosun/blob/master/cmd/scollector/main.go#L191. I would suggest putting the http listener in the main package and relaying directly to collect.
There was a problem hiding this comment.
The main reason for it is to be able to generate scollector.collector.count for this collector.
There was a problem hiding this comment.
You could just do collect.Add("localListener.relayed", tags, len(mdp)) or similar in your relay function as you see datapoints.
There was a problem hiding this comment.
I felt if any other future collectors would follow the same pattern having a StreamCollector in addition of the IntervalCollector would be beneficial. I'll gladly remove it and attach the collector in main.go if you wish though.
There was a problem hiding this comment.
I suppose at least add a good comment here explaining the purpose of StreamCollector, and when I would use it.
mathpl
force-pushed
the
feature/local_listener
branch
from
58d67c5
to
7c89e48
Compare
| for { | ||
| next := time.After(time.Minute * 5) | ||
| s.Lock() | ||
| s.enabled = s.Enable() |
There was a problem hiding this comment.
is Enabled really a useful concept here? The interval collector uses it to determine "Is X installed on this system" so it can avoid issuing unecessary requests and such. If you real collector is running external to the Stream Collector, all you are doing with enabled is dropping datapoints. I would rather just always be enabled on these, and make the user "opt in" by enabling it in config.
There was a problem hiding this comment.
Agreed, I removed it.
mathpl
force-pushed
the
feature/local_listener
branch
from
7c89e48
to
1d31e9c
Compare
|
Can you fix conflicts? |
|
|
||
| type StreamCollector struct { | ||
| F func() <-chan *opentsdb.MultiDataPoint | ||
| Enable func() bool |
There was a problem hiding this comment.
remove Enable from the struct.
mathpl
force-pushed
the
feature/local_listener
branch
from
1d31e9c
to
03b4064
Compare
|
Been a while, anything else I can do? |
|
rebase on master and resolve conflicts please. |
mathpl
force-pushed
the
feature/local_listener
branch
from
03b4064
to
90c4a82
Compare
|
Done. Not quite sure what's wrong with TravisCI though. |
mathpl
force-pushed
the
feature/local_listener
branch
from
90c4a82
to
d5692ef
Compare
|
rebased again to get the build fixed. |
mathpl
force-pushed
the
feature/local_listener
branch
from
d5692ef
to
8f692fc
Compare
mathpl
force-pushed
the
feature/local_listener
branch
from
8f692fc
to
abedcc5
Compare
cmd/scollector collector: local listener
This adds the local_listener collector, it starts an http server locally and listens for /api/put and /api/metadata/put http requests. It ingest that data and forward it through its usual mechanism, basically acting as a proxy.