Skip to content

Feat: record origin of client requests in metrics #2411

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

Merged
merged 30 commits into from
Apr 8, 2025

Conversation

sfauvel
Copy link
Collaborator

@sfauvel sfauvel commented Apr 4, 2025

Content

This PR includes support for a new HTTP header in the aggregator REST API which allows recording the origin of requests.

The aggregator can set a list of authorized origin tags in its configuration. The default list contains EXPLORER, BENCHMARK, CI and NA which can be extended with additional tags.

The clients have also been updated to be able to provide the origin tag parameter.


Rust example using the mithril-client crate:

    let client =
        ClientBuilder::aggregator(&args.aggregator_endpoint, &args.genesis_verification_key)
            .with_origin_tag(Some("EXAMPLE".to_string()))
            .build()?;

Javascript example using the mithril-client-wasm library:

  let client_options = {
    origin_tag: "EXAMPLE"
  };
  let client = new MithrilClient(aggregator_endpoint, genesis_verification_key, client_options);

Client CLI command example:

mithril-client -- --origin-tag EXAMPLE cardano-db snapshot list
  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)
    • Add ADR blog post or Dev ADR entry (if relevant)
    • No new TODOs introduced

Issue(s)

Closes #2382

@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch from 7ab0eab to ffb9193 Compare April 4, 2025 12:42
Copy link

github-actions bot commented Apr 4, 2025

Test Results

    3 files  ± 0     57 suites  ±0   11m 27s ⏱️ -17s
1 830 tests +18  1 830 ✅ +18  0 💤 ±0  0 ❌ ±0 
2 244 runs  +26  2 244 ✅ +26  0 💤 ±0  0 ❌ ±0 

Results for commit af7c77b. ± Comparison against base commit 02301d9.

♻️ This comment has been updated with latest results.

@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch 2 times, most recently from 649300e to 39e045a Compare April 4, 2025 13:44
@dlachaume dlachaume changed the title WIP Record origin of client requests in metrics Feat: Record origin of client requests in metrics Apr 4, 2025
@dlachaume dlachaume changed the title Feat: Record origin of client requests in metrics Feat: record origin of client requests in metrics Apr 4, 2025
@dlachaume dlachaume self-assigned this Apr 4, 2025
@dlachaume dlachaume temporarily deployed to testing-preview April 4, 2025 13:53 — with GitHub Actions Inactive
@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch from 647e477 to 7e27ff9 Compare April 4, 2025 15:04
@dlachaume dlachaume temporarily deployed to testing-preview April 4, 2025 15:13 — with GitHub Actions Inactive
@dlachaume dlachaume marked this pull request as ready for review April 4, 2025 15:20
@dlachaume dlachaume requested a review from Copilot April 4, 2025 15:21
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 55 out of 55 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

mithril-aggregator/src/http_server/routes/artifact_routes/cardano_database.rs:32

  • The 'with_origin_tag' middleware is passed 'dependency_manager' here instead of 'router_state' as used in similar routes; please verify this is intentional.
.and(middlewares::with_origin_tag(dependency_manager))

@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch 2 times, most recently from e190665 to 025b74b Compare April 4, 2025 15:38
@dlachaume dlachaume requested a review from Copilot April 4, 2025 15:45
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 55 out of 55 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

mithril-aggregator/src/http_server/routes/artifact_routes/cardano_database.rs:32

  • Consider using 'router_state' consistently for the 'with_origin_tag' middleware. Verify if passing 'dependency_manager' here is intentional compared to other endpoints that use 'router_state'.
.and(middlewares::with_origin_tag(dependency_manager))

Copy link
Collaborator

@turmelclem turmelclem left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Alenar and others added 9 commits April 7, 2025 18:56
Co-authored-by: Clément TURMEL <turmelclem@users.noreply.github.com>
…tead of the deleted parameter.

Co-authored-by: Clément TURMEL <turmelclem@users.noreply.github.com>
Co-authored-by: Clément TURMEL <turmelclem@users.noreply.github.com>
…on for the aggregator node and how to specify an origin tag with the mithril-client library and mithril-client CLI

Co-authored-by: Clément TURMEL <turmelclem@users.noreply.github.com>
…rate in a dedicated function the computation of the HTTP headers from the HTTP headers in the options and with the optional origin tag.

Co-authored-by: Clément TURMEL <turmelclem@users.noreply.github.com>
Co-authored-by: DJO <Alenar@users.noreply.github.com>
@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch from eec0720 to 5b9480b Compare April 7, 2025 16:57
@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch from 5b9480b to e2b6281 Compare April 7, 2025 17:09
@dlachaume dlachaume temporarily deployed to testing-preview April 7, 2025 17:18 — with GitHub Actions Inactive
@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch from ef9c8e4 to 088d255 Compare April 8, 2025 08:23
@dlachaume dlachaume temporarily deployed to testing-preview April 8, 2025 08:31 — with GitHub Actions Inactive
@dlachaume dlachaume force-pushed the sfa/2382/record_origin_of_client_requests_in_metrics branch from 088d255 to 8c3404b Compare April 8, 2025 08:32
@dlachaume dlachaume temporarily deployed to testing-preview April 8, 2025 08:41 — with GitHub Actions Inactive
Copy link
Collaborator

@Alenar Alenar left a comment

Choose a reason for hiding this comment

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

LGTM

dlachaume and others added 3 commits April 8, 2025 12:29
…rigin_tag`

This fixes the unwanted behavior where calling `with_options` after `with_origin_tag` would erase the `origin_tag` value.

Co-authored-by: DJO <Alenar@users.noreply.github.com>
 name = "client-cardano-database-v2" -version = "0.0.3" +version = "0.0.4" * client-cardano-database from `0.1.28` to `0.1.29`
* client-cardano-stake-distribution from `0.1.10` to `0.1.11`
* client-cardano-transaction from `0.1.20` to `0.1.21`
* client-mithril-stake-distribution from `0.2.8` to `0.2.9`
* mithril-metric from `0.1.9` to `0.1.10`
* mithril-aggregator from `0.7.29` to `0.7.30`
* mithril-client-cli from `0.11.12` to `0.11.13`
* mithril-client-wasm from `0.8.6` to `0.8.7`
* mithril-client from `0.11.18` to `0.11.19`
* mithril-common from `0.5.20` to `0.5.21`
* mithril-aggregator-fake from `0.4.4` to `0.4.5`
* mithril-end-to-end from `0.4.77` to `0.4.78`
* [js] client-wasm-nodejs from `0.3.6` to `0.3.7`
* [js] client-wasm-web from `0.3.6` to `0.3.7`
* [js] client-wasm-ci-test from `0.3.6` to `0.3.7`
* [js] mithril-client-wasm from `0.8.6` to `0.8.7`
* [js] mithril-explorer from `0.7.32` to `0.7.33`
@dlachaume dlachaume temporarily deployed to testing-preview April 8, 2025 12:19 — with GitHub Actions Inactive
@dlachaume dlachaume merged commit efae09c into main Apr 8, 2025
41 checks passed
@dlachaume dlachaume deleted the sfa/2382/record_origin_of_client_requests_in_metrics branch April 8, 2025 12:48
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.

Record origin of client requests in metrics
5 participants