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(routing): success based routing metrics #5951

Merged
merged 115 commits into from
Sep 26, 2024
Merged

feat(routing): success based routing metrics #5951

merged 115 commits into from
Sep 26, 2024

Conversation

prajjwalkumar17
Copy link
Contributor

@prajjwalkumar17 prajjwalkumar17 commented Sep 19, 2024

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

This PR will enable logging of metrics for Dynamic Routing feature. This will be an async call inside the post_update_tracker.
Screenshot 2024-09-19 at 4 53 42 PM

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Once the PR goes to sandbox a grafana dashboard will be created.
tested the metrics (DYNAMIC_SUCCESS_BASED_ROUTING) in local Grafana. Here is the screenshot for the same:
Screenshot 2024-09-25 at 6 08 20 PM

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

prajjwalkumar17 and others added 30 commits September 6, 2024 21:05
@prajjwalkumar17 prajjwalkumar17 marked this pull request as ready for review September 24, 2024 04:16
crates/hyperswitch_interfaces/src/api.rs Outdated Show resolved Hide resolved
crates/router/src/core/payments.rs Outdated Show resolved Hide resolved
crates/router/src/core/payments.rs Outdated Show resolved Hide resolved
crates/router/src/core/payments.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing.rs Outdated Show resolved Hide resolved
crates/storage_impl/src/redis/cache.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
crates/router/src/core/routing/helpers.rs Show resolved Hide resolved
);

let success_based_connectors = client
.calculate_success_rate(
Copy link
Member

Choose a reason for hiding this comment

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

why is this function not accepting &str?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@Aprabhat19 Aprabhat19 Sep 26, 2024

Choose a reason for hiding this comment

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

For id_type we are anyhow passing a String value to the service, so that's why we chose not to pass a &str and then clone the value later

crates/router/src/core/routing/helpers.rs Outdated Show resolved Hide resolved
Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Could you update the "How did you test it" section in the PR description to include the name of the metric that you queried for?

@likhinbopanna likhinbopanna added this pull request to the merge queue Sep 26, 2024
Merged via the queue into main with commit 809c92b Sep 26, 2024
20 checks passed
@likhinbopanna likhinbopanna deleted the dr_metrics branch September 26, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-routing Area: Routing C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration with Payments core for metrics creation
6 participants