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

Feature/doxygen #383

Closed
wants to merge 89 commits into from
Closed

Feature/doxygen #383

wants to merge 89 commits into from

Conversation

simonnagl
Copy link

@simonnagl simonnagl commented May 6, 2016

Added doxygen-style documentation to proc_self_mountinfo.h
This should not be merged as is.

Things to be done before we can merge this:

  • Include generation of the doc to into the build process.
  • Document at least all header files.

To view the documentation these steps are needed:

  1. Change dir to project root.
  2. doxygen doxygen.conf
  3. View doc/html/index.html to see the documentation of the file.

@simonnagl
Copy link
Author

Commented appconfig.h. @ktsaou Please tell me if you like this kind of documentation. If you like it I would document all the header files like this. I think this makes a enormous step in code quality and can help new contributors.

If you like it also comment if I made some mistakes and comment the lines I requested your help. Also tell me if you think it is to much work for the benefit.

@ktsaou
Copy link
Member

ktsaou commented Jan 11, 2017

@simonnagl nice work.

Include generation of the doc to into the build process.

Keep in mind I would like this to be opt-in, not opt-out.

@simonnagl
Copy link
Author

simonnagl commented Jan 11, 2017

I have added comments to commit beb13dc

I have read them and updated the comments. Thank you for that

Keep in mind I would like this to be opt-in, not opt-out.

This was my intention too.

@ktsaou
Copy link
Member

ktsaou commented Jan 11, 2017

perfect!

@simonnagl
Copy link
Author

Documented avl.h. @ktsaou There I also need your help. Do you want me to always notify you if there is something new your help is requested or do you want to get it bundled after I finished all header files.

Now doxygen is included into the build chain. The Output format of doxygen can be configured. For example --disable-doxygen-pdf for a complete list run ./configure --help

There are some new make targets:

  • doxygen-doc: Generate all doxygen documentation.
  • doxygen-run: Run doxygen, which will generate some of the documentation (HTML, CHM, CHI, MAN, RTF, XML) but will not do the post processing required for the rest of it (PS, PDF).
  • doxygen-ps: Generate doxygen PostScript documentation.
  • doxygen-pdf: Generate doxygen PDF documentation.

Now there is one question to discuss. Do we want to add an option --generate-doxygen to the netdata installer? Where should we document this feature?

.gitignore Outdated
@@ -83,6 +83,7 @@ CMakeFiles/
cmake_install.cmake

.jetbrains*
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

oops!

@ktsaou
Copy link
Member

ktsaou commented Jan 14, 2017

sorry.. I broke this...

@simonnagl
Copy link
Author

No problem. Rebased to master. Your doing real good work!

@simonnagl
Copy link
Author

@ktsaou Added some new TODO's where I need your help...

src/avl.h Outdated
void avl_init(avl_tree *t, int (*compar)(void *a, void *b));


