See www.zabbix.com for the official Zabbix site.

Docs/specs/development guidelines

From Zabbix.org
Jump to: navigation, search

Version control

Zabbix uses Git with Bitbucket for version control - https://git.zabbix.com/

Git Setup

  • Git configuration parameters user.name and user.email must be set to values matching user account in JIRA.
  • Git configuration parameter push.default must be "simple".
  • Git configuration paremeter core.eol must be either unset or "native".

Branches

  • All changes should be first made in a development (feature) branch, which must be created in feature/* from an original branch, e.g. release/4.2 (git checkout -b feature/ZBX-1234-4.2 release/4.2).
  • Trivial fixes (e.g., typos) can be made directly in master and stable branches without independent testing. Developer is fully responsible for such changes. Translations and tests can also be updated without creating a separate feature branch.
  • All feature branch names must consist of the issue number they are solving (including case) followed by minus sign and the version suffix. For instance, ZBXNEXT-1234-4.2.
  • Issues referenced in branch names can only be from ZBX, ZBXNEXT and DEV projects.
  • If work on a feature branch takes a long time then changes from the source branch should be periodically merged into it (in the feature branch: git pull origin release/4.2 --rebase) and conflicts resolved.
  • After work on the feature branch has been completed, source branch must be merged into it (in the feature branch: git pull origin release/4.2 --rebase) and pushed git push before testing and code review may happen.
  • Once the changes are prepared for review and testing the change author must create a Pull Request (PR) and specify the feature branch and release or master branches as PR parameters.
  • After successful testing and code review source branch should be merged into the feature branch again, then PR must be merged back into the source branch (in the original branch release/4.2: git merge --no-ff feature/ZBX-1234-4.2; git push).
  • If required, the changes to the original branch should be cherry-picked into other relevant branches (e.g. master).


Note: It is important to merge the feature branch into the source by merge commit even if a fast-forward merge is possible.


Note: Periodic merging from the source branch and merging from the source branch prior the testing must happen because:
a) it is better to discover and easier to solve conflicts as soon as possible and in smaller batches;
b) when testing, it is not worth wasting time on a development branch which is outdated, compared to the current source branch;
c) changes in the source branch may affect the functionality that is tested in the development branch


  • If there are any conflicts when merging into a feature branch, a commit message should indicate that.
  • If there are any conflicts when merging or cherry-picking into master, someone else should review or test the changes.
  • Feature branches should be removed immediately after they have been merged (git branch -d feature/ZBX-1234; git push origin --delete feature/ZBX-1234).
  • Merge and cherry-pick commits also should have informative commit messages, "cherry-picked commit a1b2c3d" is not sufficient.
  • It's ok to force-push your changes into the remote feature branch unless someone else is working in the same branch.
  • It's strictly prohibited to force-push changes into master or release branches.
  • Main rules to remember when working with Git:
    • all changes from feature branches must be merged into master/releases by merge commits
    • direct changes in master and releases must be fast-forward
    • merged changes in one branch (e.g. release/4.0) must be integrated into other branches (e.g. master, release/4.2) by cherry-picking

Commits

  • Changes should not be pooled in huge commits - instead, multiple smaller commits (with related changes limited to a single commit) are preferred.
    • Code formatting changes should never be pooled together with other changes.
    • If commit contains changes in .scss files, make css command should be called, and updated .css files also should be added to commit. .css files should be regenerated together with each merge commit, where any .scss file were modified.
  • All Git commits should include a short description of what this commit does, not what is some future goal of that particular development branch. Ideally, others should understand the nature of changes without looking at the diff. For example, if some error was caused by off-by-one, commit message should list both what problem it fixes and what was the cause for it.
  • Commit message format for direct changes:
 COMPONENTS [ISSUES] description

Where ISSUES is a list of comma-separated issues, COMPONENTS is a fixed-size list of components affected by this commit (see #Components). Examples:

 A.F.I...S. [ZBX-7105] fixed performance of housekeeper; improved indexes for table events; fixed SQL statements to use the new indexes
 A......... [ZBX-10318,ZBX-10517] added validation of vsize and hsize parameters in screen.create() and screen.update() methods
  • All Git commits should reference at least one JIRA issue so that the commits are visible in the tracker. If a change does not have an open issue in the tracker (e.g., a typo), one of generic issues should be used.
  • All Git commits that fix development bugs should include a list of development bugs fixed.
  • Issues referenced in Git commits can only be from ZBX, ZBXNEXT and DEV projects.
  • When fixing regressions, we should always find the commit(s) that broke that particular feature, so that we know what else could have been broken.

Git ignores, attributes, empty directories

  • .gitignore in Zabbix root folder contains all common directory and file names that must be ignored by Git (such as .deps, configure, Makefile.in etc)
  • .gitattributes in Zabbix root folder sets default handling of line endings as text=auto !eol. This is roughly equivalent to svn:eol-style=native property in Subversion unless Git's configuration parameter core.eol is set to values other than native and doesn't match platform's convention (it's ok if core.eol is unset).
  • .gitattributes also defines files that should be treated as binary:
*.dll binary
*.lib binary
*.exe binary
*.png binary
*.gif binary
*.jpg binary
*.svg binary
*.ico binary
*.jar binary
*.wav binary
*.ttf binary
*.ai binary
  • Git is technically incapable of storing empty directories. Put .gitignore file into empty directory to work around this limitation:
# Ignore everything in this directory
*
# Except this file
!.gitignore

Filetype specific notes

SVG

Files that have been edited with Inkscape may contain sensitive information (inkscape:export-filename attribute may contain full path exposing directory structure, username etc - see Inkscape bugreport) or useless information in attributes inkscape:export-xdpi and inkscape:export-ydpi. These attributes should be removed before committing svg files.

In the Zabbix repository, there's misc/images/strip_inkscape_attributes.xslt file that allows to easily strip those attributes.

Issue tracker

Zabbix uses Jira for issue tracking - https://support.zabbix.com/.

Working with Jira

Creating an issue

  • The ZBX and ZBXNEXT projects are for user visible bug fixes and enhancements.
  • ZBX must be used for bugs only and ZBXNEXT for new features, refactoring and other non-bug related changes.
  • Use DEV project for tests (e.g. Cmocka or Selenium tests) and infrastructure-related code changes.
  • When testing issues, usually it should be noted which version or commit hash was used and whether the issue was reproducible (still there) or not reproducible (works as expected).
  • "Fix Version/s" field is used for two purposes:
    • It may be used to denote when the issue is planned to be fixed - that would be used for open issues with an unreleased version set here,
    • When an issue is fixed (and closed).

Working with an issue

  • If a problem is not reproducible in latest versions, a reasonable effort should be made to discover which version provided the fix (as a minimum, searching the changelog). If version that fixed the issue has been found, it is set as the "Fix Version/s" version. If it can not be found, no version should be set there, and comment should explain the decision. If some older version is not available as "Fix Version/s" to add or delete them (they are "archived" already), ask Sasha to unarchive them temporary.
  • If the problem is not reproducible, but it is not clear whether it has been fixed (for example, it might be suspected that some configuration in the reporter's system exposes the problem), more information should be asked from the reporter and issue should be set to "Need info" state. If the required information is provided, it should be changed away from "Need info" state.
  • Workflow:
    • Person who starts to work on an issue assigns that issue to himself. It is highly recommended to watch the issue. That would allow the developer to respond to comments or reopening of the issue by users later.
    • When a developer starts working on an issue, it is usually known which versions will be fixed. It is suggested to set "Fix Version/s" field to appropriate values as soon as possible - it reduces the possibility of some issue being neglected and makes planning for releases easier.
    • For issues that change at least one frontend file, a sub-issue must be created before marking the issue as resolved. This sub-issue must list all changed, removed and added translatable strings. Special attention should be paid to changed strings instead of simply listing them as removal of one and adding of another string. If no translation string changes happened in this issue, sub-issue must explicitly say that.
    • When development is finished, issue is marked as "Resolved" and a comment with a feature branch name and a link to PR is added, e. g.: Fixed in development branch [feature/ZBX-1234-4.2|https://git.zabbix.com/projects/ZBX/repos/zabbix/pull-requests/36/overview]. This will display a branch name linking to the corresponding Bitbucket PR in a JIRA comment.
    • "Resolved" issues are processed by tester.
  • Issues found when testing the fix:
    • When tester starts working on an issue he assigns it to himself.
    • A problem or remark during a code review or testing is called sub-issue. For each sub-issue a separate comment is added and is edited when working with it.
    • These comments are always visible to everybody unless there is some reason to keep them visible to developers only (internal information, non-English comments). If possible, maximum amount of information should be in a public comment and the internal information is added as a separate comment with limited access permissions.
    • Sub-issues are enumerated starting from (1).
    • Sub-issue number can be followed by component (optional).
    • Sub-issue comments follow a dialog style. When a developer resolves a sub-issue and commits the fix to Git, a corresponding comment is edited and the text with a nickname, "RESOLVED" string, commit hash and optional description is appended. When tester considers the sub-issue to be fixed, "CLOSED" string is added. Additional comments are allowed and encouraged.
    • Every sub-issue dialog must end with "CLOSED".
    • There might be a case when closed sub-issue needs to be reopened. In this case use "REOPENED" string.
  • If additional fixes/changes are being done, they are added as sub-issues.

An example of sub-issue dialog:

(1) [GUI] During the fix we broke feature X. This is very sad.

[~developer] Oops, sorry. RESOLVED in a1b2c3d

[~tester] Now feature Y is broken. See, if we do A, then hide B by clicking on C, we are
left with D being on top of E. Considering that F should not be there either, this is not good.

[~developer] Ouch, fixed A handling in B. RESOLVED in 2a3b4cd, 1b2c3de

[~tester] CLOSED

An example of sub-issue dialog with 2 testers:

(3) There is no error message when host becomes unavailable.

[~developer] Right. RESOLVED in 2a3b4cd

[~tester-1] CLOSED

[~tester-2] There is a typo in the error message "canot". REOPENED

[~developer] RESOLVED in 1b2c3de

[~tester-2] Perfect. CLOSED
Checking for changed translation strings

An easy way to check for translation string changes is to run update_po.sh in the locale directory. Then a simple script may be used:

#!/bin/bash
lang="${1:-sk}"
git diff "$lang" | grep -v "^[-+]#:\|^[-+]\"POT-Creation-Date\|[-+]\{3\} $lang/" | grep "^[-+]"

This defaults to checking the sk locale, but you can manually choose any other. It lists changed msgid and msgstr lines (thus also showing whether removed strings were translated in this particular locale).

After checking these changes should be reverted, as this all happens before testing which is likely to cause additional changes.

Testing an issue

  • When tester accepts the fix he marks an issue as "MERGING" (passed testing successfully) and assigns back to developer.
  • Developer makes sure that the JIRA issue has proper status and then merges development branch into upstream.

Documenting changes

  • If the change requires to update the documentation, the developer marks the issue as "Needs documenting" and assigns it to whoever is responsible for making the changes. The developer may also update the documentation himself.
  • When the changes have been made, the author of the changes sets the status to "Documented" and asks somebody to review.
  • If the issue doesn't need any documentation updates, these steps can be skipped.
  • After the documentation has been updated and reviewed, the issue can be closed.
What should be documented where

Developer should try to think about all the possible implications of a change - what pages change, which labels change, should any screenshots be updated.

Many changes also have to be noted in the "what's new" and "upgrade notes" pages. When deciding what should be documented where, this example might help:

  • "what's new" is to note everything that can be touted as a feature or improvement in any way. Those pages are suggested for users to read, but if they only want to keep on using existing functionality, they can skip it.
  • "upgrade notes" are for noting things that users should know when upgrading, as they change existing functionality or behaviour in some way.

For example, in 4.2.0 Zabbix sender started using all addresses found in ServerActive parameter. This is clearly an improvement over previous versions and should be put in "What's new". But this change requires responses to be presented in a different way by indicating what server the response is from. Users upgrading from earlier versions must be aware of that and make appropriate changes in their scripts if needed. The fact Zabbix sender changed output should be documented in "Upgrade notes".

Closing an issue

  • An issue may be closed after all sub-issues have been CLOSED and the required changes to the documentation have been made.
  • When closing the issue, a comment must indicate a revision and Zabbix version(s) that will contain the fix, e. g.:Fixed in pre-4.0.8 a1b2c3d, pre-4.2.2 1b2c3de
  • "Fix Version/s" value/s should be reviewed and are set to versions where this issue was fixed. For example, if it was only fixed in master, "Fix Version/s" could be 4.4.0. If it was fixed in several branches, all of them should be present in the "Fix Version/s" field (for example, 4.0.8, 4.2.2). Incorrect value, if any, must be removed.
  • Issues closed with any Resolution except "Fixed" have to have empty value of Fix Version/s. Without exceptions.
  • Issues closed with the Resolution as "Duplicate" should have linked duplicated issue. It is suggested to add a comment "closed as a duplicate" or similar - it makes it easier to follow who closed the issue.

Components

In Jira, the following components are available. The letter in parenthesis match with letters in changelog entries and Git commit messages.

  • API (A)
  • Documentation (D)
  • Frontend (F)
  • Agent (G)
  • Installation (I)
  • Java gateway (J)
  • Appliance (L)
  • Proxy (P)
  • Server (S)
  • Templates (T)

Documentation includes manpages and changelog fixes. Installation includes initial database schema and data, autotool related changes and other similar issues. Templates deal with the templates that are shipped with Zabbix.

Changelog and commit messages

Each changelog entry and commit message since Zabbix 1.8.10 and 1.9.9 starts with a string that identifies components, changed in that commit or issue. It is a 10 character string (one per component) with component letter for each component that was changed, empty spaces filled with dots. Commit messages include only things that were changed in the commit itself, changelog entry includes all components that were changed regarding that issue.

Identification letters are always positionally placed like this:

 ADFGIJLPST

For example, API and Frontend would be:

 A.F.......

Server and proxy would be:

 .......PS.

Commit messages that do not actually change any parts from the above (for example, merges, unit test edits) have all components unset:

 ..........

ChangeLog

  • After the component, all changelog entries must have "[ISSUE] ", where ISSUE is issue number on the tracker (like "ZBX-3")
    • When referencing multiple issues in a single changelog entry, a format of [ZBX-1,ZBX-2] must be used.
    • Issues referenced in changelog entries can only be from ZBX and ZBXNEXT projects.
  • All user visible changes between two versions should be listed in the changelog. Thus if a problem appeared after the last release and was fixed before the next, it must not be added to the changelog.
  • Changelog entries should describe the fix from user perspective. Instead of "fixed off by one in some_function", "fixed inability to delete last user macro in host properties" is preferred.
    • Changelog entries should be short summaries of changes done, not exact copies of issue summaries.
  • Changelog entries are separated in new features and improvements, and bugfixes. An example:
 Changes for 4.2.0rc2
 
 New features:
 ..F....... [ZBXNEXT-5109] implemented test value saving before test sessions in preprocessing test forms (miks)
 ..F.....S. [ZBXNEXT-1238,ZBXNEXT-4988] added ability to test media types from UI (Ivo, vso)
 ..F....... [ZBXNEXT-5114] improved multiline input control (ashubin)
 ...G...... [ZBXNEXT-5123] added server address and port to the Zabbix sender output (wiper)
 
 Bug fixes:
 ..F....... [ZBX-15931] fixed triggers filter for php 5.4 (talbergs)
 ..F....... [ZBX-15839] fixed broken layout of the breadcrumbs in Safari browser (ashubin)
 ........S. [ZBX-15867] fixed invalid DNS being accepted when receiving discovery contents from Zabbix proxy (vso)
 A......... [ZBX-15821] fixed SQL statement performance used in template.unlink() method (Sasha)
  • If several people worked on the issue, they must be listed in alphabetical order. For instance, see "Ivo, vso" above.
    • No spaces are used in 1.8 branch
    • Starting with 1.9, spaces must be used - for example, (Eduard, Oleg, Toms)
  • Whenever possible, submitters of patches and translations should be acknowledged. This is done by including a string like "; thanks to John Smith" in the changelog entry before the developer's name:
 ....I..... [ZBXNEXT-4841] removed hardcoded location for iconv.h; thanks to Helmut Grohne for the patch (kalimulin)
  • All entries must start with a lower case letter and in the past form (e.g. "..F....... [ZBX-3934] fixed ...").
  • Double quotes must be used for all quoting in the log.
  • If a bug fix is not merged to master, the corresponding ChangeLog entry must be added to the master ChangeLog anyway. If a bug is fixed both in stable branch and master, ChangeLog entry must be added both for stable and master version. For example, if a bug is fixed only in 4.2.1, changelog entry is added to release/4.2 branch ChangeLog and to the master ChangeLog - in 4.2.1 section. If a bug is fixed both for 4.2.1 and 4.0.7, changelog entry appears twice in the master ChangeLog, both for 4.2.1 and 4.0.7.

API

Deprecating a feature

To preserve backward compatibility when removing an API feature we first deprecate it till the next major release and only then remove it. The following needs to be done when deprecating an API feature:

  • Usage of deprecated API features must be removed from the frontend. To avoid possible errors, the API should trigger PHP notifications when such features are used.
  • The deprecated feature must be noted in the API changelog.
  • A notice must be added in the documentation of the feature. Some examples:

Database

Changing database schema

When there is a need to change database schema and create a database patch, the following changes are necessary:

  1. change database schema in create/src/schema.tmpl:
    +FIELD		|max_columns	|t_integer	|'3'	|NOT NULL	|0
    
  2. add a patch for every little change in create/src/schema.tmpl to src/libs/zbxdbupgrade/dbupgrade_XXXX.c (each patch should be as atomic as possible and make minimal changes to the database), where XXXX denotes the major and minor version the patch is developed for (e.g., src/libs/zbxdbupgrade/dbupgrade_2030.c for 2.3.x development):
    • name function that applies the patch DBpatch_XXXXYYY(), where XXXX is the version specifier described above and YYY is a sequential identifier:
      static int DBpatch_2030118(void) ...
      
    • add code to this function that changes database schema according to changes in #1:
      const ZBX_FIELD	field = {"max_columns", "3", NULL, NULL, 0, ZBX_TYPE_INT, ZBX_NOTNULL, 0}; return DBadd_field("screens_items", &field);
      
    • add this function to the list of patches using DBPATCH_ADD() macro:
      DBPATCH_ADD(2030118, 0, 1)
      
  3. update default row in dbversion table in create/src/schema.tmpl:
    ROW|2030118|2030118
    
  4. change create/src/data.tmpl, if necessary:
    vim create/src/data.tmpl
    
  5. execute the following command to update server's schema code:
    make dbschema
    
  6. make sure both template files can be imported without errors:
    cat database/mysql/{schema,images,data}.sql | mysql -u root zabbix
    
  7. recompile and run the server, and make sure the database is upgraded correctly
    make install; cd ...; sbin/zabbix_server
    
  8. update ZABBIX_DB_VERSION macro in frontends/php/include/defines.inc.php (only if mandatory database version has been changed):
    define('ZABBIX_DB_VERSION', 2030118);
    
  9. update frontends/php/include/schema.inc.php file with the new schema:
    $ php create/bin/gen_php.php && mv create/src/schema.inc.php frontends/php/include/
    

Synchronising translations

See Synchronising translations page for detailed instructions on translation maintenance.