Skip to content

feat: add trade route tracing - #2596

Merged
graphite-app[bot] merged 1 commit into
mainfrom
arda/trade-route-tracing
Jun 1, 2026
Merged

feat: add trade route tracing#2596
graphite-app[bot] merged 1 commit into
mainfrom
arda/trade-route-tracing

Conversation

@findolor

@findolor findolor commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Chained PRs

Depends on:

Motivation

The REST trade routes use shared SDK methods for transaction lookup and filtered trade lists, but the general trade paths did not expose enough info-level timing to diagnose slow local DB/subgraph reads, merge/pagination costs, or transaction-specific fetch failures.

Solution

  • Add an info-level get_trades span with page, filter-shape, and chain-source fields.
  • Promote get_trades local DB, subgraph, and final merge/page timing summaries to info-level logs while preserving debug details.
  • Add an info-level get_trades_for_transaction span with transaction hash and filter-shape fields.
  • Add local DB/subgraph timing and error logs for transaction trade lookup.
  • Add focused tests for the new trace metadata summarization helpers.

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Verified locally:

  • nix develop -c cargo fmt --all
  • nix develop -c env COMMIT_SHA=local cargo check -p raindex_common
  • nix develop -c env COMMIT_SHA=local cargo test -p raindex_common trade_filter_trace_summary_counts_optional_filters
  • nix develop -c env COMMIT_SHA=local cargo test -p raindex_common transaction_trade_trace_summary_counts_optional_filters
  • nix develop -c env COMMIT_SHA=local cargo test -p raindex_common get_by_tx::tests::non_wasm
  • nix develop -c env COMMIT_SHA=local cargo test -p raindex_common get_all::tests

@findolor findolor mentioned this pull request May 27, 2026
4 tasks

findolor commented May 27, 2026

Copy link
Copy Markdown
Collaborator Author

How to use the Graphite Merge Queue

Add the label Raindex-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@findolor findolor mentioned this pull request May 27, 2026
4 tasks
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@graphite-app[bot], we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 4 minutes and 8 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 731e766e-1448-4f73-b651-c8c13cc6fb76

📥 Commits

Reviewing files that changed from the base of the PR and between d111acd and 40d1713.

📒 Files selected for processing (2)
  • crates/common/src/raindex_client/trades/get_all.rs
  • crates/common/src/raindex_client/trades/get_by_tx.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch arda/trade-route-tracing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@findolor findolor self-assigned this May 27, 2026
@findolor findolor changed the title Add tracing for trade route SDK paths feat: add trade route tracing May 27, 2026
@findolor
findolor force-pushed the arda/order-route-tracing branch from 751d7e3 to 49e40c3 Compare June 1, 2026 10:34
@findolor
findolor force-pushed the arda/trade-route-tracing branch from f76c476 to b55444c Compare June 1, 2026 10:34
@findolor
findolor force-pushed the arda/trade-route-tracing branch from b55444c to ad20ab2 Compare June 1, 2026 15:47
@findolor
findolor force-pushed the arda/order-route-tracing branch from 49e40c3 to 2684265 Compare June 1, 2026 15:47
@graphite-app
graphite-app Bot changed the base branch from arda/order-route-tracing to graphite-base/2596 June 1, 2026 16:07
@graphite-app
graphite-app Bot force-pushed the graphite-base/2596 branch from 2684265 to d111acd Compare June 1, 2026 16:30
@graphite-app
graphite-app Bot force-pushed the arda/trade-route-tracing branch from ad20ab2 to caec664 Compare June 1, 2026 16:30
@graphite-app
graphite-app Bot changed the base branch from graphite-base/2596 to main June 1, 2026 16:31
@graphite-app
graphite-app Bot force-pushed the arda/trade-route-tracing branch from caec664 to 424baed Compare June 1, 2026 16:31
@graphite-app

graphite-app Bot commented Jun 1, 2026

Copy link
Copy Markdown

Merge activity

## Chained PRs

Depends on:

- #2595

## Motivation

The REST trade routes use shared SDK methods for transaction lookup and filtered trade lists, but the general trade paths did not expose enough info-level timing to diagnose slow local DB/subgraph reads, merge/pagination costs, or transaction-specific fetch failures.

## Solution

- Add an info-level `get_trades` span with page, filter-shape, and chain-source fields.
- Promote `get_trades` local DB, subgraph, and final merge/page timing summaries to info-level logs while preserving debug details.
- Add an info-level `get_trades_for_transaction` span with transaction hash and filter-shape fields.
- Add local DB/subgraph timing and error logs for transaction trade lookup.
- Add focused tests for the new trace metadata summarization helpers.

## Checks

By submitting this for review, I'm confirming I've done the following:

- [x] made this PR as small as possible
- [x] unit-tested any new functionality
- [ ] linked any relevant issues or PRs
- [ ] included screenshots (if this involves a front-end change)

Verified locally:

- `nix develop -c cargo fmt --all`
- `nix develop -c env COMMIT_SHA=local cargo check -p raindex_common`
- `nix develop -c env COMMIT_SHA=local cargo test -p raindex_common trade_filter_trace_summary_counts_optional_filters`
- `nix develop -c env COMMIT_SHA=local cargo test -p raindex_common transaction_trade_trace_summary_counts_optional_filters`
- `nix develop -c env COMMIT_SHA=local cargo test -p raindex_common get_by_tx::tests::non_wasm`
- `nix develop -c env COMMIT_SHA=local cargo test -p raindex_common get_all::tests`
@graphite-app
graphite-app Bot force-pushed the arda/trade-route-tracing branch from 424baed to 40d1713 Compare June 1, 2026 17:03
graphite-app Bot pushed a commit that referenced this pull request Jun 1, 2026
## Dependent PRs

- Depends on #2596
- Follows up on Gleb's review comment in #2566 to add `tracing-test` coverage.

## Motivation

The tracing stack adds SDK instrumentation that the REST API relies on for order, trade, and take-order routes. This PR makes those logs test-covered so future changes do not silently remove the observability signals.

## Solution

- Add `tracing-test` as a dev dependency for `raindex_common`.
- Assert captured tracing output for `get_orders`, empty quote batch, remove calldata, transaction trades, generic trades, trades by order hash, and take-order approval.
- Reuse existing fixtures where available and add minimal empty-result trade fixtures for the generic trade paths.

## Checks

- `nix develop -c cargo fmt --all`
- `nix develop -c env COMMIT_SHA=local cargo check -p raindex_common`
- `nix develop -c cargo test -p raindex_common --lib`
@graphite-app
graphite-app Bot merged commit 40d1713 into main Jun 1, 2026
18 checks passed
@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

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