/**
* \todo kstou Your help needed.
Copy link
Member

@ktsaou ktsaou Jan 16, 2017

Choose a reason for hiding this comment

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

walk through all entries in the AVL tree. The function int callback(entry, data) will be called for each node (data is the data parameter to the traverse function).

The return value of callback() is interpreted as follows:

  • zero, or positive = add it to total.
  • negative = stop traversing the tree.

If no call to callback returns a negative number, the return value of the traverse function is the total.
If any call to callback returns a negative number, the return value is the number returned by that callback function.

I have never verified, if this calls the callback function in a sorted order (I hope it does, but I never really cared since netdata uses these AVL trees with string hashes - so sorting them based on the hashes is meaningless).

The lock version, locks the tree for the entire walk through (so use it with care).

src/clocks.h Outdated
*
* @see man 2 gettimeofday
*
* @param clk_id Not used. \todo Should we change API here?
Copy link
Member

Choose a reason for hiding this comment

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

this is work in progress. There is PR #1368 with more changes.

src/main.h Outdated
*
* \section sec_outline Outline
*
* \todo
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Author

Choose a reason for hiding this comment

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

This comment block starts with \mainpage.

This will be the initial start page of doxygen. I am not shure anymore if this is the right place to put it because your question mark. The todo is for me.

I think on the homepage of the doxygen documentation we should give an overview where to find wich part of net data. Also there should be additional information about coding rules etc. (Things the user of netdata does not need to no but the contributor)

What do you think?

src/common.h Outdated
extern void strreverse(char* begin, char* end);
/**
* \todo ktsaou, can you please explain whats different to strsep here?
Copy link
Member

Choose a reason for hiding this comment

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

like strsep() but it automatically skips adjusting delimiters (so if the delimiter is a space, it will will skip all spaces).

src/common.h Outdated
extern void freez(void *ptr);
#endif

/**
* \todo ktsaou: your help needed. Especially when this should be used.
Copy link
Member

Choose a reason for hiding this comment

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

When a string is to be added to a JSON object, we need to escape it (i.e. escape double quotes and backslashes). It is like strncpy(), but it escapes the string while copying it.

src/common.h Outdated
extern void json_escape_string(char *dst, const char *src, size_t size);

/**
* \todo ktsaou: your help needed. Especially what is the difference to mmap
Copy link
Member

Choose a reason for hiding this comment

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

  1. it creates the file if does not exist (to the correct size).
  2. it truncates the file if it already exists and is bigger than expected.
  3. sets various memory management heuristics we know netdata memory databases use (like MADV_SEQUENTIAL)
  4. prevents this memory from being copied when netdata forks to execute its plugins.
  5. enables KSM for it.
  6. in memory mode ram, it loads the file into memory (in all other modes, loading it is on-demand by the kernel).

src/common.h Outdated
extern int read_single_number_file(const char *filename, unsigned long long *result);

/// \todo ktsaou: your help needed
Copy link
Member

Choose a reason for hiding this comment

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

haha... this is old... you need to fetch the latest.

To understand simple patterns, check this #1571 (comment)

You can run netdata -W simple-pattern for more help or to play with it...

Copy link
Member

Choose a reason for hiding this comment

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

The general rule is:

  1. The create call builds a linked list (more like a tree) of substrings to be matched, each matched as PREFIX, SUFFIX, SUBSTRING or EXACT.
  2. The match call checks a string against a pattern that has been parsed with create.
  3. The free call frees the memory allocated by create.

The whole mechanism supports both positive and negative matches.

Copy link
Author

Choose a reason for hiding this comment

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

haha... this is old... you need to fetch the latest

This is in the current master... Did I understand you wrong?

src/common.h Outdated
NETDATA_SIMPLE_PATTERN_MODE_SUFFIX, ///< \todo ktsaou: your help needed
NETDATA_SIMPLE_PATTERN_MODE_SUBSTRING ///< \todo ktsaou: your help needed
} NETDATA_SIMPLE_PREFIX_MODE;
typedef void NETDATA_SIMPLE_PATTERN; ///< \todo ktsaou: you help needed
Copy link
Member

Choose a reason for hiding this comment

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

This is a way to prevent exposing the internals of simple patterns to the rest of netdata. So, all the calls accept SIMPLE_PATTERN *, which is actually a void *.

@ktsaou
Copy link
Member

ktsaou commented Jan 16, 2017

I think I answered all. If you find any missing, please let me know.
You need to update your repo...

Copy link
Author

@simonnagl simonnagl left a comment

Choose a reason for hiding this comment

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

Thank you for helping out. I removed @ktsaou from the commit message.

src/main.h Outdated
*
* \section sec_outline Outline
*
* \todo
Copy link
Author

Choose a reason for hiding this comment

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

This comment block starts with \mainpage.

This will be the initial start page of doxygen. I am not shure anymore if this is the right place to put it because your question mark. The todo is for me.

I think on the homepage of the doxygen documentation we should give an overview where to find wich part of net data. Also there should be additional information about coding rules etc. (Things the user of netdata does not need to no but the contributor)

What do you think?

src/common.h Outdated
extern int read_single_number_file(const char *filename, unsigned long long *result);

/// \todo ktsaou: your help needed
Copy link
Author

Choose a reason for hiding this comment

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

haha... this is old... you need to fetch the latest

This is in the current master... Did I understand you wrong?

@ktsaou
Copy link
Member

ktsaou commented Jan 17, 2017

ok. sounds nice.

@simonnagl
Copy link
Author

I went on and documented all structs.

@ktsaou There are some TODOs I nee your help.

volatile uint16_t connected_clients; ///< number of connected clients

volatile uint64_t web_requests; ///< Number of web requests.
volatile uint64_t web_usec; ///< ktsaou: Your help needed
Copy link
Member

Choose a reason for hiding this comment

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

sum of the duration to serve web_requests (from the reception of the request, to the dispatch of the last byte).


volatile uint64_t web_requests; ///< Number of web requests.
volatile uint64_t web_usec; ///< ktsaou: Your help needed
volatile uint64_t web_usec_max; ///< ktsaou: Your help needed
Copy link
Member

Choose a reason for hiding this comment

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

the max time to serve a request. This is reset to zero every time the chart is updated, so it shows the max time for each iteration.

/**
* ktsaou: Your help needed.
*
* @return ktsaou: Your help needed.
Copy link
Author

Choose a reason for hiding this comment

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

I understand that log is something like a DB. Can you please clarify what's the difference between registry_log_load and registry_db_load?

Copy link
Member

Choose a reason for hiding this comment

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

of course.

registry_db is a dump of the registry database in memory.
registry_log is the transaction log, that alters the database in memory.

So, for a variable X in the database that is updated 10 times, you will have 10 rows in registry_log immediately after the value is changed.

When 1.000.000 (configurable) rows are written to registry_log, the database is dumped again in registry_db and registry_log is rotated.

This way, even if netdata crashes, transactions are not lost. When netdata starts, it loads registry_db and then registry_log, to continue from the point it was.


typedef long long total_number;
#define TOTAL_NUMBER_FORMAT "%lld"
typedef long long total_number; ///< ktsaou: Your help needed.
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou Your help needed.

extern RRDSET *rrdset_find_byname(const char *name);

/**
* ktsaou: Your help needed.
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou Your help needed.

/**
* ktsaou: Your help needed.
*
* @param st RRDSET.
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou Your help needed

extern void rrdset_next_usec(RRDSET *st, usec_t microseconds);
/**
* ktsaou: Your help needed.
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou Your help needed

* @param st RRDSET of RRDDIM.
* @param rd RRDDIM.
* @param value to add.
* @return ktsaou: Your help needed. Last interpolated value?
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou: Your help needed

#define GROUP_MIN 2 ///< (a,b) => a>b?b:a
#define GROUP_MAX 3 ///< (a,b) => a>b?a:b
#define GROUP_SUM 4 ///< (a,b) => a+b
#define GROUP_INCREMENTAL_SUM 5 ///< ktsaou: Your help needed.
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou: Your help needed.

/**
* Run all unit tests.
*
* @param delay ktsaou: Your help needed
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou: Your help needed

* Run all unit tests.
*
* @param delay ktsaou: Your help needed
* @param shift kstaou: Your help needed
Copy link
Author

Choose a reason for hiding this comment

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

@ktsaou: Your help

@ktsaou
Copy link
Member

ktsaou commented Feb 22, 2017

@simonnagl I know you wait for me. I need to focus on multi-host support (which unfortunately will break a lot of your changes - sorry...). Give me a few days to finish it.

@simonnagl
Copy link
Author

No problem!

@simonnagl
Copy link
Author

I see many many changes. I'll start rebasing now. I do not know if I can finish this soon.
When I adapted the changes to current master I will notify you.

If you agree even if there are some points left I would like you to merge this. There won't be wrong documentation but missing one. I will change to TODOs instead of ktsaou: Your help needed. These can be done in feature merge requests then.

We can discuss this if it is mergeable again.

@paulfantom
Copy link
Contributor

@cakrit look at this.

@cakrit cakrit self-assigned this Oct 23, 2018
@paulfantom paulfantom added the no changelog Issues which are not going to be added to changelog label Dec 4, 2018
@simonnagl
Copy link
Author

@cakrit If you are interested in this I can adopt this PR for the new directory structure. Just tell me.

@cakrit
Copy link
Contributor

cakrit commented Dec 26, 2018

Thanks @simonnagl. Although I won't be able to look into this in the short term, you're probably right and it won't be possible to proceed at all if it's not adapted to the new structure. I'd be a bit hesitant to take up your time before we've decided to proceed with this though. We'll have a new dev joining in Jan, so I'll discuss it with him first.

@simonnagl
Copy link
Author

I will close this for now. If you want to have this in Netdata please contact me. I am willing to provide a setup with the basic work again.

@simonnagl simonnagl closed this Feb 8, 2019
@simonnagl simonnagl deleted the feature/doxygen branch November 19, 2019 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs no changelog Issues which are not going to be added to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants