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

Support for using tracing span fields as metrics labels #87

Merged

Conversation

MOZGIII
Copy link
Contributor

@MOZGIII MOZGIII commented Sep 4, 2020

This PR adds support for using values defined via the tracing::span! macro as labels in the metrics crate.

It builds upon the refactor/metrics-v2 branch (metrics 0.13 series).

Closes #70
Closes #89


Current status

  • discussion is ongoing at https://discord.gg/RqTk6bq;
  • PoC with a workaround for callsite optimization is implemented;
  • API switched to use Key (a container KeyData) instead of Identifier to allow proper registry swap and enable decoupling required for altering the Key at runtime (at Layers);
  • Registry has been reimplemented to better fit the new Recorder API;
  • simple and extensible label filtering is implemented;
  • review in progress merged.

To do

  • get approval on the design
  • uncover edge cases
  • improve the UX
    • dynamic label filtering
    • explicit definitions of the allowed dynamic labels list at the macros
  • optimize Registry API and impl for the new operation mode
  • add proper test coverage
  • add proper docs
  • bench the macros
  • bench the Registry

@MOZGIII MOZGIII force-pushed the refactor/metrics-v2-tracing-context branch from 2d7081c to d7c7523 Compare September 11, 2020 07:12
@MOZGIII MOZGIII marked this pull request as ready for review September 14, 2020 20:06
Copy link
Member

@tobz tobz left a comment

Choose a reason for hiding this comment

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

Mostly just a lot of nits about documentation formatting/spelling.

metrics-macros/src/lib.rs Outdated Show resolved Hide resolved
metrics-macros/src/lib.rs Outdated Show resolved Hide resolved
metrics-macros/src/lib.rs Show resolved Hide resolved
metrics-tracing-context/src/label_filter.rs Outdated Show resolved Hide resolved
metrics-tracing-context/src/lib.rs Outdated Show resolved Hide resolved
metrics/src/key.rs Outdated Show resolved Hide resolved
metrics/src/key.rs Outdated Show resolved Hide resolved
MOZGIII and others added 6 commits September 15, 2020 14:37
Co-authored-by: Toby Lawrence <tobz@users.noreply.github.com>
Co-authored-by: Toby Lawrence <tobz@users.noreply.github.com>
Co-authored-by: Toby Lawrence <tobz@users.noreply.github.com>
Co-authored-by: Toby Lawrence <tobz@users.noreply.github.com>
Co-authored-by: Toby Lawrence <tobz@users.noreply.github.com>
Co-authored-by: Toby Lawrence <tobz@users.noreply.github.com>
@MOZGIII MOZGIII requested a review from tobz September 15, 2020 11:40
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.

None yet

2 participants