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

Update stream documentation bringing explanation of some errors #6995

Merged
merged 17 commits into from Jan 27, 2020

Conversation

thiagoftsm
Copy link
Contributor

@thiagoftsm thiagoftsm commented Oct 4, 2019

Summary

Fixes #6946

We are having reports from users about "bugs" in Netdata stream, but while we were helping them we are are verifying that the majority of these errors are bad configuration, this PR has the goal to clarify for the user possible errors in the stream.

Component Name

Stream documentation

Additional Information

Update documentation for stream to clarify the users some errors
@squash-labs
Copy link

squash-labs bot commented Oct 4, 2019

Manage this branch in Squash

Test this branch here: https://thiagoftsmstream-doc-7iejc.squash.io

Explain more errors that can happen on master
Copy link
Contributor

@cosmix cosmix left a comment

Choose a reason for hiding this comment

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

@joelhans please review and improve the text. There's several parts that could use some English-language love. Thanks!


### Connection between slave and master is slow

When we have a slow connection between master and `slave`, Netdata can raise different errors related to this problem,
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are going to use backticks for slave, you might as well use them for master too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the beginning I was writing backticks to call attention of the people for both master and slave, but when I began to review I saw that this was not a pattern, after this I began to remove, but I forgot this one. Thank you!


where the value X, Y, Z and W are integer numbers.

in the master side the we can have different reports when the master reads everything from the stream and it did not
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the error, thank you again!

@ilyam8
Copy link
Member

ilyam8 commented Oct 5, 2019

I have a feeling that the correct solution is to make errors clear and descriptive. In that case we dont need to maintain page with errors explanations and keep it up to date. @thiagoftsm think about it. Just a suggestion.

@thiagoftsm
Copy link
Contributor Author

Thank you @cosmix and @ilyam8 , I will work on this tomorrow again, I wanna bring more some more errors and I will try to improve the text, before @joelhans touches it on Monday.

This commit fixes errors reported by @cosmix
This commit brings more fixes for the text and description for more errors
Copy link
Contributor

@joelhans joelhans left a comment

Choose a reason for hiding this comment

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

Some requests here and one question about the nature of the error.

streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
This commit brings all the fixes suggested by Joel
This commit brings the last fix suggested by Joel
I missed a message format in the previous sprint
This commit fix the errors reported by Joel
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
streaming/README.md Outdated Show resolved Hide resolved
This commit brings part of the fixes requested from Christopher
The previous explanation for these two features were not good enough,
so I rewrite it to avoid confusion
@thiagoftsm thiagoftsm dismissed a stale review via 196300b October 17, 2019 16:59
After to receive the text from Joel, I am bringing all the fix for the new section
on documentation
@thiagoftsm
Copy link
Contributor Author

Thank you very much @joelhans !
I brought all the changes you gave me for the documentation.

Best regards!

@amoss
Copy link
Contributor

amoss commented Oct 31, 2019

I've just worked through this myself to setup a streaming configuration for testing. It seems that there is something missing in the documentation, which is why so many people are struggling when they go through it for the first time. It could be improved with a howto / walkthrough for a minimal example at the top of the document.

Something like:

  • 192.168.1.100 as a master
  • 192.168.1.101 as the first slave
  • 192.168.1.102 as the second slave.

It should show exactly what to type to achieve this setup, and not involve any decisions (i.e. pick all the options to be reasonable for a first-time setup).

Once a user is over the hurdle of getting it working for the first-time the rest of the document works well as reference material:

  • Common problems.
  • Different options and their effects.
  • Ideas for what is possible with more complex setups.

There is currently a forward-reference in the text to the ephemeral nodes section as an example, but it is not simple enough to act as an initial guide and it does not drop down to a step-by-step level of detail showing exactly what to type.

@thiagoftsm
Copy link
Contributor Author

Hi @amoss ,

