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 #4995

Open
AndreiEres opened this issue Jul 10, 2024 · 2 comments · May be fixed by #5875
Open

Remove jaeger #4995

AndreiEres opened this issue Jul 10, 2024 · 2 comments · May be fixed by #5875
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network.

Comments

@AndreiEres
Copy link
Contributor

In the past, jaeger spans were used for 2 purposes:

  • debugging (nested spans across multiple nodes seemed to be useful)
  • coarse-grained profiling

Now that we have better tools for all of them, and seems like the need for spans no longer exists.

So now jaeger is providing no value with a cpu cost. Along with quite big code blocks which make it harder to understand the logic in the code.

Also we can consider removing gum tracing which is a wrapping around tracing integrated with jaeger.

@AndreiEres AndreiEres added T0-node This PR/Issue is related to the topic “node”. I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Jul 10, 2024
@skunert
Copy link
Contributor

skunert commented Jul 10, 2024

Now that we have better tools for all of them

What tools are you refering to here?

I recently thought about how we can make debugging of parachains easier. I saw that we have some jaeger intregration in the codebase but have never used it. Did you use it before, what are the problems?

@sandreim
Copy link
Contributor

The goal of this implementation was to be able to trace candidate progress in parachain consensus and get some idea on how much time is being spent in different stages. You can actually do some of that with some limitations, but the Grafana UX was never great and did not improve the debugging process over what can be achieved the same using just logs and searching for hashes.

@AndreiEres AndreiEres added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Sep 4, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 27, 2024
Jaeger spans were not usable for debugging, see
#4995, but we pay a
price in CPU cost, subsystem-benchmarks show this brings a reduction of
about 10-15% in CPU usage per subsystem, so remove it.

---------

Signed-off-by: Alexandru Gheorghe <[email protected]>
@alexggh alexggh linked a pull request Sep 30, 2024 that will close this issue
github-merge-queue bot pushed a commit that referenced this issue 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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants