Skip to content

feat: add observability to anchoring worker #1199

Open
samika98 wants to merge 4 commits into
developfrom
feat/add_obs_to_cas
Open

feat: add observability to anchoring worker #1199
samika98 wants to merge 4 commits into
developfrom
feat/add_obs_to_cas

Conversation

@samika98

Copy link
Copy Markdown
Contributor

[Replace Me With Meaningful Name] - #[Issue]

Description

Added spans to see how long each transaction while anchoring candidates takes in the worker.
This will help us identify bottlenecks and improve scalability.

How Has This Been Tested?

Describe the tests that you ran to verify your changes. Provide instructions for reproduction.

  • Test A (e.g. Test A - New test that ... ran in local, docker, and dev unstable.)
  • Test B

Definition of Done

Before submitting this PR, please make sure:

  • The work addresses the description and outcomes in the issue
  • I have added relevant tests for new or updated functionality
  • My code follows conventions, is well commented, and easy to understand
  • My code builds and tests pass without any errors or warnings
  • I have tagged the relevant reviewers
  • I have updated the READMEs of affected packages
  • I have made corresponding changes to the documentation
  • The changes have been communicated to interested parties

References:

Please list relevant documentation (e.g. tech specs, articles, related work etc.) relevant to this change, and note if the documentation has been updated.

@samika98 samika98 requested a review from smrz2001 April 26, 2024 19:57
@codecov-commenter

codecov-commenter commented Apr 26, 2024

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.70%. Comparing base (dc77998) to head (f57a9c4).
⚠️ Report is 62 commits behind head on develop.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1199      +/-   ##
===========================================
+ Coverage    81.51%   81.70%   +0.18%     
===========================================
  Files           46       46              
  Lines         1829     1847      +18     
  Branches       289      291       +2     
===========================================
+ Hits          1491     1509      +18     
  Misses         338      338              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

logger.imp(`Creating Merkle tree from ${candidates.length} selected streams`)
const span = Metrics.startSpan('anchor_candidates')

const buildMerkleTreeSpan = Metrics.startSpan('build_merkle_tree')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does startSpan create timing metrics automatically? As in, how long a given span takes on avg, etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A span represents a single operation within a specific trace. Traces are used to monitor the execution paths in a system. startSpan() will handle timing by reporting start and endTime.
It does not allow us to do aggregations, like counts or gauges. So the answer to your question is : no https://opentelemetry.io/docs/concepts/signals/traces/#spans

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In that case, can we please add metrics for the time spent in each of those spans?

Counter for how many time a span is taken, a summary for how long each took, etc.

@samika98 samika98 May 10, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am wondering if this is necessary after we data-dog profiling enabled on CAS @christianlavoie @smrz2001

Base automatically changed from feat/no-ipfs to develop May 2, 2024 19:41
@samika98 samika98 requested a review from gvelez17 May 7, 2024 16:44
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.

4 participants