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

instrument macro doesn't allow declaration of fields that have a period . in their name #785

Closed
glademiller opened this issue Jul 7, 2020 · 3 comments · Fixed by #773
Closed
Labels
crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request

Comments

@glademiller
Copy link

Feature Request

Motivation

I would like to set http.status_code to empty in order to be able to record the value later the most convenient way to do this would be with the macro.

Proposal

Allow
#[tracing::instrument(fields(http.status_code = ...))]

Alternatives

Allow recording of fields without declaring them upfront.

@hawkw
Copy link
Member

hawkw commented Jul 7, 2020

This was implemented in #672. However, it has yet to be released as that PR also causes instrument to reject unrecognized input that it did not previously reject, which we think may constitute a breaking change (#773 (comment)). Stay tuned, though --- I'm planning on trying to make some additional changes to make that change backwards compatible.

@hawkw hawkw added crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request labels Jul 7, 2020
@glademiller
Copy link
Author

glademiller commented Jul 7, 2020

@hawkw awesome thanks for the update. Would this also allow for declaring the value on that field in the macro to be tracing::field::Empty? I suppose I can always just put another dummy value but being able to use Empty would more clearly communicate my intent.

Edit: I should have read through the linked comments first it looks like you are working through arbitrary expressions etc.

@hawkw
Copy link
Member

hawkw commented Jul 7, 2020

Yup, that's right. :)

Currently, field names with no value in instrument are mapped to tracing::field::Empty, which is what that PR does as well, for backwards-compatibility (although you will also be able to use Empty explicitly as well when #672 is published) . However, in tracing-attributes 0.2, we want to change this so that instrument supports the same local variable shorthand as the function-like tracing macros, so you will need to use Empty explicitly instead, the same as all the other macros.

@hawkw hawkw closed this as completed in #773 Jul 8, 2020
hawkw added a commit that referenced this issue Jul 8, 2020
# 0.1.9 (July 8, 2020)

### Added

- Support for arbitrary expressions as fields in `#[instrument]` (#672)

### Changed

- `#[instrument]` now emits a compiler warning when ignoring 
  unrecognized input (#672, #786)

Fixes #785 

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/attributes Related to the `tracing-attributes` crate kind/feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants