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.

Sign up for GitHub

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

Jump to bottom

loadtest: add support for push metrics #662

Closed
wants to merge 2 commits into from
Closed

loadtest: add support for push metrics #662

wants to merge 2 commits into from

Conversation

calvinrzachman
Copy link
Contributor

@calvinrzachman calvinrzachman commented Nov 8, 2023 •
edited
Loading

Equip the test orchestrator with the ability to push metrics on execution time to a configurable remote prometheus push gateway.

Related to: #662 (loadtest metrics vs. tapd metrics)

@calvinrzachman
loadtest: add support for push metrics
2929f7b 
Equip the test orchestrator with the ability to push
metrics on execution time to a configurable remote prometheus
push gateway.
@calvinrzachman
loadtest: add config validation for prom gateway
33a1ac5 
Apply some basic sanity checks that the Prometheus PushGateway
configuration is valid.
@dstadulis dstadulis requested a review from ffranr November 8, 2023 16:24
ffranr
ffranr reviewed Nov 8, 2023
View reviewed changes
Copy link
Contributor

@ffranr ffranr left a comment

Choose a reason for hiding this comment

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

Nice! Good to see an example of how this is done.

We usually hard wrap at 80 characters, I think some lines go over right now.

I think you should squash these commits.

itest/loadtest/config.go
Enabled bool `long:"enabled" description:"Enable pushing metrics to Prometheus PushGateway"`
Host string `long:"host" description:"Prometheus PushGateway host address"`
Port int `long:"port" description:"Prometheus PushGateway port"`
PushURL string
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think you should add a doc comment to describe PushURL.

@Roasbeef Roasbeef requested a review from GeorgeTsagk November 10, 2023 20:17
@dstadulis
Copy link
Collaborator

Other metric criteria to collect:

  • Universe
  • Balance
  • Number of UTXOs
  • Queries per second QPS
  • Aspects exposed on the RPC
  • Time to fetch proof
  • "Delay metrics"
  • Other criteria to make alerts

End goal:

  • Loop-service-like metrics
  • Sanity checks

Comments:

  • Avoid high cardinality metrics
  • Collector is pased an interface

@lightninglabs-deploy
Copy link

@GeorgeTsagk: review reminder
@calvinrzachman, remember to re-request review from reviewers when ready

@GeorgeTsagk GeorgeTsagk mentioned this pull request Dec 13, 2023
Merged
GeorgeTsagk
GeorgeTsagk reviewed Dec 13, 2023
View reviewed changes
Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

Closing this in favor of #737

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ffranr ffranr ffranr left review comments

@GeorgeTsagk GeorgeTsagk GeorgeTsagk left review comments

Assignees

@GeorgeTsagk GeorgeTsagk

Labels
performance testing
Projects
Taproot-Assets Project Board
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@calvinrzachman @dstadulis @lightninglabs-deploy @ffranr @GeorgeTsagk