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

feat: Add route based metrics across API #1465

Merged
merged 21 commits into from
Aug 8, 2024
Merged

feat: Add route based metrics across API #1465

merged 21 commits into from
Aug 8, 2024

Conversation

filvecchiato
Copy link
Contributor

Enables the metrics for all routes available in the API. An extra arg "-pq" has been created to allow the user to select if queryparams should be taken into consideration when creating metrics. An extra command has been included to run all benchmarks in one command in /benchmarks/init.sh

the following histograms have been created:

  • route latency
  • route response size
  • route response size - latency ratio

example use:
yarn dev -p => metrics route based

yarn dev -p -pq => metrics route based with queryparams

@filvecchiato filvecchiato requested a review from a team as a code owner July 30, 2024 05:58
@filvecchiato filvecchiato marked this pull request as draft July 30, 2024 05:58
@filvecchiato filvecchiato requested a review from Imod7 July 30, 2024 05:59
src/main.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
@TarikGul
Copy link
Member

TarikGul commented Aug 1, 2024

Overall this is really well done. Just a few nits to clean this up. Feel free to ping me when this changes from being a draft.

@filvecchiato filvecchiato marked this pull request as ready for review August 6, 2024 15:03
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Great Job! Lgtm 🚀

Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

Nice work! Those metrics are great additions! 💯

docs/src/openapi-v1.yaml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
src/util/metrics.ts Outdated Show resolved Hide resolved
@filvecchiato filvecchiato requested a review from Imod7 August 8, 2024 09:25
Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

Awesome work! Just a last tiny comment.
Very excited with these metrics and what insights they will give us in Sidecar's performance.

README.md Outdated Show resolved Hide resolved
@filvecchiato filvecchiato merged commit a4bbcb8 into master Aug 8, 2024
15 checks passed
@filvecchiato filvecchiato deleted the metrics branch August 8, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants