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

Add support for typed task Labels. #138

Merged
merged 1 commit into from
Mar 7, 2024
Merged

Conversation

jorajeev
Copy link
Member

@jorajeev jorajeev commented Mar 4, 2024

PR98 added support for associated a single untyped Tag with each task and thread. As we've gained experience with Tags, we've increasingly felt a need to have a mechanism that is both better typed, and allows more than one tag to be associated with tasks. This commit introduces Labels, which are inspired by Extensions in the http crate. Users can attach any set of Labels to a task or thread, with the only caveat being that there can be at most one Label for a given type T. This is not too onerous a restriction, since one can use the common newtype idiom to easily work around this.

For tracing, we also provide a newtype TaskName that can be converted to and from a String. If the TaskName label is set for a task, tracing output will show the TaskName (in addition to the TaskId) to make logs easier to read.

Since the current functionality provided by Tag is superseded by Labels, we also mark Tag as deprecated. They will be removed in a future release.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

src/current.rs Outdated Show resolved Hide resolved
src/runtime/task/mod.rs Outdated Show resolved Hide resolved
src/runtime/task/mod.rs Outdated Show resolved Hide resolved
src/runtime/task/mod.rs Show resolved Hide resolved
tests/basic/labels.rs Outdated Show resolved Hide resolved
tests/basic/labels.rs Outdated Show resolved Hide resolved
tests/basic/labels.rs Outdated Show resolved Hide resolved
@sarsko
Copy link
Contributor

sarsko commented Mar 4, 2024

(discussed with Rajeev, posting here to not forget it / have an entry where it may be discussed)

One thing which is lost from Labels vs Tags is the ability to gain more information than a simple string in the tracing output from Shuttle. There is a question of whether there should be the possibility to have more information logged, for instance by exposing a way to provide a closure with a type signature ala &[Label] -> Debug.

@sarsko
Copy link
Contributor

sarsko commented Mar 4, 2024

Send + Sync bound can be removed: 9822138

@jorajeev jorajeev force-pushed the add-labels branch 2 times, most recently from 39df4fe to 1d64b00 Compare March 5, 2024 01:31
@jorajeev
Copy link
Member Author

jorajeev commented Mar 5, 2024

Summary of changes since the first reviewed version:

  • renamed LabelFn -> ChildLabelFn, and changed the function signature to Fn(TaskId, &mut Labels), because there are situations where the TaskId is useful
  • removed Send + Sync requirement for types usable as Labels
  • removed the clearing of all Labels at the beginning of an execution (the clearing is already done in ExecutionState::cleanup)

@jorajeev jorajeev force-pushed the add-labels branch 2 times, most recently from 467b7b5 to 981d5e9 Compare March 5, 2024 14:57
Copy link
Member

@jamesbornholt jamesbornholt left a comment

Choose a reason for hiding this comment

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

just docs nits

src/current.rs Outdated Show resolved Hide resolved
src/runtime/task/labels.rs Show resolved Hide resolved
src/current.rs Show resolved Hide resolved
src/runtime/task/mod.rs Show resolved Hide resolved
src/current.rs Show resolved Hide resolved
src/runtime/task/mod.rs Show resolved Hide resolved
[PR98](awslabs#98) added support for associated a single untyped `Tag`
with each task and thread.  As we've gained experience with Tags, we've increasingly felt a need
to have a mechanism that is both better typed, and allows more than one tag to be associated with tasks.
This commit introduces `Labels`, which are inspired by `Extensions` in [the http crate](https://docs.rs/http/latest/http/struct.Extensions.html).
Users can attach any set of Labels to a task or thread, with the only caveat being that there can be
at most one Label for a given type T.  This is not too onerous a restriction, since one can use the
common [newtype idiom](https://doc.rust-lang.org/rust-by-example/generics/new_types.html) to easily
work around this.

For tracing, we also provide a newtype `TaskName` that can be converted to and from a `String`.
If the `TaskName` label is set for a task, tracing output will show the `TaskName` (in addition
to the `TaskId`) to make logs easier to read.

Since the current functionality provided by `Tag` is superseded by `Labels`, we also mark
`Tag` as deprecated.  They will be removed in a future release.
@jorajeev jorajeev merged commit 8886b1c into awslabs:main Mar 7, 2024
5 checks passed
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.

3 participants