metrics-rs / metrics

A metrics ecosystem for Rust.
MIT License
1.08k stars 148 forks source link

Add support for `tracing::Span::record`ed fields in `metrics-tracing-context` #408

Closed zohnannor closed 9 months ago

zohnannor commented 9 months ago

I wondered for the reason of why recorded fields are not added to the Labels and found none. Implemented it as just Label::extend_from_labels call in a Layer::on_record method. For the tests I am not sure, maybe I missed some corner case, but I can't see such a case.

P.S. Should I ignore falling CI checks?

zohnannor commented 9 months ago

Pretty sad perf stats. Ready for a review though.

     Running benches/layer.rs (/home/zohnannor/dev/metrics/target/release/deps/layer-5bf703a34fc6c1b2)
Gnuplot not found, using plotters backend
layer/base case         time:   [923.19 ps 923.33 ps 923.48 ps]
                        change: [-16.774% -16.758% -16.741%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  4 (4.00%) high mild
  4 (4.00%) high severe
layer/no integration    time:   [928.33 ps 928.41 ps 928.52 ps]
                        change: [-14.183% -14.125% -14.074%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe
layer/tracing layer only
                        time:   [923.75 ps 923.78 ps 923.81 ps]
                        change: [-14.670% -14.622% -14.579%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe
layer/metrics layer only
                        time:   [25.805 ns 26.988 ns 28.176 ns]
                        change: [+17.291% +21.416% +25.369%] (p = 0.00 < 0.05)
                        Performance has regressed.
layer/full integration  time:   [270.72 ns 270.85 ns 270.98 ns]
                        change: [+50.150% +50.248% +50.344%] (p = 0.00 < 0.05)
                        Performance has regressed.

     Running benches/visit.rs (/home/zohnannor/dev/metrics/target/release/deps/visit-1cf93c53aa04b250)
Gnuplot not found, using plotters backend
visit/record_str        time:   [23.671 ns 23.686 ns 23.699 ns]
                        change: [+137.50% +138.60% +139.73%] (p = 0.00 < 0.05)
                        Performance has regressed.
visit/record_bool[true] time:   [18.172 ns 18.179 ns 18.186 ns]
                        change: [+201.05% +201.53% +201.80%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low severe
  1 (1.00%) low mild
  4 (4.00%) high mild
visit/record_bool[false]
                        time:   [18.097 ns 18.099 ns 18.101 ns]
                        change: [+199.13% +199.29% +199.44%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 11 outliers among 100 measurements (11.00%)
  3 (3.00%) high mild
  8 (8.00%) high severe
visit/record_i64        time:   [25.666 ns 25.673 ns 25.681 ns]
                        change: [+92.582% +92.723% +92.862%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low severe
  2 (2.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe
visit/record_u64        time:   [25.482 ns 25.488 ns 25.495 ns]
                        change: [+94.980% +95.075% +95.167%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
visit/record_debug      time:   [163.06 ns 163.16 ns 163.26 ns]
                        change: [+21.900% +22.026% +22.130%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low severe
  1 (1.00%) low mild
  3 (3.00%) high mild
visit/record_debug[bool]
                        time:   [39.089 ns 39.096 ns 39.104 ns]
                        change: [+73.869% +74.077% +74.278%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low severe
  3 (3.00%) low mild
  1 (1.00%) high mild
  1 (1.00%) high severe
visit/record_debug[i64] time:   [46.002 ns 46.010 ns 46.018 ns]
                        change: [+67.113% +67.556% +67.989%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) high mild
  3 (3.00%) high severe
visit/record_debug[u64] time:   [43.717 ns 43.731 ns 43.746 ns]
                        change: [+62.503% +62.569% +62.631%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild
As an image ![image](https://github.com/metrics-rs/metrics/assets/35764628/f8a80521-8535-4e22-8a0d-651273603914)
zohnannor commented 9 months ago

Cool! Thank you for review! When should we expect the release? 🙂

tobz commented 9 months ago

I would say within the next two weeks.

There's a lot of changes queued up on master, with some that I still need to work on and get merged, plus some testing, documentation/changelog/release notes updates.

zohnannor commented 8 months ago

@tobz friendly ping~

tobz commented 8 months ago

👋🏻

Sorry, life has been very busy 😭 ... although I'm finally on holiday vacation starting today. 😀 Going to try and see if I can get release notes written up today and then cut releases for everything.

tobz commented 8 months ago

Released as metrics-tracing-context@v0.15.0.

Thanks again for your contribution. ❤️