Skip to content

Record signatures origin in the signature processor #2489

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

Conversation

turmelclem
Copy link
Collaborator

@turmelclem turmelclem commented May 16, 2025

Content

We want to record the number of incoming signatures coming from the DMQ network. Once the DMQ is delivering enough signatures to reach the quorum, we will be able to sunset signature publication to the aggregator REST API.

  • Extend the SignatureConsumer trait to expose the origin metric of the consumer
  • increment the get_signature_registration_total_received_since_startup metric with the origin for every signature processed
  • Modify usage of metric signature_registration_total_received_since_startup incremented in HTTP server when calling register-signatures with HTTP value instead of origin_tag
  • remove usage of origin_tag header in register-signatures endpoint

Pre-submit checklist

  • 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

Closes #2478

Copy link

github-actions bot commented May 16, 2025

Test Results

    3 files  ±0     77 suites  ±0   14m 41s ⏱️ -15s
1 913 tests ±0  1 913 ✅ ±0  0 💤 ±0  0 ❌ ±0 
3 225 runs  ±0  3 225 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1d4de25. ± Comparison against base commit 090b7dc.

♻️ This comment has been updated with latest results.

@turmelclem turmelclem force-pushed the ctl/2478-record-signatures-origin-in-the-signature-processor branch 4 times, most recently from 21857b1 to 3a42c30 Compare May 16, 2025 16:38
@turmelclem turmelclem temporarily deployed to testing-preview May 16, 2025 16:47 — with GitHub Actions Inactive
@turmelclem turmelclem changed the title WIP: Ctl/2478 record signatures origin in the signature processor ctl/2478 record signatures origin in the signature processor May 19, 2025
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.

Pull Request Overview

This PR adds tracking of signature origins by injecting a MetricsService into the signature processor and labeling metrics with the consumer’s origin.

  • Inject MetricsService into SequentialSignatureProcessor and increment a labeled counter per processed signature
  • Extend SignatureConsumer with get_origin_network, update noop/fake implementations
  • Update metrics definitions to use specific origin label constants and adjust HTTP routes/tests to use a fixed "HTTP" origin

Reviewed Changes

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

Show a summary per file
File Description
src/services/signature_processor.rs Inject and store metrics_service, record origin‐labeled metrics
src/services/signature_consumer/interface.rs Add get_origin_network() to trait
src/services/signature_consumer/noop.rs Implement get_origin_network() returning "NOOP"
src/services/signature_consumer/fake.rs Implement get_origin_network() returning "FAKE"
src/metrics/service.rs Define distinct origin label constants and update metric builders
src/http_server/routes/signatures_routes.rs Remove dynamic origin header, hard-code "HTTP" for metrics
src/dependency_injection/builder/enablers/misc.rs Pass metrics_service into SequentialSignatureProcessor::new

@turmelclem turmelclem force-pushed the ctl/2478-record-signatures-origin-in-the-signature-processor branch from 3a42c30 to c79725b Compare May 19, 2025 12:21
@turmelclem turmelclem temporarily deployed to testing-preview May 19, 2025 12:30 — with GitHub Actions Inactive
Copy link
Collaborator

@dlachaume dlachaume left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@turmelclem turmelclem changed the title ctl/2478 record signatures origin in the signature processor Record signatures origin in the signature processor May 19, 2025
@turmelclem turmelclem force-pushed the ctl/2478-record-signatures-origin-in-the-signature-processor branch from c79725b to 03d9d83 Compare May 19, 2025 14:11
@turmelclem turmelclem temporarily deployed to testing-preview May 19, 2025 14:21 — with GitHub Actions Inactive
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 👍

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

@turmelclem turmelclem self-assigned this May 20, 2025
@turmelclem turmelclem force-pushed the ctl/2478-record-signatures-origin-in-the-signature-processor branch from 03d9d83 to db33b96 Compare May 20, 2025 07:38
@turmelclem turmelclem temporarily deployed to testing-preview May 20, 2025 07:47 — with GitHub Actions Inactive
* mithril-aggregator from `0.7.52` to `0.7.53`
@turmelclem turmelclem temporarily deployed to testing-preview May 20, 2025 08:06 — with GitHub Actions Inactive
@turmelclem turmelclem merged commit 2b88e17 into main May 20, 2025
38 checks passed
@turmelclem turmelclem deleted the ctl/2478-record-signatures-origin-in-the-signature-processor branch May 20, 2025 08:21
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 signatures origin in the signature processor of aggregator
4 participants