The development guide explains where INFO fields are documented but gives no guidance on which section should contain a new field. Add the criteria discussed in #4067: broadly useful metrics go in default sections, stable feature-specific metrics go in documented non-default sections, and changeable internal metrics go in the explicitly requested `debug` section. Also capture the review concern about keeping field counts small: prefer values derivable from existing metrics, and account for monitoring tools collecting `INFO all`. Point contributors to the section selection code. Fixes #4067. Validation: - Checked the guidance against `genInfoSectionDict()` and `genValkeyInfoString()` in `src/server.c`. - Verified against a local server that `INFO` and `INFO default` select the same sections, `commandstats` is omitted by default but included in `INFO all`, and `Debug` appears only when explicitly requested, not in `INFO`, `INFO all`, or `INFO everything`. - `git diff --check` passed. Documentation-only change; no server behavior changes. --------- Signed-off-by: Roy Chen <rchen1386@gmail.com> Signed-off-by: Roy Chen <63842058+chky1386@users.noreply.github.com> Signed-off-by: Madelyn Olson <madelyneolson@gmail.com> Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
6.1 KiB
Valkey development guidelines
This document provides a general overview for writing and designing code for Valkey. During our long development history, we've made a lot of inconsistent decisions, but we strive to get incrementally better.
General Best practices
- Try to limit the number of lines changed in a PR when possible. We do a lot of backporting as a project, and the more lines changed, the higher the chance of having to resolve merge conflicts. Please separate refactoring and functional changes into separate PRs, to make it easier to handle backporting.
- Avoid adding configuration when a feature can be fully controlled by heuristics. We want Valkey to work correctly out of the box without much tuning. Configurations can be added to provide additional tuning of features. When the workload characteristics can't be inferred or imply a tradeoff (CPU vs memory), then provide a configuration.
General style guidelines
Most of the style guidelines are enforced by clang format, but some additional comments are included here.
- C style comments
/* comment */can be used for both single and multi-line comments. C++ comments//can only be used for single line comments. Multi line comments should have the leading*align and the final*/should be on the same line as the last line of text. e.g.
/* Blah Blah
* Blah Blah. */
- Comments should generally be used to describe behavior that is not obvious from reading the code itself. This includes complex behavior, why code was written the way it was and describing non-obvious behavior. Additionally, functions should be documented to explain all of the function's behavior without having to read the code.
- Generally keep line lengths below 90 characters when reasonable, however there is no explicit line length enforcement. Use your best judgement for readability.
- Use static functions when a function is only intended to be accessed from the same file.
For historical reasons, some private functions are prefixed by
_, and they are kept as is to make it easier to backport changes. - Use the boolean type for true/false values. For historical reasons, some functions used the integer type, and they are kept as is to make it easier to backport changes.
Naming conventions
Valkey has a long history of inconsistent naming conventions. Generally follow the style of the surrounding code, but you can also always use the following conventions for variable and structure names:
- Variable names:
snake_caseor all lower case for short names (e.g.cached_replyorkeylen). - Function names:
camelCaseornamespace_camelCase(e.g.createStringObjectorIOJobQueue_isFull). - Macros:
UPPER_CASE(e.g.MAKE_CMD). - Structures:
camelCase(e.g.user).
Licensing information
When creating new source code files, use the following snippet to indicate the license:
/*
* Copyright (c) Valkey Contributors
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
*/
If you are making material changes to a file that has a different license at the top, also add the above license snippet. There isn't a well defined test for what is considered a material change, but a good rule of thumb is that material changes are more than 100 lines of code.
Test coverage
Valkey uses two types of tests: unit and integration tests. All contributions should include a test of some form.
Unit tests are present in the src/unit directory, and are intended to test individual structures or files.
For example, most changes to data structures should include corresponding unit tests.
Integration tests are located in the tests/ directory, and are intended to test end-to-end functionality.
Adding new commands should come with corresponding integration tests.
When writing cluster mode tests, do not use the legacy tests/cluster framework, which has been deprecated, and instead write tests in unit/cluster.
Documentation
Valkey keeps most of the user documentation in the valkey-doc repository in a few areas:
- Major functionality is documented in the topics section.
- Specific command behavior is documented in the commands section. Command history is also documented in the command json file.
- Server info fields are documented in the INFO command.
When a PR is opened that requires documentation to be updated, the needs-doc-pr should be added until the corresponding documentation PR is open.
Adding INFO fields
An INFO field is a name:value line in the output of the INFO command.
Fields are grouped into sections, each starting with a header line such as # Server or # Memory.
Users and monitoring tools parse these fields.
The following are best practices for adding INFO fields:
- Keep the number of INFO fields small.
- Before adding a metric, check whether users can derive it from existing fields.
- Prefer counters over gauges. Counters let monitoring systems compute changes over time, while gauges can miss short spikes that happen between samples.
- Choose the section for a new field based on the intended audience and whether the field is expected to remain stable:
- Metrics that are broadly useful to users belong in a default section, returned by
INFOandINFO default. Document these fields in the INFO command documentation. - Stable metrics for specific features or less common troubleshooting needs belong in a non-default section.
Document these fields too.
Non-default server sections are included in
INFO all, which monitoring tools may collect, so avoid adding unnecessary fields just because a section is not returned by default. - Internal debugging metrics that may change or be removed belong in the
debugsection and are not part of the user documentation. This section must be requested explicitly withINFO debug; it is excluded fromINFO,INFO all, andINFO everything.
- Metrics that are broadly useful to users belong in a default section, returned by