monitoring PR handling failures. #42

Oded-B · 2024-11-29T12:31:23Z

This PR adds a few metrics to Telefonistka process

Description

Name:      "open_prs",
Help:      "The total number of open PRs",

Note: Just a metric to track activity in repo.

Name:      "open_promotion_prs",
Help:      "The total number of open PRs with promotion label",

Note: Could be used to check if we need to enforce merging/closing stale promotion PRs

Name:      "open_prs_with_pending_telefonistka_checks",
Help:      "The total number of open PRs with pending Telefonistka checks(excluding PRs with very recent commits)",

Note: those represent cases the Telefonsitka totaly failed, not even marking the checks as fialed

Name:      "commit_status_updates_total",
Help:      "The total number of commit status updates, and their status (success/pending/failure)",

We can use this track failure telefonistika is aware of

Type of Change

Checklist

I have read the contributing guidelines
Existing issues have been referenced (where applicable)
I have verified this change is not present in other open pull requests
Functionality is documented
All code style checks pass
New code contribution is covered by automated tests
All new and existing tests pass

We both explicit failures with pr_handle_failures_total And cases where telefonistka commit status check is left in "pending" state (because Telefonistka exploded while handling that event )

internal/pkg/githubapi/pr_metrics.go

This way we can also monitor the successes, so we can get a ratio or just the activity rate of the repo and such

Express const timeframes in time (not int)

improve error handeling Add context timeout

Oded-B · 2024-12-05T15:48:06Z

time.After(60time.Second)
context.WithTimeout(context.Background(), 60time.Second)

Oded-B · 2024-12-09T12:13:29Z

Sample from Lab:

# HELP telefonistka_github_commit_status_updates_total The total number of commit status updates, and their status (success/pending/failure)
# TYPE telefonistka_github_commit_status_updates_total counter
telefonistka_github_commit_status_updates_total{repo_slug="commercetools/k8s-manifests-poc",status="error"} 1
telefonistka_github_commit_status_updates_total{repo_slug="commercetools/k8s-manifests-poc",status="pending"} 1
# HELP telefonistka_github_open_promotion_prs The total number of open PRs with promotion label
# TYPE telefonistka_github_open_promotion_prs gauge
telefonistka_github_open_promotion_prs{repo_slug="commercetools/cf-cd-poc-code"} 0
telefonistka_github_open_promotion_prs{repo_slug="commercetools/k8s-manifests-poc"} 10
# HELP telefonistka_github_open_prs The total number of open PRs
# TYPE telefonistka_github_open_prs gauge
telefonistka_github_open_prs{repo_slug="commercetools/cf-cd-poc-code"} 0
telefonistka_github_open_prs{repo_slug="commercetools/k8s-manifests-poc"} 21
# HELP telefonistka_github_open_prs_with_pending_telefonistka_checks The total number of open PRs with pending Telefonistka checks(excluding PRs with very recent commits)
# TYPE telefonistka_github_open_prs_with_pending_telefonistka_checks gauge
telefonistka_github_open_prs_with_pending_telefonistka_checks{repo_slug="commercetools/cf-cd-poc-code"} 0
telefonistka_github_open_prs_with_pending_telefonistka_checks{repo_slug="commercetools/k8s-manifests-poc"} 0

docs/observability.md

internal/pkg/githubapi/pr_metrics.go

yzdann · 2024-12-11T11:10:15Z

internal/pkg/githubapi/pr_metrics.go

+		if err != nil {
+			log.Errorf("error getting PRs for %s/%s: %v", ghOwner, repo.GetName(), err)
+		}


Why not checking the error first, and then calling the prom.InstrumentGhCall(resp)?
Another question if the output for prom.InstrumentGhCall(resp) is not important why do bother to assign it something?

The InstrumentGhCall function returns labels just for testing.
During runtime these labels are just used internally in the function.
I chose to instrument before error handling to ensure we still instrument even if we change the error handling to break out of that code path in the future, be I can change the order if you feel it makes it less readable

