api/prometheus/v1/api.go: Add support for status/runtimeinfo endpoint #755

lilic · 2020-05-18T18:15:45Z

Adds support for the status/runtimeinfo API endpoint as per the API docs https://prometheus.io/docs/prometheus/latest/querying/api/#runtime-information

Closes #746

cc @beorn7

Signed-off-by: Lili Cosic <cosiclili@gmail.com>

simonpasquier

LGTM

bwplotka

Thanks. Looks good, just minor nits.

Also question about status code... not sure why we discard this!

bwplotka · 2020-05-19T09:47:32Z

api/prometheus/v1/api.go

@@ -238,6 +239,8 @@ type API interface {
 	Query(ctx context.Context, query string, ts time.Time) (model.Value, Warnings, error)
 	// QueryRange performs a query for the given range.
 	QueryRange(ctx context.Context, query string, r Range) (model.Value, Warnings, error)
+	// Runtimeinfo returns the various runtime information properties about the Prometheus server.


Suggested change

// Runtimeinfo returns the various runtime information properties about the Prometheus server.

// RuntimeInfo returns the various runtime information properties about the Prometheus server.

I disagree with this, as the endpoint is /runtimeinfo not /runtime_info, at least that was the logic I used here. :)

Interesting, not a blocker, but this looks weird (:

bwplotka · 2020-05-19T09:47:54Z

api/prometheus/v1/api.go

@@ -238,6 +239,8 @@ type API interface {
 	Query(ctx context.Context, query string, ts time.Time) (model.Value, Warnings, error)
 	// QueryRange performs a query for the given range.
 	QueryRange(ctx context.Context, query string, r Range) (model.Value, Warnings, error)
+	// Runtimeinfo returns the various runtime information properties about the Prometheus server.
+	Runtimeinfo(ctx context.Context) (RuntimeinfoResult, error)


Suggested change

Runtimeinfo(ctx context.Context) (RuntimeinfoResult, error)

RuntimeInfo(ctx context.Context) (RuntimeinfoResult, error)

Suggested change

Runtimeinfo(ctx context.Context) (RuntimeinfoResult, error)

Runtimeinfo(ctx context.Context) (RuntimeInfoResult, error)

bwplotka · 2020-05-19T09:49:17Z

api/prometheus/v1/api.go

@@ -277,6 +280,22 @@ type ConfigResult struct {
 // FlagsResult contains the result from querying the flag endpoint.
 type FlagsResult map[string]string

+// RuntimeinfoResult contains the result from querying the runtimeinfo endpoint.
+type RuntimeinfoResult struct {


bwplotka · 2020-05-19T09:53:01Z

api/prometheus/v1/api.go

+		return RuntimeinfoResult{}, err
+	}
+
+	_, body, _, err := h.client.Do(ctx, req)


It looks like matches the other methods, but why we discard the status code completely? No point in marshaling if we don't check 2** status. 🤔

Like you mentioned this just uses what the other functions use, but the custom .Do does check for status :) https://github.com/prometheus/client_golang/blob/master/api/prometheus/v1/api.go#L880

I think this is something to fix in follow up PRs if any, just it feels wrong, will add issue to follow up (:

Agreed, we should fix it for all the functions! If you open issue can you ping me on it, thanks!

brancz

Style seems aligned with all other functions.

lgtm 👍

lilic · 2020-06-03T08:10:22Z

@beorn7 @bwplotka PTAL, thanks! :)

beorn7 · 2020-06-03T11:17:46Z

Sorry for the waiting time, @lilic . I'm not really an expert of this HTTP client code. So I was just waiting for @bwplotka to signal if his nits have been addressed. And then I missed for how long nothing happened here. I now checked myself, and from my somewhat limited perspective, it looks fine to me. As @brancz seems happy, too, I'll merge this.

Thank you very much!

lilic · 2020-06-03T11:47:17Z

@beorn7 no problem, thanks! if @bwplotka is not happy I can fix it in a follow up as well! :)

bwplotka

LGTM,

Sorry @lilic you need to ping us, I totally forgot about it 🤗

bwplotka · 2020-06-03T11:53:25Z

api/prometheus/v1/api.go

@@ -238,6 +239,8 @@ type API interface {
 	Query(ctx context.Context, query string, ts time.Time) (model.Value, Warnings, error)
 	// QueryRange performs a query for the given range.
 	QueryRange(ctx context.Context, query string, r Range) (model.Value, Warnings, error)
+	// Runtimeinfo returns the various runtime information properties about the Prometheus server.


Interesting, not a blocker, but this looks weird (:

bwplotka · 2020-06-03T11:54:23Z

api/prometheus/v1/api.go

+		return RuntimeinfoResult{}, err
+	}
+
+	_, body, _, err := h.client.Do(ctx, req)


I think this is something to fix in follow up PRs if any, just it feels wrong, will add issue to follow up (:

lilic added 2 commits May 18, 2020 20:12

api/prometheus/v1/api.go: Add support for /runtimeinfo endpoint

c9e3c02

Signed-off-by: Lili Cosic <cosiclili@gmail.com>

api/prometheus/v1/api_test.go: Add test case for runtimeinfo endpoint

6c43f2e

Signed-off-by: Lili Cosic <cosiclili@gmail.com>

simonpasquier approved these changes May 19, 2020

View reviewed changes

bwplotka approved these changes May 19, 2020

View reviewed changes

brancz approved these changes Jun 3, 2020

View reviewed changes

beorn7 merged commit 056e8af into prometheus:master Jun 3, 2020

lilic deleted the add-runtimeinfo branch June 3, 2020 11:46

bwplotka reviewed Jun 3, 2020

View reviewed changes

bwplotka mentioned this pull request Jun 3, 2020

api: Check status code before marshalling and returning response #763

Open

This was referenced Mar 16, 2021

build(deps): bump github.com/prometheus/client_golang from 1.4.0 to 1.9.0 concourse/dex#14

Closed

build(deps): bump github.com/prometheus/client_golang from 1.4.0 to 1.10.0 WorldProgrammingLtd/dex#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api/prometheus/v1/api.go: Add support for status/runtimeinfo endpoint #755

api/prometheus/v1/api.go: Add support for status/runtimeinfo endpoint #755

lilic commented May 18, 2020

simonpasquier left a comment

bwplotka left a comment

bwplotka May 19, 2020

lilic May 19, 2020

bwplotka Jun 3, 2020

bwplotka May 19, 2020

bwplotka May 19, 2020

bwplotka May 19, 2020

lilic May 19, 2020

bwplotka Jun 3, 2020

lilic Jun 3, 2020 •

edited

Loading

brancz left a comment

lilic commented Jun 3, 2020

beorn7 commented Jun 3, 2020

lilic commented Jun 3, 2020 •

edited

Loading

bwplotka left a comment

bwplotka Jun 3, 2020

bwplotka Jun 3, 2020

	// Runtimeinfo returns the various runtime information properties about the Prometheus server.
	// RuntimeInfo returns the various runtime information properties about the Prometheus server.

	Runtimeinfo(ctx context.Context) (RuntimeinfoResult, error)
	RuntimeInfo(ctx context.Context) (RuntimeinfoResult, error)

api/prometheus/v1/api.go: Add support for status/runtimeinfo endpoint #755

api/prometheus/v1/api.go: Add support for status/runtimeinfo endpoint #755

Conversation

lilic commented May 18, 2020

simonpasquier left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilic Jun 3, 2020 • edited Loading

Choose a reason for hiding this comment

brancz left a comment

Choose a reason for hiding this comment

lilic commented Jun 3, 2020

beorn7 commented Jun 3, 2020

lilic commented Jun 3, 2020 • edited Loading

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilic Jun 3, 2020 •

edited

Loading

lilic commented Jun 3, 2020 •

edited

Loading