Support multi-target #143
Support multi-target #143
Conversation
mateusduboli
force-pushed
the
add-multi-target
branch
2 times, most recently
from
9a61dc0
to
3654d56
Compare
|
@roidelapluie can you help us with this PR? |
|
Apologies for the radio silence @mateusduboli. @SuperQ @roidelapluie So I see that the project's stance on multi-targets has changed? I found @SuperQ's comment from February suggesting exactly this change #111 (comment). The change looks good to me in general. I believe users should explicitly opt-in into the new behavior though by setting a flag. It might be unexpected and even harmful for users upgrading to the newest version if the exporter can be suddenly used to make requests against arbitrary endpoints. |
|
Yes, we've been adding multi-target exporter scrape support across the ecosystem to better support hosted/blackbox managed instances like Elasticache. |
|
I don't think we've added any blockers / flags to any other exporter that disable multi-target by default. We typically recommend users that care implement auth/tls. |
There was a problem hiding this comment.
Found a recurring typo: s/scrapp/scrap/ 🙂
scrape / scraping: copy (data) from a website using a computer program.
scrap / scrapping: discard or remove from service (a retired, old, or inoperative vehicle, vessel, or machine), especially so as to convert it to scrap metal.
|
@SuperQ @maxbrunet |
Signed-off-by: Mateus Dubiela Oliveira <mateus.dubiela@nubank.com.br>
mateusduboli
force-pushed
the
add-multi-target
branch
from
e2a0a12
to
7b73e3d
Compare
|
@SuperQ done ✅ |
|
Can this get merged and released? It is a super-useful feature, and it looks like everything that was asked for has been done. |
| logger: logger, | ||
| timeout: timeout, | ||
| scrapeCount: prometheus.NewCounter(prometheus.CounterOpts{ | ||
| Name: "memcached_exporter_scrape_count", |
There was a problem hiding this comment.
| Name: "memcached_exporter_scrape_count", | |
| Name: "memcached_exporter_scrapes_total", |
| Help: "Count of memcached exporter scapes.", | ||
| }), | ||
| scrapeErrors: prometheus.NewCounter(prometheus.CounterOpts{ | ||
| Name: "memcached_exporter_scrape_error_count", |
There was a problem hiding this comment.
| Name: "memcached_exporter_scrape_error_count", | |
| Name: "memcached_exporter_scrape_errors_total", |
|
This feature seems helpful, but it has been open for several months without recent activity or approval. Is there a specific reason for the delay? Could someone help resolve any conflicts and get this merged? |
|
We wrapped this up in #173. |
|
Completed in #173 |
|
Thank you for your contribution! |
Adds support for multi-target pattern. Refers to #130
Discussion points:
scrapperby default? Is this possibly a security problem?