I chose to instrument before error handling to ensure we still instrument even if we change the error handling to break out of that code path in the future, be I can change the order if you feel it makes it less readable

I would add that as a comment

internal/pkg/githubapi/pr_metrics.go

internal/pkg/githubapi/pr_metrics_test.go

internal/pkg/githubapi/pr_metrics.go

Co-authored-by: Yazdan Mohammadi <[email protected]>

yzdann

LGTM!
Optional: https://github.com/commercetools/telefonistka/pull/42/files#r1883774668

Oded-B added 2 commits November 29, 2024 13:27

Initial commit for monitoring PR handling failures.

14f9bda

We both explicit failures with pr_handle_failures_total And cases where telefonistka commit status check is left in "pending" state (because Telefonistka exploded while handling that event )

lint fixes

336f921

sunchill06 reviewed Nov 29, 2024

View reviewed changes

internal/pkg/githubapi/pr_metrics.go Outdated Show resolved Hide resolved

Oded-B added 8 commits November 29, 2024 16:22

Add test

5d3f248

Fix typo

e6aa7f4

Use a more generic approach to monitoring PR handle failures.

755b603

This way we can also monitor the successes, so we can get a ratio or just the activity rate of the repo and such

Document new metrics

2c29af9

Move metrics loop to metrics file.

84399ab

Express const timeframes in time (not int)

Fix type issue

9017b87

Fix lint issue

3ed74c6

Lint stuff

0682da7

Oded-B marked this pull request as ready for review December 4, 2024 12:39

Oded-B marked this pull request as draft December 4, 2024 12:45

Document dep' on GH apps

6f5801c

improve error handeling Add context timeout

Oded-B marked this pull request as ready for review December 4, 2024 13:54

Oded-B marked this pull request as draft December 5, 2024 15:47

Switch sleep for Ticker

662d47c

Oded-B marked this pull request as ready for review December 9, 2024 13:20

typo fix

be4d974

yzdann previously approved these changes Dec 11, 2024

View reviewed changes

yzdann reviewed Dec 11, 2024

View reviewed changes

internal/pkg/githubapi/pr_metrics.go Outdated Show resolved Hide resolved

yzdann reviewed Dec 11, 2024

View reviewed changes

internal/pkg/githubapi/pr_metrics.go Outdated Show resolved Hide resolved

yzdann reviewed Dec 11, 2024

View reviewed changes

internal/pkg/githubapi/pr_metrics.go Show resolved Hide resolved

Apply suggestions from code review

78f43e7

Co-authored-by: Yazdan Mohammadi <[email protected]>

Oded-B dismissed yzdann’s stale review via 78f43e7 December 12, 2024 08:06

Oded-B added 2 commits December 12, 2024 09:49

Use a struct to pass data between getRepoPrMetrics and PublishPrMetrics

4cc9584

Add new metrics to example

e51d579

Oded-B requested a review from yzdann December 12, 2024 10:25

yzdann approved these changes Dec 13, 2024

View reviewed changes

Oded-B merged commit 8439b61 into main Dec 13, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

monitoring PR handling failures. #42

monitoring PR handling failures. #42

Oded-B commented Nov 29, 2024 •

edited

Loading

Oded-B commented Dec 5, 2024

Oded-B commented Dec 9, 2024

yzdann Dec 11, 2024

Oded-B Dec 12, 2024

yzdann Dec 13, 2024

yzdann left a comment

monitoring PR handling failures. #42

monitoring PR handling failures. #42

Conversation

Oded-B commented Nov 29, 2024 • edited Loading

Description

Type of Change

Checklist

Oded-B commented Dec 5, 2024

Oded-B commented Dec 9, 2024

yzdann Dec 11, 2024

Choose a reason for hiding this comment

Oded-B Dec 12, 2024

Choose a reason for hiding this comment

yzdann Dec 13, 2024

Choose a reason for hiding this comment

yzdann left a comment

Choose a reason for hiding this comment

Oded-B commented Nov 29, 2024 •

edited

Loading