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

fix(subscriber): unset subscriber in aggregator #169

Closed
wants to merge 7 commits into from

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Sep 30, 2021

This changes the spawned aggregator task in console_subscriber to
override the default tracing subscriber with NoSubscriber, which
does nothing. This means that any events that occur inside the aggregator
task will not be recorded. When instrumentation that the console
aggregates is triggered inside the aggregator, it should therefore not
cause more data to be recorded.

Signed-off-by: Eliza Weisman eliza@buoyant.io

@hawkw hawkw requested a review from zaharidichev September 30, 2021 17:49
hawkw added 7 commits November 4, 2021 11:08
This changes the spawned aggregator task in `console_subscriber` to
override the default `tracing` subscriber with `NoSubscriber`, which
 does nothing. This means that any events that occur inside the aggregator
task will _not_ be recorded. When instrumentation that the console
aggregates is triggered inside the aggregator, it should therefore not
cause more data to be recorded.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
it will now never be recorded

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw force-pushed the eliza/no-subscriber-aggregator branch from eb762cc to fc87f5c Compare November 4, 2021 18:13
@hawkw hawkw force-pushed the main branch 2 times, most recently from 802ee42 to c3a7660 Compare December 17, 2021 00:56
@hawkw
Copy link
Member Author

hawkw commented Apr 12, 2022

I think this work was obsoleted by #314

@hawkw hawkw closed this Apr 12, 2022
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.

1 participant