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.

Sign up for GitHub

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

Jump to bottom

core: expose an accessor for the FieldSet on Attributes #1331

Merged
merged 5 commits into from
Apr 15, 2021
Merged

core: expose an accessor for the FieldSet on Attributes #1331

merged 5 commits into from
Apr 15, 2021

Conversation

Folyd
Copy link
Contributor

@Folyd Folyd commented Mar 27, 2021 •
edited

Motivation

Exposing an accessor for the FieldSet on Attributes can motivate the subscriber to achieve
some performance improvement, such as OpenTelemetrySubscriber #1327.

Alternative

Make ValueSet::field_set() be pub instead of pub(crate).

@hawkw
Copy link
Member

hawkw commented Mar 30, 2021

Hmm. I wonder if it's better to just expose an accessor for the FieldSet on Attributes?

@Folyd
Copy link
Contributor Author

Folyd commented Mar 31, 2021

Hmm. I wonder if it's better to just expose an accessor for the FieldSet on Attributes?

A good alternative. 👍 I'll do some changes to this PR.

@Folyd Folyd changed the title core: add len() method to Attributes core: expose an accessor for the FieldSet on Attributes Mar 31, 2021
hawkw
hawkw requested changes Apr 1, 2021
View reviewed changes
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.

A couple of small nits. Once those are addressed, I'll be happy to merge this, thank you!

tracing-core/src/span.rs Outdated Show resolved Hide resolved
tracing-core/src/span.rs Outdated Show resolved Hide resolved
hawkw
hawkw approved these changes Apr 13, 2021
View reviewed changes
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.

looks good to me!