It was good you write here about the fact our documentation is not clear in some subjects, because we are working to bring Netdata Learning and this is an important characteristic for we work there. @joelhans , please, pay attention in @amoss words for all the course.
We have the sections https://docs.netdata.cloud/streaming/#configuring-the-master and https://docs.netdata.cloud/streaming/#configuring-the-slaves that are responsible to teach everybody how to configure, they are not a how to and probably they are missing parts or they are not clear enough.
I agree that we are missing some images and here I won't put them, but check the list below to verify whether they can help you and our users:

Master:
1 - Firstly run the command uuidgen

uuidgen 
dfc307cc-9b7f-487d-86f5-349199f06004

2 - now it is time to edit your stream.conf:

/etc/netdata/edit-config stream.conf

3 - Finally set the following variables:

# -----------------------------------------------------------------------------
# 2. ON MASTER NETDATA - THE ONE THAT WILL BE RECEIVING METRICS
[dfc307cc-9b7f-487d-86f5-349199f06004]
    enabled = yes
    default history = 3600
    default memory = ram
    health enabled by default = auto
    multiple connections = allow

Now it is necessary to configure your Slave:
4 - We will execute the step 3 on your slave again:

/etc/netdata/edit-config stream.conf

5 - On Slave we work in a different section:

# -----------------------------------------------------------------------------
# 1. ON SLAVE NETDATA - THE ONE THAT WILL BE SENDING METRICS
[stream]
    enabled = yes
    destination = 192.168.1.100
    api key =dfc307cc-9b7f-487d-86f5-349199f06004

The steps for the next slaves are exactly the same.

Let us know whether this steps are helping, because they will help @joelhans and I move in front with documentation and Netdata learning.

@amoss
Copy link
Contributor

amoss commented Oct 31, 2019

Yes, this is good. I think it works well as a walk-through and clarifies what to do.

@thiagoftsm
Copy link
Contributor Author

Yes, this is good. I think it works well as a walk-through and clarifies what to do.

Thank you for the feedback! :)
I will check with @joelhans whether this is a road to use at Netdata Learning or we can bring something for our official documentation too.

@joelhans joelhans added this to In progress in Documentation Dec 2, 2019
@cosmix
Copy link
Contributor

cosmix commented Jan 17, 2020

Why is this still open? Is there anything more to do on this PR before it gets accepted?

@thiagoftsm
Copy link
Contributor Author

Why is this still open? Is there anything more to do on this PR before it gets accepted?

We are missing reviews, there is not any necessity to add more info in this PR.

@cakrit
Copy link
Contributor

cakrit commented Jan 18, 2020

@joelhans plz review again, it looks good from my side.

@joelhans joelhans added this to the Jan 24 - Jan 30 milestone Jan 24, 2020
@thiagoftsm thiagoftsm merged commit cb7f19b into netdata:master Jan 27, 2020
Documentation automation moved this from In progress to Done Jan 27, 2020
Saruspete pushed a commit to Saruspete/netdata that referenced this pull request May 21, 2020
…ata#6995)

* stream_doc: Update documentation

Update documentation for stream to clarify the users some errors

* stream_doc: Update documentation 2

Explain more errors that can happen on master

* Fix errors reported

This commit fixes errors reported by @cosmix

* Fix and more errors

This commit brings more fixes for the text and description for more errors

* stream_doc: Gramatical fixes

This commit brings all the fixes suggested by Joel

* stream_doc: Gramatical fixes

This commit brings the last fix suggested by Joel

* stream_doc: Message format

I missed a message format in the previous sprint

* Fix doc:

This commit fix the errors reported by Joel

* Fix test

This commit brings part of the fixes requested from Christopher

* New explanation:

The previous explanation for these two features were not good enough,
so I rewrite it to avoid confusion

* stream_doc: Fix text

After to receive the text from Joel, I am bringing all the fix for the new section
on documentation
@thiagoftsm thiagoftsm deleted the stream_doc branch July 8, 2020 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Documentation
  
Done
Development

Successfully merging this pull request may close these issues.

Slaves not connecting to master
7 participants