Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metrics endpoints #665

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions backend/api/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,41 @@ paths:
description: Activity not found response
"500":
description: List activity error response
/instance-metrics/prometheus:
get:
description: Get latest instance stats
operationId: getLatestInstanceStats
security:
- oidcBearerAuth: []
- oidcCookieAuth: []
- githubCookieAuth: []
responses:
"200":
description: Get latest instance stats success response
content:
text/plain:
schema:
type: string
"500":
description: Get latest instance stats error response
/instance-metrics/json:
get:
description: Get instance stats
operationId: getInstanceStats
security:
- oidcBearerAuth: []
- oidcCookieAuth: []
- githubCookieAuth: []
responses:
"200":
description: Get instance stats success response
content:
application/x-ndjson:
schema:
$ref: "#/components/schemas/instanceStats"
"500":
description: Get instance stats error response
Comment on lines +1197 to +1213
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a response stream? And do we want to return (via stream) all the instance stats in this endpoint? I would suggest to use some form pagination instead of stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the goal is to output all instance stats here, and I agree with the pagination suggestion


components:
schemas:
## request Body
Expand Down Expand Up @@ -1339,6 +1374,30 @@ components:
package_id:
type: string

instanceStats:
type: object
required:
- type
- channel
- version
- arch
- timestamp
- instances
properties:
type:
type: string
enum: ["instance_count"]
channel:
type: string
version:
type: string
arch:
type: string
timestamp:
type: string
count:
type: integer


## response
config:
Expand Down
4 changes: 2 additions & 2 deletions backend/pkg/api/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const (

const (
validityInterval postgresDuration = "1 days"
defaultInterval time.Duration = time.Hour
defaultInterval time.Duration = 2 * time.Hour
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change intentional here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! This was just an issue with rebasing, see #666

)

// Instance represents an instance running one or more applications for which
Expand Down Expand Up @@ -651,7 +651,7 @@ func (api *API) GetDefaultInterval() time.Duration {
// that have been checked in during a given duration from a given time.
func (api *API) instanceStatsQuery(t *time.Time, duration *time.Duration) *goqu.SelectDataset {
if t == nil {
now := time.Now()
now := time.Now().UTC()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is done in #664, please rebase with main to avoid redoing this part.

t = &now
}

Expand Down
35 changes: 35 additions & 0 deletions backend/pkg/api/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ WHERE a.id = e.application_id AND e.event_type_id = et.id AND et.result = 0 AND
GROUP BY app_name
ORDER BY app_name
`, ignoreFakeInstanceCondition("e.instance_id"))

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

`
)

type AppInstancesPerChannelMetric struct {
Expand Down Expand Up @@ -77,6 +83,35 @@ func (api *API) GetFailedUpdatesMetrics() ([]FailedUpdatesMetric, error) {
return metrics, nil
}

type LatestInstanceStatsMetric struct {
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"`
InstancesCount int `db:"instances_count" json:"instances_count"`
}

func (api *API) GetLatestInstanceStatsMetrics() ([]LatestInstanceStatsMetric, error) {
var metrics []LatestInstanceStatsMetric
rows, err := api.db.Queryx(latestInstanceStatsSQL)
if err != nil {
return nil, err
}
defer rows.Close()
for rows.Next() {
var metric LatestInstanceStatsMetric
err := rows.StructScan(&metric)
if err != nil {
return nil, err
}
metrics = append(metrics, metric)
}
if err := rows.Err(); err != nil {
return nil, err
}
return metrics, nil
}

func (api *API) DbStats() sql.DBStats {
return api.db.Stats()
}
182 changes: 182 additions & 0 deletions backend/pkg/codegen/client.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading