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

scuffle-metrics: tests and docs #252

Merged
merged 28 commits into from
Jan 20, 2025
Merged

Conversation

lennartkloock
Copy link
Member

  • Add test coverage
  • Add docs

Closes CLOUD-6

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 90.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 62.77%. Comparing base (494a60b) to head (2e946bd).
Report is 36 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/bootstrap/telemetry/src/opentelemetry.rs 90.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
+ Coverage   55.74%   62.77%   +7.03%     
==========================================
  Files         195      196       +1     
  Lines       15850    15850              
==========================================
+ Hits         8835     9950    +1115     
+ Misses       7015     5900    -1115     
Files with missing lines Coverage Δ
crates/bootstrap/telemetry/src/lib.rs 76.78% <ø> (+76.78%) ⬆️
crates/metrics/derive/src/lib.rs 80.00% <ø> (ø)
crates/metrics/src/collector.rs 30.76% <ø> (+30.76%) ⬆️
crates/metrics/src/prometheus/mod.rs 62.08% <ø> (+62.08%) ⬆️
crates/bootstrap/telemetry/src/opentelemetry.rs 90.00% <90.00%> (ø)

... and 14 files with indirect coverage changes

Components Coverage Δ
scuffle-batching 100.00% <ø> (ø)
scuffle-bootstrap 86.50% <90.00%> (+33.89%) ⬆️
scuffle-context 100.00% <ø> (ø)
scuffle-ffmpeg 31.81% <ø> (ø)
scuffle-h3-webtransport 0.00% <ø> (ø)
scuffle-http 22.48% <ø> (+22.48%) ⬆️
scuffle-metrics 66.39% <ø> (+23.70%) ⬆️
scuffle-pprof 100.00% <ø> (+100.00%) ⬆️
scuffle-settings 0.00% <ø> (ø)
scuffle-signal 100.00% <ø> (ø)
postcompile 76.87% <ø> (+76.87%) ⬆️
scuffle-image-processor ∅ <ø> (∅)

@lennartkloock lennartkloock force-pushed the lennart/CLOUD-6-scuffle-metrics branch from 081a2d5 to e2a804c Compare January 13, 2025 13:50
@lennartkloock lennartkloock changed the title scuffle-metrics: test coverage and docs scuffle-metrics: docs Jan 13, 2025
@lennartkloock lennartkloock force-pushed the lennart/CLOUD-6-scuffle-metrics branch from a580388 to 2d3c5d8 Compare January 13, 2025 21:33
@lennartkloock lennartkloock changed the title scuffle-metrics: docs scuffle-metrics: tests and docs Jan 14, 2025
@lennartkloock lennartkloock force-pushed the lennart/CLOUD-6-scuffle-metrics branch from 1ee5a68 to e75c1da Compare January 15, 2025 18:42
@lennartkloock lennartkloock marked this pull request as ready for review January 15, 2025 21:23
@lennartkloock lennartkloock force-pushed the lennart/CLOUD-6-scuffle-metrics branch from 9e073f9 to fb6afcf Compare January 16, 2025 21:30
crates/metrics/Cargo.toml Outdated Show resolved Hide resolved
crates/metrics/src/lib.rs Outdated Show resolved Hide resolved
Copy link

cloudflare-workers-and-pages bot commented Jan 16, 2025

Deploying scuffle-docrs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2e946bd
Status: ✅  Deploy successful!
Preview URL: https://11ece6b0.scuffle-docrs.pages.dev
Branch Preview URL: https://pr-252.scuffle-docrs.pages.dev

View logs

Justfile Outdated Show resolved Hide resolved
@TroyKomodo
Copy link
Member

We should likely add an example using it with the scuffle_bootstrap_telemetry crate or just somehow to add it to the open telemetry exporter.

We should add what the #[metrics] attribute does here in the #[derive(MetricEnum)] context.

We are missing docs on the types under the scuffle_metrics::collector and scuffle_metrics::prometheus modules.

Is there a way to fix the weird formatting here?

We should add a doc example with using it with the bootstrap crate on the TelemetrySvc or on the root scuffle_bootstrap_telemetry?

Missing some documentation on scuffle_bootstrap_telemetry::opentelemetry::OpenTelemetry & scuffle_bootstrap_telemetry::opentelemetry::OpenTelemetryError

@TroyKomodo
Copy link
Member

The derive macro also allows for renaming fields.

https://pr-252.scuffle-docrs.pages.dev/scuffle_metrics/collector/struct.Collector the new function here talks about a metrics! macro but this is a bit misleading since this is a attribute macro not a declarative macro. I think better phrasing should be, this is typically used internally for constructing types when using the #[metrics] module or function attribute.

https://pr-252.scuffle-docrs.pages.dev/scuffle_metrics/prometheus/struct.PrometheusExporter the links here do not embed, not sure if desired. perhaps link directly to the upstream docs.rs?

@lennartkloock lennartkloock force-pushed the lennart/CLOUD-6-scuffle-metrics branch from e66aedc to fa3f3aa Compare January 19, 2025 02:48
@TroyKomodo
Copy link
Member

?brawl merge

@scuffle-brawl
Copy link
Contributor

scuffle-brawl bot commented Jan 20, 2025

📌 Commit 2e946bd has been approved and added to the merge queue.

Requested by: @TroyKomodo

Approved by: @TroyKomodo

@scuffle-brawl
Copy link
Contributor

scuffle-brawl bot commented Jan 20, 2025

⌛ Trying commit 2e946bd with merge fa14d64...

@scuffle-brawl
Copy link
Contributor

scuffle-brawl bot commented Jan 20, 2025

🎉 Build successful!
Completed in 8:23

Approved by: @TroyKomodo
Pushing fa14d64 to main

@scuffle-brawl scuffle-brawl bot merged commit fa14d64 into main Jan 20, 2025
11 checks passed
@scuffle-brawl scuffle-brawl bot deleted the lennart/CLOUD-6-scuffle-metrics branch January 20, 2025 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants