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

Remove jaeger everywhere #5875

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

alexggh
Copy link
Contributor

@alexggh alexggh commented Sep 30, 2024

Jaeger tracing went mostly unused and it created bigger problems like wasting CPU or memory leaks, so remove it entirely.

Fixes: #4995

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Ty! ❤️

Next up, we can remove tracing-gum, since it's main motivation was the use of jaeger: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/node/gum/README.md

@bkchr bkchr added the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Sep 30, 2024
Copy link
Contributor

@sandreim sandreim left a comment

Choose a reason for hiding this comment

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

✂️

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@alexggh alexggh mentioned this pull request Oct 1, 2024
@alexggh
Copy link
Contributor Author

alexggh commented Oct 1, 2024

Ty! ❤️

Next up, we can remove tracing-gum, since it's main motivation was the use of jaeger: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/node/gum/README.md

Logged this for now: #5881. I don't think is as simple as replacing gum! with tracing! because we added some extra functionality like gum::warn_if_frequent!

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@alexggh alexggh enabled auto-merge October 1, 2024 10:37
Copy link
Contributor

@AndreiEres AndreiEres left a comment

Choose a reason for hiding this comment

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

Good job!

polkadot/cli/src/command.rs Show resolved Hide resolved
polkadot/node/overseer/src/lib.rs Show resolved Hide resolved
Copy link
Contributor

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

Nice 👍

@alexggh alexggh added this pull request to the merge queue Oct 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 2, 2024
@alexggh alexggh added this pull request to the merge queue Oct 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 2, 2024
@alexggh alexggh added this pull request to the merge queue Oct 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 2, 2024
@alexggh alexggh added this pull request to the merge queue Oct 2, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 2, 2024
Jaeger tracing went mostly unused and it created bigger problems like
wasting CPU or memory leaks, so remove it entirely.

Fixes: #4995

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 2, 2024
@alexggh alexggh enabled auto-merge October 3, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove jaeger
6 participants