chore: extends single-node.sh script to support local tracing and traced data retrieval #3496

staheri14 · 2024-05-17T19:44:49Z

This PR extends the single-node.sh script by providing an option for the user to enable local tracing and retrieve traced data.

How to test

Run single-node.sh under the scripts folder
Respond y to Do you want to set up local tracing with the ability to pull traced data? [y/n]
Once the node starts, navigate to ./celestia-app/data/traces folder and you will find the locally traced files.

Also, the trace package from celestia-core has a method to pull the traced data which can be found here.

…onfig-single-node-script

coderabbitai · 2024-05-17T19:46:13Z

Walkthrough

The single-node.sh script has been enhanced to include a new feature that prompts users to set up local tracing. This update modifies the config.toml configuration file to enable tracing and allows users to pull traced data if desired. The script provides feedback to ensure users are aware of the setup status.

Changes

File	Change Summary
`scripts/single-node.sh`	Added user prompt and configuration updates for enabling local tracing and pulling traced data.

Recent Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 33f818b and 3982f88.

Files selected for processing (1)

scripts/single-node.sh (1 hunks)

Additional Context Used

ShellCheck (4)

scripts/single-node.sh (4)

[warning] 97-97: In POSIX sh, [[ ]] is undefined.

[info] 99-99: Double quote to prevent globbing and word splitting.

[info] 101-101: Double quote to prevent globbing and word splitting.

[info] 103-103: Double quote to prevent globbing and word splitting.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Note: Auto-reply has been disabled for this repository by the repository owner. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

rootulp

silly question but how do I pull the traced data? Do I need to have Grafana + Prometheus running locally and configure it to fetch from http://127.0.0.1:26661?

[not blocking] but it would be cool if we had a script (perhaps in this one or a separate one) to launch a local Grafana + Prometheus that is already configured to fetch traced data from that endpoint. Alternatively, if we can use Grafana Cloud and not run Grafana + Prometheus locally, it would be nice if there was a step or screenshot on how to configure Grafana Cloud to read traced data from the single node running via this script.

scripts/single-node.sh

staheri14 · 2024-05-17T19:56:57Z

silly question but how do I pull the traced data? Do I need to have Grafana + Prometheus running locally and configure it to fetch from http://127.0.0.1:26661?

[not blocking] but it would be cool if we had a script (perhaps in this one or a separate one) to launch a local Grafana + Prometheus that is already configured to fetch traced data from that endpoint. Alternatively, if we can use Grafana Cloud and not run Grafana + Prometheus locally, it would be nice if there was a step or screenshot on how to configure Grafana Cloud to read traced data from the single node running via this script.

Sure, I'll create an issue for this one, but, in the meanwhile, I have updated the PR description and explained how to locate the local traced data.

coderabbitai

Actionable comments posted: 3

coderabbitai · 2024-05-17T20:00:56Z

scripts/single-node.sh

+echo "Do you want to set up local tracing with the ability to pull traced data? [y/n]"
+read -r response
+if [[ $response == "y" ]]; then
+  trace_type="local"
+  sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml
+  trace_pull_address=":26661"
+  sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" ${CELESTIA_APP_HOME}/config/config.toml
+  trace_push_batch_size=1000
+  sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" ${CELESTIA_APP_HOME}/config/config.toml
+  echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}"
+fi


Consider replacing [[ ]] with [ ] for POSIX compliance.

- if [[ $response == "y" ]]; then + if [ "$response" = "y" ]; then

This change ensures that the script remains compatible with all POSIX-compliant shells, as [[ ]] is not defined in standard POSIX sh.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

echo "Do you want to set up local tracing with the ability to pull traced data? [y/n]"

read -r response

if [[ $response == "y" ]]; then

trace_type="local"

sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml

trace_pull_address=":26661"

sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" ${CELESTIA_APP_HOME}/config/config.toml

trace_push_batch_size=1000

sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" ${CELESTIA_APP_HOME}/config/config.toml

echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}"

fi

echo "Do you want to set up local tracing with the ability to pull traced data? [y/n]"

read -r response

if [ "$response" = "y" ]; then

trace_type="local"

sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml

trace_pull_address=":26661"

sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" ${CELESTIA_APP_HOME}/config/config.toml

trace_push_batch_size=1000

sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" ${CELESTIA_APP_HOME}/config/config.toml

echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}"

fi

coderabbitai · 2024-05-17T20:00:57Z

scripts/single-node.sh

+read -r response
+if [[ $response == "y" ]]; then
+  trace_type="local"
+  sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml


Ensure variables are properly quoted to prevent word splitting and globbing issues.

- sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml + sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" "${CELESTIA_APP_HOME}/config/config.toml" - sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" ${CELESTIA_APP_HOME}/config/config.toml + sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" "${CELESTIA_APP_HOME}/config/config.toml" - sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" ${CELESTIA_APP_HOME}/config/config.toml + sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" "${CELESTIA_APP_HOME}/config/config.toml"

Quoting the variable ${CELESTIA_APP_HOME} prevents issues if the path contains spaces or special characters.

Also applies to: 101-101, 103-103

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml

sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" "${CELESTIA_APP_HOME}/config/config.toml"

sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" "${CELESTIA_APP_HOME}/config/config.toml"

sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" "${CELESTIA_APP_HOME}/config/config.toml"

coderabbitai · 2024-05-17T20:00:57Z

scripts/single-node.sh

+  sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" ${CELESTIA_APP_HOME}/config/config.toml
+  trace_push_batch_size=1000
+  sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" ${CELESTIA_APP_HOME}/config/config.toml
+  echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}"


Address the typo in the printed address for traced data.

- echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}" + echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1:${trace_pull_address}"

This change corrects the URL format by ensuring the port number is preceded by a colon, which aligns with the standard URL format.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}"

echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1:${trace_pull_address}"

staheri14 · 2024-05-17T20:03:19Z

@rootulp Until I address your question in a separate GitHub PR, I wanted to let you know that the trace package from celestia-core has a method to pull the traced data. You can find it here.

rootulp · 2024-05-17T20:08:07Z

Cool thanks, asked out of curiosity :)

rootulp

The log looks fixed to me: Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1:26661.

Tangent: I would also be okay with always enabling tracing and getting rid of the conditional prompt for y/n

staheri14 · 2024-05-17T21:31:45Z

The log looks fixed to me: Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1:26661.

Tangent: I would also be okay with always enabling tracing and getting rid of the conditional prompt for y/n

The amount of traced data can become quite large, so I think it is prudent to let the user decide (note that local tracing by default is disabled).

staheri14 added 2 commits May 17, 2024 12:40

adds a script for enabling local tracing

e22eeab

provides the full tracing pull address

299b5ac

staheri14 self-assigned this May 17, 2024

staheri14 added chore optional label for items that follow the `chore` conventional commit WS: Big Blonks 🔭 Improving consensus critical gossiping protocols labels May 17, 2024

Merge remote-tracking branch 'origin/main' into sanaz/local-tracing-c…

0c7a759

…onfig-single-node-script

staheri14 marked this pull request as ready for review May 17, 2024 19:46

staheri14 requested a review from a team as a code owner May 17, 2024 19:46

staheri14 requested review from rootulp and cmwaters and removed request for a team May 17, 2024 19:46

staheri14 requested review from evan-forbes, rach-id and ninabarbakadze May 17, 2024 19:46

rootulp previously approved these changes May 17, 2024

View reviewed changes

scripts/single-node.sh Outdated Show resolved Hide resolved

removes excess colon

3982f88

staheri14 dismissed rootulp’s stale review via 3982f88 May 17, 2024 19:57

celestia-bot requested a review from a team May 17, 2024 19:57

coderabbitai bot reviewed May 17, 2024

View reviewed changes

staheri14 requested a review from rootulp May 17, 2024 20:05

staheri14 mentioned this pull request May 17, 2024

chore: explain how to pull the traced data #3497

Open

rootulp approved these changes May 17, 2024

View reviewed changes

rach-id approved these changes May 17, 2024

View reviewed changes

staheri14 merged commit 475b979 into main May 17, 2024
34 checks passed

staheri14 deleted the sanaz/local-tracing-config-single-node-script branch May 17, 2024 21:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: extends single-node.sh script to support local tracing and traced data retrieval #3496

chore: extends single-node.sh script to support local tracing and traced data retrieval #3496

staheri14 commented May 17, 2024 •

edited

Loading

coderabbitai bot commented May 17, 2024 •

edited

Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

rootulp left a comment

staheri14 commented May 17, 2024

coderabbitai bot left a comment

coderabbitai bot May 17, 2024

coderabbitai bot May 17, 2024

coderabbitai bot May 17, 2024

staheri14 commented May 17, 2024 •

edited

Loading

rootulp commented May 17, 2024

rootulp left a comment

staheri14 commented May 17, 2024

-  sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" ${CELESTIA_APP_HOME}/config/config.toml
+  sed -i.bak -e "s/^trace_type *=.*/trace_type = \"$trace_type\"/" "${CELESTIA_APP_HOME}/config/config.toml"
+  sed -i.bak -e "s/^trace_pull_address *=.*/trace_pull_address = \"$trace_pull_address\"/" "${CELESTIA_APP_HOME}/config/config.toml"
+  sed -i.bak -e "s/^trace_push_batch_size *=.*/trace_push_batch_size = \"$trace_push_batch_size\"/" "${CELESTIA_APP_HOME}/config/config.toml"

	echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1${trace_pull_address}"
	echo "Tracing is set up with the ability to pull traced data from the node on the address http://127.0.0.1:${trace_pull_address}"

chore: extends single-node.sh script to support local tracing and traced data retrieval #3496

chore: extends single-node.sh script to support local tracing and traced data retrieval #3496

Conversation

staheri14 commented May 17, 2024 • edited Loading

coderabbitai bot commented May 17, 2024 • edited Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

rootulp left a comment

Choose a reason for hiding this comment

staheri14 commented May 17, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot May 17, 2024

Choose a reason for hiding this comment

coderabbitai bot May 17, 2024

Choose a reason for hiding this comment

coderabbitai bot May 17, 2024

Choose a reason for hiding this comment

staheri14 commented May 17, 2024 • edited Loading

rootulp commented May 17, 2024

rootulp left a comment

Choose a reason for hiding this comment

staheri14 commented May 17, 2024

staheri14 commented May 17, 2024 •

edited

Loading

coderabbitai bot commented May 17, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

staheri14 commented May 17, 2024 •

edited

Loading