feat: Expose firecracker metrics #357

yitsushi · 2022-01-18T13:34:52Z

What this PR does / why we need it:

Basic logic for the metrics exporter.
Basic docker compose file to start a prometheus and grafana instance
for testing.
Metrics collection under provider.
Documentation

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #210

Special notes for your reviewer:

Checklist:

squashed commits into logical changes
includes documentation
adds unit tests
adds or updates e2e tests

yitsushi · 2022-01-18T13:35:11Z

.golangci.yml

@@ -26,8 +26,6 @@ linters-settings:
    local-prefixes: github.com/weaveworks/flintlock
  govet:
    check-shadowing: true
-  misspell:
-    locale: GB


Duplicated entry, removed one of them.

internal/command/metrics/machine_metrics.go

internal/command/metrics/main.go

yitsushi · 2022-01-18T13:41:49Z

internal/command/flags/urfave.go

+
+type WithFlagsFunc func() []cli.Flag
+
+func CLIFlags(options ...WithFlagsFunc) []cli.Flag {


I like this component builder pattern here because that way when/if we add more binaries (now exporter, maybe a helper to hide a lot of "setup" steps into a single binary to deploy, an agent that can sit in the microvm, or basically anything) we can reused the same flags. Maybe it will not work long term, but now it has a nice structure.

i like everything going on in this file ✨

yitsushi · 2022-01-18T13:42:14Z

internal/command/flags/urfave.go

+	}
+}
+
+func ParseFlags(cfg *config.Config) cli.BeforeFunc {


It will parse everything we know, and if not defined it will be empty or default value.

internal/command/metrics/machine_metrics.go

codecov-commenter · 2022-01-25T10:11:47Z

Codecov Report

Merging #357 (f7cbd61) into main (dd27bc6) will decrease coverage by 0.70%.
The diff coverage is 0.00%.

❗ Current head f7cbd61 differs from pull request most recent head ea1014c. Consider uploading reports for the commit ea1014c to get more accurate results

@@            Coverage Diff             @@
##             main     #357      +/-   ##
==========================================
- Coverage   57.34%   56.63%   -0.71%     
==========================================
  Files          51       52       +1     
  Lines        2567     2599      +32     
==========================================
  Hits         1472     1472              
- Misses        970     1003      +33     
+ Partials      125      124       -1

Impacted Files	Coverage Δ
infrastructure/firecracker/metrics.go	`0.00% <0.00%> (ø)`
infrastructure/firecracker/provider.go	`0.00% <0.00%> (ø)`
infrastructure/containerd/event_service.go	`56.14% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd27bc6...ea1014c. Read the comment docs.

yitsushi · 2022-01-27T10:04:06Z

Example metrics output:

❯ curl -s http://localhost:8000/machine | grep vcpu_failures
vcpu_failures{namespace="ns1",name="mvm1",uid="01FTBC51BK5V82XEDBASXAF61Z"} 0
vcpu_failures{namespace="ns1",name="mvm2",uid="01FTBC5ZEKE79FGYNRHPC8MME5"} 0
vcpu_failures{namespace="ns1",name="mvm2",uid="01FTBFG9KAC46TVDYGH1JMH665"} 0
vcpu_failures{namespace="ns2",name="mvm5",uid="01FTDFSRDFKMBR5GVT6J8F6K2E"} 0

❯ curl -s http://localhost:8000/machine/ns1 | grep vcpu_failures
vcpu_failures{namespace="ns1",name="mvm2",uid="01FTBC5ZEKE79FGYNRHPC8MME5"} 0
vcpu_failures{namespace="ns1",name="mvm2",uid="01FTBFG9KAC46TVDYGH1JMH665"} 0
vcpu_failures{namespace="ns1",name="mvm1",uid="01FTBC51BK5V82XEDBASXAF61Z"} 0

❯ curl -s http://localhost:8000/machine/ns1/mvm2 | grep vcpu_failures
vcpu_failures{namespace="ns1",name="mvm2",uid="01FTBC5ZEKE79FGYNRHPC8MME5"} 0
vcpu_failures{namespace="ns1",name="mvm2",uid="01FTBFG9KAC46TVDYGH1JMH665"} 0

❯ curl -s http://localhost:8000/machine/uid/01FTDFSRDFKMBR5GVT6J8F6K2E | grep vcpu_failures
vcpu_failures{namespace="ns2",name="mvm5",uid="01FTDFSRDFKMBR5GVT6J8F6K2E"} 0

yitsushi · 2022-01-27T10:15:51Z

Hmm actually it's not #209 but #210.

@richardcase : is there anything in your mind what can we expose from flintlock right other than firecracker metrics? Or this PR can close both of them now and when we have something to expose we file a new ticket?

Update: #209 is not just exposing but implementing stuff to expose.

* Basic logic for the metrics exporter. * Basic docker compose file to start a prometheus and grafana instance for testing. * Metrics collection under provider. * Documentation Fixes liquidmetal-dev#210

Callisto13

can you add a bit in the Makefile under build for this?

are we having tests?

Callisto13 · 2022-01-31T12:24:01Z

internal/command/flags/urfave.go

+
+type WithFlagsFunc func() []cli.Flag
+
+func CLIFlags(options ...WithFlagsFunc) []cli.Flag {


i like everything going on in this file ✨

Callisto13 · 2022-01-31T14:47:04Z

i am trying really hard to run this thing but containerd is not behaving today so i can't even create anything 😫

Callisto13

LGTM, the only thing i would love is to refactor all the handlefuncs into their own little things for tidiness

oh and a build bit in Makefile

yitsushi · 2022-02-01T09:32:43Z

@Callisto13 : Can I get a new review? Changes:

Separate function for different serve endpoints.
Make with build and build-release for metric
Updated release workflow to include the new binaries.

Callisto13

🚀

yitsushi added the kind/feature New feature or request label Jan 18, 2022

yitsushi commented Jan 18, 2022

View reviewed changes

internal/command/metrics/machine_metrics.go Outdated Show resolved Hide resolved

yitsushi commented Jan 18, 2022

View reviewed changes

internal/command/metrics/main.go Show resolved Hide resolved

yitsushi commented Jan 18, 2022

View reviewed changes

yitsushi commented Jan 19, 2022

View reviewed changes

internal/command/metrics/machine_metrics.go Outdated Show resolved Hide resolved

yitsushi force-pushed the 209-expose-metrics branch from bf30e74 to f7bb26b Compare January 25, 2022 10:11

yitsushi marked this pull request as ready for review January 25, 2022 10:24

yitsushi force-pushed the 209-expose-metrics branch from 8c8a262 to 57e0aae Compare January 25, 2022 10:30

yitsushi marked this pull request as draft January 26, 2022 10:29

yitsushi force-pushed the 209-expose-metrics branch from 6903ad8 to f39fae1 Compare January 27, 2022 09:59

yitsushi marked this pull request as ready for review January 27, 2022 09:59

yitsushi force-pushed the 209-expose-metrics branch from f39fae1 to 7e31a76 Compare January 27, 2022 10:39

yitsushi changed the title ~~feat: Expose flintlock metrics~~ feat: Expose firecracker metrics Jan 27, 2022

yitsushi requested a review from richardcase January 28, 2022 11:33

feat: Expose firecracker metrics

5f6e6e8

* Basic logic for the metrics exporter. * Basic docker compose file to start a prometheus and grafana instance for testing. * Metrics collection under provider. * Documentation Fixes liquidmetal-dev#210

yitsushi force-pushed the 209-expose-metrics branch from 7e31a76 to 5f6e6e8 Compare January 31, 2022 10:23

Callisto13 reviewed Jan 31, 2022

View reviewed changes

Callisto13 previously approved these changes Jan 31, 2022

View reviewed changes

Refactor serve and add metrics binary to build and release

6ab49a7

yitsushi dismissed Callisto13’s stale review via 6ab49a7 February 1, 2022 09:31

Callisto13 approved these changes Feb 1, 2022

View reviewed changes

Merge branch 'main' into 209-expose-metrics

ea1014c

yitsushi merged commit a5f4023 into liquidmetal-dev:main Feb 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Expose firecracker metrics #357

feat: Expose firecracker metrics #357

yitsushi commented Jan 18, 2022 •

edited

Loading

yitsushi Jan 18, 2022

yitsushi Jan 18, 2022

Callisto13 Jan 31, 2022

yitsushi Jan 18, 2022

codecov-commenter commented Jan 25, 2022 •

edited

Loading

yitsushi commented Jan 27, 2022

yitsushi commented Jan 27, 2022 •

edited

Loading

Callisto13 left a comment

Callisto13 Jan 31, 2022

Callisto13 commented Jan 31, 2022

Callisto13 left a comment •

edited

Loading

yitsushi commented Feb 1, 2022

Callisto13 left a comment


		type WithFlagsFunc func() []cli.Flag

		func CLIFlags(options ...WithFlagsFunc) []cli.Flag {

feat: Expose firecracker metrics #357

feat: Expose firecracker metrics #357

Conversation

yitsushi commented Jan 18, 2022 • edited Loading

yitsushi Jan 18, 2022

Choose a reason for hiding this comment

yitsushi Jan 18, 2022

Choose a reason for hiding this comment

Callisto13 Jan 31, 2022

Choose a reason for hiding this comment

yitsushi Jan 18, 2022

Choose a reason for hiding this comment

codecov-commenter commented Jan 25, 2022 • edited Loading

Codecov Report

yitsushi commented Jan 27, 2022

yitsushi commented Jan 27, 2022 • edited Loading

Callisto13 left a comment

Choose a reason for hiding this comment

Callisto13 Jan 31, 2022

Choose a reason for hiding this comment

Callisto13 commented Jan 31, 2022

Callisto13 left a comment • edited Loading

Choose a reason for hiding this comment

yitsushi commented Feb 1, 2022

Callisto13 left a comment

Choose a reason for hiding this comment

yitsushi commented Jan 18, 2022 •

edited

Loading

codecov-commenter commented Jan 25, 2022 •

edited

Loading

yitsushi commented Jan 27, 2022 •

edited

Loading

Callisto13 left a comment •

edited

Loading