Correct HTTP Push end point #101
Conversation
…n /job/. This commit attempt to address this Signed-off-by: Viren Negi <meetme2meat@gmail.com>
| PATH = '/metrics/jobs/%s'.freeze | ||
| INSTANCE_PATH = '/metrics/jobs/%s/instances/%s'.freeze | ||
| PATH = '/metrics/job/%s'.freeze | ||
| INSTANCE_PATH = '/metrics/job/%s/instances/%s'.freeze |
There was a problem hiding this comment.
@beorn7 I can change that but I do seem to find the anything mentioned in the README.md regarding the instances thing?
| end | ||
|
|
||
| it 'uses the full metrics path if an instance value is given' do | ||
| push = Prometheus::Client::Push.new('bar-job', 'foo') | ||
|
|
||
| expect(push.path).to eql('/metrics/jobs/bar-job/instances/foo') | ||
| expect(push.path).to eql('/metrics/job/bar-job/instances/foo') |
| end | ||
|
|
||
| it 'escapes non-URL characters' do | ||
| push = Prometheus::Client::Push.new('bar job', 'foo <my instance>') | ||
|
|
||
| expected = '/metrics/jobs/bar%20job/instances/foo%20%3Cmy%20instance%3E' | ||
| expected = '/metrics/job/bar%20job/instances/foo%20%3Cmy%20instance%3E' |
|
Shouldn’t it be instance (singular) not instances ? See https://github.com/prometheus/pushgateway/pull/227/files |
|
It can be anything, but with the current endpoint, if you use Thus, if you still want to push an instance tag (which is not recommended in general), you need to use |
|
@beorn7 Yes, naming it as instance would help in the scenario where we are interested in knowing the instance from where the metric was pushed, else Prometheus db would have On the other note, the honour_labels options which I planning to use on prometheus_scarper will fail if it stays as instances. @beorn7 I can make the change. If my appeal sounds reasonable to you. |
|
The minimum fix of client_ruby is to do s/jobs/job/ and s/instances/instance/ without changing the function signature. Deprecation of pushing with an instance label can be done separately. @grobie is the maintainer and has to shepherd this. |
|
I talked to @grobie and @stuartnelson3 and created #102 with the suggested changes. Thus, this has been superseded. My apologies for the slow processing. We should have done a better job as maintainers. |
As of 0.7.0 the HTTP push endpoint has been changed from /jobs/ -> /job/.
This pull request attempt to correct that.