@hawkw hawkw merged commit 03e9ee4 into tokio-rs:master Apr 15, 2021
@Folyd Folyd deleted the attributes-len branch April 16, 2021 02:32
hawkw added a commit that referenced this pull request Apr 16, 2021
@Folyd @hawkw
core: expose an accessor for the FieldSet on Attributes (#1331)
14eee0a 
## Motivation

Exposing an accessor for the `FieldSet` on `Attributes` can motivate the
subscriber to achieve some performance improvement, such as
`OpenTelemetrySubscriber` #1327.

## Alternative

Make `ValueSet::field_set()` be `pub` instead of `pub(crate)`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw mentioned this pull request Apr 16, 2021
Merged
hawkw added a commit that referenced this pull request Apr 16, 2021
@Folyd @hawkw
core: expose an accessor for the FieldSet on Attributes (#1331)
d45b6c5 
## Motivation

Exposing an accessor for the `FieldSet` on `Attributes` can motivate the
subscriber to achieve some performance improvement, such as
`OpenTelemetrySubscriber` #1327.

## Alternative

Make `ValueSet::field_set()` be `pub` instead of `pub(crate)`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Apr 16, 2021
@Folyd @hawkw
core: expose an accessor for the FieldSet on Attributes (#1331)
e2f7324 
## Motivation

Exposing an accessor for the `FieldSet` on `Attributes` can motivate the
subscriber to achieve some performance improvement, such as
`OpenTelemetrySubscriber` #1327.

## Alternative

Make `ValueSet::field_set()` be `pub` instead of `pub(crate)`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Apr 16, 2021
@Folyd @hawkw
core: expose an accessor for the FieldSet on Attributes (#1331)
34b0bd8 
## Motivation

Exposing an accessor for the `FieldSet` on `Attributes` can motivate the
subscriber to achieve some performance improvement, such as
`OpenTelemetrySubscriber` #1327.

## Alternative

Make `ValueSet::field_set()` be `pub` instead of `pub(crate)`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Apr 17, 2021
@Folyd @hawkw
core: expose an accessor for the FieldSet on Attributes (#1331)
95fe9a3 
## Motivation

Exposing an accessor for the `FieldSet` on `Attributes` can motivate the
subscriber to achieve some performance improvement, such as
`OpenTelemetrySubscriber` #1327.

## Alternative

Make `ValueSet::field_set()` be `pub` instead of `pub(crate)`.

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Apr 30, 2021
@hawkw
core: prepare to release v0.1.18
6c935b8 
# 0.1.18 (April 30, 2010)

### Added

- `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>`
  ([#1358])
- `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>`
  ([#1374])
- Symmetric `From` impls for existing `Into` impls on `Current` and
  `Option<Id>` ([#1335])
- `Attributes::fields` accessor that returns the set of fields defined
  on a span's `Attributes` ([#1331])

Thanks to @Folyd for contributing to this release!

[#1358]: #1358
[#1374]: #1374
[#1335]: #1335
[#1331]: #1331
@hawkw hawkw mentioned this pull request Apr 30, 2021
Merged
hawkw added a commit that referenced this pull request Apr 30, 2021
@hawkw
core: prepare to release v0.1.18
8d428b2 
# 0.1.18 (April 30, 2010)

### Added

- `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>`
  ([#1358])
- `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>`
  ([#1374])
- Symmetric `From` impls for existing `Into` impls on `Current` and
  `Option<Id>` ([#1335])
- `Attributes::fields` accessor that returns the set of fields defined
  on a span's `Attributes` ([#1331])

Thanks to @Folyd for contributing to this release!

[#1358]: #1358
[#1374]: #1374
[#1335]: #1335
[#1331]: #1331
hawkw added a commit that referenced this pull request Apr 30, 2021
@hawkw
tracing: prepare to release 0.1.26
0e92b71 
# 0.1.26 (April 30, 2021)

### Fixed

- **attributes**: Compatibility between `#[instrument]` and `async-trait`
  v0.1.43 and newer ([#1228])
- Several documentation fixes ([#1305], [#1344])
### Added

- `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>`
  ([#1358])
- `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>`
  ([#1374])
- Symmetric `From` impls for existing `Into` impls on `span::Current`,
  `Span`, and `Option<Id>` ([#1335], [#1338])
- `From<EnteredSpan>` implementation for `Option<Id>`, allowing
  `EnteredSpan` to be used in a `span!` macro's `parent:` field ([#1325])
- `Attributes::fields` accessor that returns the set of fields defined
  on a span's `Attributes` ([#1331])

Thanks to @Folyd, @nightmared, and new contributors @rmsc and @Fishrock123 for
contributing to this release!

[#1227]: #1228
[#1305]: #1305
[#1325]: #1325
[#1338]: #1338
[#1344]: #1344
[#1358]: #1358
[#1374]: #1374
[#1335]: #1335
[#1331]: #1331
@hawkw hawkw mentioned this pull request Apr 30, 2021
Merged
hawkw added a commit that referenced this pull request Apr 30, 2021
@hawkw
tracing: prepare to release 0.1.26 (#1383)
d9e8ece 
# 0.1.26 (April 30, 2021)

### Fixed

- **attributes**: Compatibility between `#[instrument]` and `async-trait`
  v0.1.43 and newer ([#1228])
- Several documentation fixes ([#1305], [#1344])
### Added

- `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>`
  ([#1358])
- `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>`
  ([#1374])
- Symmetric `From` impls for existing `Into` impls on `span::Current`,
  `Span`, and `Option<Id>` ([#1335], [#1338])
- `From<EnteredSpan>` implementation for `Option<Id>`, allowing
  `EnteredSpan` to be used in a `span!` macro's `parent:` field ([#1325])
- `Attributes::fields` accessor that returns the set of fields defined
  on a span's `Attributes` ([#1331])

Thanks to @Folyd, @nightmared, and new contributors @rmsc and @Fishrock123 for
contributing to this release!

[#1227]: #1228
[#1305]: #1305
[#1325]: #1325
[#1338]: #1338
[#1344]: #1344
[#1358]: #1358
[#1374]: #1374
[#1335]: #1335
[#1331]: #1331
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
core: prepare to release v0.1.18
b67cbea 
# 0.1.18 (April 30, 2010)

### Added

- `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>`
  ([tokio-rs#1358])
- `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>`
  ([tokio-rs#1374])
- Symmetric `From` impls for existing `Into` impls on `Current` and
  `Option<Id>` ([tokio-rs#1335])
- `Attributes::fields` accessor that returns the set of fields defined
  on a span's `Attributes` ([tokio-rs#1331])

Thanks to @Folyd for contributing to this release!

[tokio-rs#1358]: tokio-rs#1358
[tokio-rs#1374]: tokio-rs#1374
[tokio-rs#1335]: tokio-rs#1335
[tokio-rs#1331]: tokio-rs#1331
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
tracing: prepare to release 0.1.26 (tokio-rs#1383)
11cc462 
# 0.1.26 (April 30, 2021)

### Fixed

- **attributes**: Compatibility between `#[instrument]` and `async-trait`
  v0.1.43 and newer ([tokio-rs#1228])
- Several documentation fixes ([tokio-rs#1305], [tokio-rs#1344])
### Added

- `Subscriber` impl for `Box<dyn Subscriber + Send + Sync + 'static>`
  ([tokio-rs#1358])
- `Subscriber` impl for `Arc<dyn Subscriber + Send + Sync + 'static>`
  ([tokio-rs#1374])
- Symmetric `From` impls for existing `Into` impls on `span::Current`,
  `Span`, and `Option<Id>` ([tokio-rs#1335], [tokio-rs#1338])
- `From<EnteredSpan>` implementation for `Option<Id>`, allowing
  `EnteredSpan` to be used in a `span!` macro's `parent:` field ([tokio-rs#1325])
- `Attributes::fields` accessor that returns the set of fields defined
  on a span's `Attributes` ([tokio-rs#1331])

Thanks to @Folyd, @nightmared, and new contributors @rmsc and @Fishrock123 for
contributing to this release!

[tokio-rs#1227]: tokio-rs#1228
[tokio-rs#1305]: tokio-rs#1305
[tokio-rs#1325]: tokio-rs#1325
[tokio-rs#1338]: tokio-rs#1338
[tokio-rs#1344]: tokio-rs#1344
[tokio-rs#1358]: tokio-rs#1358
[tokio-rs#1374]: tokio-rs#1374
[tokio-rs#1335]: tokio-rs#1335
[tokio-rs#1331]: tokio-rs#1331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@carllerche carllerche Awaiting requested review from carllerche carllerche is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Folyd @hawkw