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

subscriber: support special chars in span names #1368

Merged
merged 4 commits into from Apr 27, 2021

Conversation

aym-v
Copy link
Contributor

@aym-v aym-v commented Apr 23, 2021

Filtering log events using the target[span{field=value}]=level syntax doesn't work when the span name contains a special char.

This PR closes #1367 by adding dash support in span names.

@aym-v aym-v requested review from davidbarsky, hawkw and a team as code owners April 23, 2021 11:39
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I wonder if we could make this more general than just permitting dashes in span names --- see my comment.

tracing-subscriber/src/filter/env/directive.rs Outdated Show resolved Hide resolved
@aym-v aym-v force-pushed the aym/support-dashes-in-span-names branch from f438b1f to 8183be1 Compare April 24, 2021 13:30
@aym-v
Copy link
Contributor Author

aym-v commented Apr 24, 2021

@hawkw Thanks for your feedback, span names are now open to all characters except ] and {. I added tests accordingly.

@aym-v aym-v changed the title subscriber: support dashes in span names subscriber: support special chars in span names Apr 24, 2021
@aym-v aym-v requested a review from hawkw April 24, 2021 14:08
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

thanks, this looks great to me!

@hawkw
Copy link
Member

hawkw commented Apr 27, 2021

CI is all passing, but GitHub thinks this is missing a required check, for some reason...I'm just going to go ahead and merge it.

@hawkw hawkw merged commit 1de85c5 into tokio-rs:master Apr 27, 2021
hawkw added a commit that referenced this pull request Apr 30, 2021
Filtering log events using the `target[span{field=value}]=level` syntax
doesn't work when the span name contains a special char.

This PR closes #1367 by adding support for span names with any
characters other than `{` and `]` to `EnvFilter`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw mentioned this pull request Apr 30, 2021
hawkw added a commit that referenced this pull request Apr 30, 2021
Filtering log events using the `target[span{field=value}]=level` syntax
doesn't work when the span name contains a special char.

This PR closes #1367 by adding support for span names with any
characters other than `{` and `]` to `EnvFilter`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Apr 30, 2021
# 0.2.18 (April 30, 2021)

### Deprecated

- Deprecated the `CurrentSpan` type, which is inefficient and largely
  superseded by the `registry` API ([#1321])

### Fixed

- **json**: Invalid JSON emitted for events in spans with no fields
  ([#1333])
- **json**: Missing span data for synthesized new span, exit, and close
  events ([#1334])
- **fmt**: Extra space before log lines when timestamps are disabled
  ([#1355])

### Added

- **env-filter**: Support for filters on spans whose names contain any
  characters other than `{` and `]` ([#1368])

Thanks to @Folyd, and new contributors @akinnane and @aym-v for
contributing to  this release!

[#1321]: #1321
[#1333]: #1333
[#1334]: #1334
[#1355]: #1355
[#1368]: #1368
hawkw added a commit that referenced this pull request May 1, 2021
# 0.2.18 (April 30, 2021)

### Deprecated

- Deprecated the `CurrentSpan` type, which is inefficient and largely
  superseded by the `registry` API ([#1321])

### Fixed

- **json**: Invalid JSON emitted for events in spans with no fields
  ([#1333])
- **json**: Missing span data for synthesized new span, exit, and close
  events ([#1334])
- **fmt**: Extra space before log lines when timestamps are disabled
  ([#1355])

### Added

- **env-filter**: Support for filters on spans whose names contain any
  characters other than `{` and `]` ([#1368])

Thanks to @Folyd, and new contributors @akinnane and @aym-v for
contributing to  this release!

[#1321]: #1321
[#1333]: #1333
[#1334]: #1334
[#1355]: #1355
[#1368]: #1368
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.

tracing-subscriber: filtering not working for spans with dashes in their names
2 participants