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(dex): add initial metrics to dex actions #2860

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Jul 21, 2023

Shipping some new dex-related metrics. Care has been taken to ensure that we are emitting Histogram, rather than Summary, data types. More info:

The buckets themselves were surely need to be tweaked, but for now, I'm eager to get these new metrics onto preview, and will follow up with some visualization work.

Refs #2788.

@conorsch conorsch temporarily deployed to smoke-test July 21, 2023 19:40 — with GitHub Actions Inactive
@conorsch conorsch temporarily deployed to smoke-test July 31, 2023 22:25 — with GitHub Actions Inactive
@conorsch conorsch marked this pull request as ready for review August 2, 2023 00:23
@conorsch conorsch temporarily deployed to smoke-test August 2, 2023 00:24 — with GitHub Actions Inactive
describe_histogram!(
DEX_TRADE_DURATION,
Unit::Seconds,
"The time spent executing trades within the DEX"
Copy link
Member

Choose a reason for hiding this comment

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

nit: later we should change this to DEX_ARB_DURATION and make it track the end_block arbitrage operation. this way we have individual measurements for both batch execution and arb execution which are the two main lifts for the dex engine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great feedback, added! Will follow up with some graphs next.

Shipping some new dex-related metrics. Care has been taken to ensure
that we are emitting Histogram, rather than Summary, data types. More
info:

  * https://prometheus.io/docs/practices/histograms/
  * https://docs.rs/metrics-exporter-prometheus/0.10.0/metrics_exporter_prometheus/struct.PrometheusBuilder.html#method.set_buckets

The buckets themselves were surely need to be tweaked, but for now, I'm
eager to get these new metrics onto preview, and will follow up with
some visualization work.

Refs #2788.
@conorsch conorsch temporarily deployed to smoke-test August 2, 2023 19:00 — with GitHub Actions Inactive
@conorsch conorsch merged commit fc3226e into main Aug 2, 2023
8 checks passed
@conorsch conorsch deleted the 2788-dex-metrics-part-1 branch August 2, 2023 19:29
@cratelyn cratelyn added the A-dex Area: Relates to the dex label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dex Area: Relates to the dex
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants