Add metrics endpoints by skoeva · Pull Request #665 · flatcar/nebraska

skoeva · 2023-08-02T19:49:56Z

Pull request #656 adds the necessary instance_stats database table as well as functions for querying instance counts (by channel, version, and architecture) from the groups, instance, and instance_application tables and storing the results in the new table.

To make this data accessible outside Nebraska, we create the following:

Prometheus metrics endpoint: A new HTTP endpoint (instance-metrics/prometheus) that serves only the latest snapshot from the instance_stats table. As Prometheus is a time-series database, this serves instance counts with the latest timestamp only.
JSON metrics data endpoint: A new HTTP endpoint that emits all instance_stats data in JSON format. The difference here is that we query for all data, and we emit one JSON document per row.

backend/api/spec.yaml

backend/pkg/api/instances.go

yolossn

Left few comments, overall the prometheus metrics endpoint looks good.

pothos · 2023-08-22T11:36:09Z

backend/pkg/api/metrics.go

+	latestInstanceStatsSQL string = `
+SELECT channel_name, version, arch, timestamp, instances AS instances_count
+FROM instance_stats
+WHERE timestamp = (SELECT MAX(timestamp) FROM instance_stats)


Instead of doing raw SQL here, could we add a new function GetInstanceStatsLatest in pkg/api/instances.go and use that? This would also allow to add a test for this new function.

I had this logic initially in pkg/api/instances.go but decided to follow the convention above, though I agree with this suggestion

skoeva · 2023-08-22T22:03:20Z

I have left an update in my project doc for you guys to go through regarding my progress as my internship has concluded and I have started the fall semester at my university

I'm unable to make changes with my current setup but happy to follow along with the work on Nebraska!

ervcz · 2025-04-23T07:28:48Z

Relates to #566

tormath1 · 2025-04-24T12:22:12Z

I wanted to add a test for GET /instance-metrics/prometheus but after checking code, I noticed that we actually test this in pkg/api/instances_test.go so I would just test the Prometheus handler, which is pointless.

Pull request #656 adds the necessary instance_stats database table as well as functions for querying instance counts (by channel, version, and architecture) from the groups, instance, and instance_application tables and storing the results in the new table. To make this data accessible outside Nebraska, we create the following: - Prometheus metrics endpoint: A new HTTP endpoint (instance-metrics/prometheus) that serves only the latest snapshot from the instance_stats table. As Prometheus is a time-series database, this serves instance counts with the latest timestamp only. - JSON metrics data endpoint: A new HTTP endpoint that emits all instance_stats data in JSON format. The difference here is that we query for all data, and we emit one JSON document per row.

ervcz · 2025-04-29T13:48:32Z

backend/api/spec.yaml

+                type: string
+        "500":
+          description: Get latest instance stats error response
+  /instance-metrics/json:


I'm wondering if we need the json sub-path, especially if we also define the response content type as json

Suggested change

/instance-metrics/json:

/instance-metrics:

In this case, I would propose to keep /instance-metrics/json and use /instance-metrics for the Prometheus endpoint to get closer from the default /metrics endpoint.

ervcz · 2025-04-29T13:49:43Z

backend/api/spec.yaml

+          content:
+            application/x-ndjson:
+              schema:
+                $ref: "#/components/schemas/instanceStats"


should not this rather be referencing instanceStatsPage?

Suggested change

$ref: "#/components/schemas/instanceStats"

$ref: "#/components/schemas/instanceStatsPage"

ervcz · 2025-04-29T13:57:57Z

backend/pkg/api/instances.go

 }

+// GetInstanceStatsCount returns the total number of InstanceStats.
+func (api *API) GetInstanceStatsCount() (int, error) {


I noticed that the GetInstanceStats function accepts the perPage param as a uint64, but that is supposed to be a smaller number than the total count, and yet it reserves more memory, so there is a minor inconsistency in the usage of the types here, or am I wrong? Is go's default int type 32 bit?

Good catch. Let's stick to int.

ervcz · 2025-04-29T14:03:48Z

backend/pkg/api/instances.go

+		From("instance_stats").
+		Limit(limit).
+		Offset(offset).
+		Order(goqu.C("timestamp").Asc()).


I'm not sure if it matters or not, but is the ordering correct here? shouldn't we get the latest stats first?

ervcz · 2025-04-29T14:11:52Z

backend/pkg/api/metrics.go

+	latestInstanceStatsSQL = `
+SELECT channel_name, version, arch, timestamp, instances AS instances_count
+FROM instance_stats
+WHERE timestamp = (SELECT MAX(timestamp) FROM instance_stats)


