private-attribution / ipa

A raw implementation of Interoperable Private Attribution
MIT License
42 stars 25 forks source link

Make disable-metrics feature turn off global recorder #1345

Closed akoshelev closed 1 month ago

akoshelev commented 1 month ago

It turns out we've been running with metrics overhead the whole time. I tested a run locally with disable-metrics feature and to my surprise I still saw the telemetry emitted from helper binary.

Upon investigating, I came across our Verbosity struct that was installing the collector unconditionally. I changed that and tested that metrics are no longer emitted

akoshelev commented 1 month ago

I can see the difference now

with metrics Running IPA for 1000000 records took 633.11591412s https://draft-mpc.vercel.app/query/view/lyric-days2024-10-10T0425

without metrics Running IPA for 1000000 records took 558.098657199s https://draft-mpc.vercel.app/query/view/gutsy-flyer2024-10-10T0414

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.54%. Comparing base (7e1c180) to head (f0672e5). Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
ipa-core/src/cli/metric_collector.rs 0.00% 1 Missing :warning:
ipa-core/src/cli/verbosity.rs 91.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1345 +/- ## ======================================= Coverage 93.54% 93.54% ======================================= Files 209 209 Lines 34331 34335 +4 ======================================= + Hits 32114 32119 +5 + Misses 2217 2216 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.