Add metrics and refactor SPKS #81

wejdross · 2024-06-24T10:55:29Z

adding prometheus metrics to catch issues when sending data to providers and Odoo
reffactoring SPKS module to fix bug and improve reliabiality and performance

Checklist

The PR has a meaningful title. It will be used to auto generate the
changelog.
The PR has a meaningful description that sums up the change. It will be
linked in the changelog.
PR contains a single logical change (to build a better changelog).
Update the documentation.
Categorize the PR by adding one of the labels:
bug, enhancement, documentation, change, breaking, dependency
as they show up in the changelog.
Link this PR to related issues or PRs.

zugao

LGTM. Check the bugs in lab cluster, also don't forget to see if the prometheus metrics are exposed.

Kidswiss · 2024-07-03T07:49:57Z

pkg/cmd/spks.go

-				logger.Error(err, "can't export data to Odoo")
-				return err
-			}
+			firstTime := make(chan bool, 1)


You could just call runSPKSBilling() here instead of creating an additional channel.

actually this is leftover from way another approach :D I have no idea why it survived, it was part of retry mechanism etc. switching to simple function call

Kidswiss · 2024-07-03T07:52:45Z

pkg/cmd/spks.go

@@ -199,11 +201,13 @@ func QueryPrometheus(ctx context.Context, v1api v1.API, query string, logger log
 	case model.ValVector:
 		vectorVal := result.(model.Vector)
 		if len(vectorVal) != 1 {
-			return 0, fmt.Errorf("expected 1 result, got %d", len(vectorVal))
+			logger.Error(err, "Result vector length is not 1, prometheus query failed and returned: ", "result", vectorVal)


Shouldn't we increase the failure metric here?

Kidswiss · 2024-07-03T07:52:52Z

pkg/cmd/spks.go

 		}
 	default:
-		return 0, fmt.Errorf("result type is not Vector: %s", result.Type())
+		logger.Error(err, "result type is not Vector: ", "result", result)


Here as well?

Kidswiss · 2024-07-03T07:54:51Z

pkg/cmd/spks.go

 		logger.Error(err, "Error querying Prometheus")
-		return 0, err
+		return -1


Why aren't you returning (int, error) for this function? That would make it much clearer.

Also, instead of increasing the metrics here, you could do it in runSPKSBilling(). It would simplify the logic, as you'd only have to catch one error case and not all three as in this function.

Kidswiss · 2024-07-03T08:00:41Z

pkg/cmd/spks.go

-				logger.Error(err, "Error querying Prometheus")
-				return err
-			}
+			now := time.Now().In(location)


Did you check with tobru if the timestamps are now really correct?

We had to change a bit the code on Monday because the timestamp was showing 00:00Z instead of 22:00Z UTC. Now data is arriving properly.

Yes it's correct as it is now. We tested and verified it in Odoo directly.

zugao

Just fix Simon's comments and the rest is fine. We tested on Monday and data arrives ok.

TheBigLee

LGTM. Just one small nitpick and Simon's fixes

TheBigLee · 2024-07-04T07:15:04Z

pkg/cmd/spks.go

-				logger.Error(err, "Error querying Prometheus")
-				return err
-			}
+			now := time.Now().In(location)


Yes it's correct as it is now. We tested and verified it in Odoo directly.

TheBigLee · 2024-07-04T07:18:43Z

pkg/cmd/spks.go

+		{
+			ProductID:     "appcat-spks-mariadb-standard",
+			InstanceID:    "mariadb-standard",
+			SalesOrder:    salesOrder,
+			UnitID:        UnitID,
+			ConsumedUnits: float64(mariadbStandard),
+			TimeRange: odoo.TimeRange{
+				From: startYesterdayAbsolute,
+				To:   endYesterdayAbsolute,
+			},
+		},
+		{
+			ProductID:     "appcat-spks-mariadb-premium",
+			InstanceID:    "mariadb-premium",
+			SalesOrder:    salesOrder,
+			UnitID:        UnitID,
+			ConsumedUnits: float64(mariadbPremium),
+			TimeRange: odoo.TimeRange{
+				From: startYesterdayAbsolute,
+				To:   endYesterdayAbsolute,
+			},
+		},
+		{
+			ProductID:     "appcat-spks-redis-standard",
+			InstanceID:    "redis-standard",
+			SalesOrder:    salesOrder,
+			UnitID:        UnitID,
+			ConsumedUnits: float64(redisStandard),
+			TimeRange: odoo.TimeRange{
+				From: startYesterdayAbsolute,
+				To:   endYesterdayAbsolute,
+			},
+		},
+		{
+			ProductID:     "appcat-spks-redis-premium",
+			InstanceID:    "redis-premium",
+			SalesOrder:    salesOrder,
+			UnitID:        UnitID,
+			ConsumedUnits: float64(redisPremium),
+			TimeRange: odoo.TimeRange{
+				From: startYesterdayAbsolute,
+				To:   endYesterdayAbsolute,
+			},
+		},


Small nitpick: Lot's of copy&paste. This can probably be compacted

extracted timerange to separate variable, but generation of those provides even more code :D

wejdross · 2024-07-05T09:28:06Z

 go run . spks (...) --prometheus-url http://127.0.0.1:9090 --days 4






2024-07-05T11:16:10.130+0200 | INFO | billing-collector-cloudservices | billing-collector-cloudservices/main.go:164 | Starting up billing-collector-cloudservices | {"date": "2024-07-05", "commit": "-dirty-", "go_os": "linux", "go_arch": "amd64", "go_version": "go1.22.3", "uid": 1000, "gid": 1000}
2024-07-05T11:16:10.130+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:63 | starting spks data collector
2024-07-05T11:16:10.130+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-01T00:00:00.000+0200", "startYesterdayAbsolute": "2024-06-30T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-01T00:00:00.000+0200"}
2024-07-05T11:16:10.130+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-02T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-01T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-02T00:00:00.000+0200"}
2024-07-05T11:16:10.131+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-03T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-02T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-03T00:00:00.000+0200"}
2024-07-05T11:16:10.131+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-04T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-03T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-04T00:00:00.000+0200"}
2024-07-05T11:16:10.131+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-05T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-04T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-05T00:00:00.000+0200"}


2024-07-05T11:16:15.131+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-05T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-04T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-05T00:00:00.000+0200"}
2024-07-05T11:16:20.134+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-05T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-04T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-05T00:00:00.000+0200"}
2024-07-05T11:16:25.134+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:115 | Running SPKS billing with such timeranges:  | {"startOfToday": "2024-07-05T00:00:00.000+0200", "startYesterdayAbsolute": "2024-07-04T00:00:00.000+0200", "endYesterdayAbsolute": "2024-07-05T00:00:00.000+0200"}
^C2024-07-05T11:16:29.888+0200 | INFO | billing-collector-cloudservices.spks | cmd/spks.go:77 | Received Context cancellation, exiting...

wejdross force-pushed the addMetrics branch 2 times, most recently from c6c341e to 7f99373 Compare June 24, 2024 12:20

zugao approved these changes Jun 25, 2024

View reviewed changes

wejdross requested review from a team, Kidswiss, TheBigLee and zugao and removed request for a team July 3, 2024 07:42

Kidswiss requested changes Jul 3, 2024

View reviewed changes

Kidswiss reviewed Jul 3, 2024

View reviewed changes

zugao approved these changes Jul 3, 2024

View reviewed changes

TheBigLee approved these changes Jul 4, 2024

View reviewed changes

refactoring SPKS and adding base metrics

083bda9

wejdross force-pushed the addMetrics branch from 482aa54 to 083bda9 Compare July 5, 2024 08:18

[email protected] added 2 commits July 5, 2024 10:47

adjusting timezone shift

08e72e8

fixing timestamp offsets

b861285

wejdross requested review from Kidswiss, TheBigLee and zugao July 5, 2024 09:24

Kidswiss approved these changes Jul 5, 2024

View reviewed changes

wejdross merged commit 34d006e into master Jul 5, 2024
3 checks passed

wejdross deleted the addMetrics branch July 5, 2024 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics and refactor SPKS #81

Add metrics and refactor SPKS #81

wejdross commented Jun 24, 2024

zugao left a comment

Kidswiss Jul 3, 2024

wejdross Jul 4, 2024 •

edited

Loading

Kidswiss Jul 3, 2024

Kidswiss Jul 3, 2024

Kidswiss Jul 3, 2024

Kidswiss Jul 3, 2024

zugao Jul 3, 2024 •

edited

Loading

TheBigLee Jul 4, 2024

zugao left a comment •

edited

Loading

TheBigLee left a comment

TheBigLee Jul 4, 2024

TheBigLee Jul 4, 2024

wejdross Jul 5, 2024

wejdross commented Jul 5, 2024

Add metrics and refactor SPKS #81

Add metrics and refactor SPKS #81

Conversation

wejdross commented Jun 24, 2024

Checklist

zugao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wejdross Jul 4, 2024 • edited Loading

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

zugao Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zugao left a comment • edited Loading

Choose a reason for hiding this comment

TheBigLee 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

wejdross commented Jul 5, 2024

wejdross Jul 4, 2024 •

edited

Loading

zugao Jul 3, 2024 •

edited

Loading

zugao left a comment •

edited

Loading