do we have a index on timestamp?

ervcz · 2025-04-29T14:16:37Z

backend/pkg/api/metrics.go

+	ChannelName    string `db:"channel_name" json:"channel_name"`
+	Version        string `db:"version" json:"version"`
+	Arch           string `db:"arch" json:"arch"`
+	Timestamp      string `db:"timestamp" json:"timestamp"`


I guess it doesn't matter as we are just forwarding the value as text, but a go time type could also be used here if the driver supports the automatic conversion. Could be nice for later to change the presentation format of the timestamp.

ervcz · 2025-04-29T14:29:22Z

backend/pkg/handler/instances.go

+		Count:      len(metrics),
+		Metrics:    m,
+	}
+	if err := json.NewEncoder(ctx.Response()).Encode(p); err != nil {


The usage of the application/x-ndjson type is unusual as x-ndljson used to be used in streaming scenarios when the payload is made of homogeneous items without a wrapper. A batch would look like

... {"id":101,…} {"id":102,…} ...

whereas here we have a wrapper for the batch

p := metricPage{ TotalCount: totalCount, Count: len(metrics), Metrics: m, }

I recommend using plain Content-Type: application/json

skoeva requested a review from yolossn August 3, 2023 14:05

skoeva force-pushed the skoeva/endpoints branch 2 times, most recently from 7cb2fd7 to 274f205 Compare August 12, 2023 04:04

skoeva marked this pull request as ready for review August 12, 2023 23:51

skoeva force-pushed the skoeva/endpoints branch from 2e6dc8b to 00ddefe Compare August 14, 2023 12:50

yolossn reviewed Aug 21, 2023

View reviewed changes

backend/api/spec.yaml Show resolved Hide resolved

yolossn reviewed Aug 21, 2023

View reviewed changes

backend/pkg/api/instances.go Show resolved Hide resolved

yolossn reviewed Aug 21, 2023

View reviewed changes

backend/pkg/api/instances.go Show resolved Hide resolved

yolossn suggested changes Aug 21, 2023

View reviewed changes

pothos reviewed Aug 22, 2023

View reviewed changes

github-actions bot mentioned this pull request Nov 10, 2023

Monthly contributions report 2023-07-22 - 2023-08-21 flatcar/Flatcar#1246

Closed

tormath1 force-pushed the skoeva/endpoints branch from 00ddefe to f467768 Compare April 16, 2025 13:54

tormath1 self-assigned this Apr 16, 2025

tormath1 force-pushed the skoeva/endpoints branch 5 times, most recently from 8ec2d7e to 3393366 Compare April 22, 2025 15:39

tormath1 force-pushed the skoeva/endpoints branch 4 times, most recently from c675bb0 to 0451999 Compare April 24, 2025 12:19

tormath1 force-pushed the skoeva/endpoints branch from 0451999 to d4ee581 Compare April 24, 2025 13:39

tormath1 force-pushed the skoeva/endpoints branch from d4ee581 to 35cba54 Compare April 24, 2025 13:43

tormath1 requested a review from ervcz April 29, 2025 12:58

ervcz reviewed Apr 29, 2025

View reviewed changes

	$ref: "#/components/schemas/instanceStats"
	$ref: "#/components/schemas/instanceStatsPage"

Conversation

skoeva commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yolossn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skoeva commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ervcz commented Apr 23, 2025

Uh oh!

tormath1 commented Apr 24, 2025

Uh oh!

ervcz Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ervcz Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

skoeva commented Aug 2, 2023 •

edited

Loading

skoeva commented Aug 22, 2023 •

edited

Loading

ervcz Apr 29, 2025 •

edited

Loading

ervcz Apr 29, 2025 •

edited

Loading